Bug 93901

Summary:

Add test to demonstrate overpainting seen in bug 92800

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

New Bugs

Assignee:

Eric Seidel (no email) <eric>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, jamesr, nduca, reed, rniwa, tony, tonyg, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

92800

Attachments:

Description	Flags
Patch	none
Now with less text dump	none
Archive of layout-test-results from gce-cq-02	none
Archive of layout-test-results from gce-cr-linux-04	none
Patch for landing	none

Eric Seidel (no email)

Reported 2012-08-13 14:39:47 PDT

Add test to demonstrate overpainting seen in bug 92800

Attachments
Patch (4.15 KB, patch) 2012-08-13 14:42 PDT, Eric Seidel (no email)	no flags	Details Formatted Diff Diff
Now with less text dump (3.12 KB, patch) 2012-08-13 15:10 PDT, Eric Seidel (no email)	no flags	Details Formatted Diff Diff
Archive of layout-test-results from gce-cq-02 (561.74 KB, application/zip) 2012-08-13 17:26 PDT, WebKit Review Bot	no flags	Details
Archive of layout-test-results from gce-cr-linux-04 (746.87 KB, application/zip) 2012-08-13 17:36 PDT, WebKit Review Bot	no flags	Details
Patch for landing (3.86 KB, patch) 2012-08-13 17:37 PDT, Eric Seidel (no email)	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2012-08-13 14:42:20 PDT

Created attachment 158109 [details] Patch

Levi Weintraub

Comment 2 2012-08-13 14:55:30 PDT

Comment on attachment 158109 [details] Patch LGTM

Eric Seidel (no email)

Comment 3 2012-08-13 15:10:04 PDT

Created attachment 158118 [details] Now with less text dump

Levi Weintraub

Comment 4 2012-08-13 15:11:35 PDT

Comment on attachment 158118 [details] Now with less text dump Better!

WebKit Review Bot

Comment 5 2012-08-13 17:26:48 PDT

Comment on attachment 158118 [details] Now with less text dump Rejecting attachment 158118 [details] from commit-queue. New failing tests: fast/repaint/add-table-overpaint.html Full output: http://queues.webkit.org/results/13494282

WebKit Review Bot

Comment 6 2012-08-13 17:26:51 PDT

Created attachment 158154 [details] Archive of layout-test-results from gce-cq-02 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: gce-cq-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

Eric Seidel (no email)

Comment 7 2012-08-13 17:29:22 PDT

Um, wow. Chrome is over-painting even more than Mac is. Unless repaint tests don't work on in chromium for some reason? Maybe the region coalescing code is different for Chromium and that's what we're seeing here?

WebKit Review Bot

Comment 8 2012-08-13 17:36:50 PDT

Comment on attachment 158118 [details] Now with less text dump Attachment 158118 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13494291 New failing tests: fast/repaint/add-table-overpaint.html

WebKit Review Bot

Comment 9 2012-08-13 17:36:55 PDT

Created attachment 158160 [details] Archive of layout-test-results from gce-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

Eric Seidel (no email)

Comment 10 2012-08-13 17:37:13 PDT

Created attachment 158161 [details] Patch for landing

James Robinson

Comment 11 2012-08-13 17:50:37 PDT

(In reply to comment #7) > Um, wow. Chrome is over-painting even more than Mac is. Unless repaint tests don't work on in chromium for some reason? > > Maybe the region coalescing code is different for Chromium and that's what we're seeing here? Chromium's DumpRenderTree implementation unions damage rects. On non-windows, scroll invalidations are treated as just invalidating the scrollable area. Neither of these behaviors match what Chromium itself does.

Eric Seidel (no email)

Comment 12 2012-08-13 17:55:46 PDT

(In reply to comment #11) > (In reply to comment #7) > > Um, wow. Chrome is over-painting even more than Mac is. Unless repaint tests don't work on in chromium for some reason? > > > > Maybe the region coalescing code is different for Chromium and that's what we're seeing here? > > Chromium's DumpRenderTree implementation unions damage rects. On non-windows, scroll invalidations are treated as just invalidating the scrollable area. Neither of these behaviors match what Chromium itself does. This seems sub-optimal. How far are we away from testing what Chromium actually does? It seems as-is the repaint tests are less-useful than they could be for Chromium.

Eric Seidel (no email)

Comment 13 2012-08-13 17:56:14 PDT

As in: do you have any idea how much work it would be to match the actual chromium repaint behavior in test_shell/DRT, etc.

James Robinson

Comment 14 2012-08-13 18:06:32 PDT

https://bugs.webkit.org/show_bug.cgi?id=53715 was meant to be the first step towards having the same logic available inside WebKit, but I think that stalled out. Nat?

WebKit Review Bot

Comment 15 2012-08-13 18:25:02 PDT

Comment on attachment 158161 [details] Patch for landing Clearing flags on attachment: 158161 Committed r125488: <http://trac.webkit.org/changeset/125488>

WebKit Review Bot

Comment 16 2012-08-13 18:25:07 PDT

All reviewed patches have been landed. Closing bug.

Tony Chang

Comment 17 2012-08-13 20:32:57 PDT

(In reply to comment #13) > As in: do you have any idea how much work it would be to match the actual chromium repaint behavior in test_shell/DRT, etc. I think the long term goal is to switch to using content_shell for running layout tests, which should use the same repaint logic as Chromium.

Nat Duca

Comment 18 2012-08-13 20:57:16 PDT

(In reply to comment #17) > I think the long term goal is to switch to using content_shell for running layout tests, which should use the same repaint logic as Chromium. The old work I was doing made the (now obsolete) assumption that we would preserve our software path. Our new approach of building a software backend on top of compositing (combined with content-shell tests) will hopefully result in a single repaint path.

Eric Seidel (no email)

Comment 19 2012-08-14 01:03:47 PDT

Excellent. Thank you both for the update.

Note You need to log in before you can comment on or make changes to this bug.