Bug 112457

Summary: Don't compute background obscuration on every repaint
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alokp, dglazkov, eric, esprehn+autocc, ojan.autocc, schenney, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
simon.fraser: review+, webkit.review.bot: commit-queue-
patch 2
webkit.review.bot: commit-queue-
Caching and invalidation of obscuration status simon.fraser: review+, webkit.review.bot: commit-queue-

Antti Koivisto
Reported 2013-03-15 12:18:22 PDT
There are less layouts than paints.
Attachments
patch (12.42 KB, patch)
2013-03-15 12:22 PDT, Antti Koivisto
simon.fraser: review+
webkit.review.bot: commit-queue-
patch 2 (14.53 KB, patch)
2013-03-15 18:48 PDT, Antti Koivisto
webkit.review.bot: commit-queue-
Caching and invalidation of obscuration status (18.27 KB, patch)
2013-03-18 17:34 PDT, Antti Koivisto
simon.fraser: review+
webkit.review.bot: commit-queue-
Antti Koivisto
Comment 1 2013-03-15 12:22:56 PDT
Simon Fraser (smfr)
Comment 2 2013-03-15 13:01:10 PDT
Comment on attachment 193352 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=193352&action=review > Source/WebCore/ChangeLog:8 > + We can put more effort into this in layout time as there are less layouts than paints. fewer layouts > Source/WebCore/rendering/RenderBox.h:596 > + virtual void determineIsBackgroundKnownToBeObscured(); Awkward name. Maybe updateBackgroundKnownToBeObscured() or computeBackgroundObscurity() > Source/WebCore/rendering/RenderObject.h:691 > + void setIsBackgroundKnownToBeObscured(bool b) { m_bitfields.setIsBackgroundKnownToBeObscured(b); } Should be setBackgroundIsKnownToBeObscured > Source/WebCore/rendering/RenderObject.h:1114 > + ADD_BOOLEAN_BITFIELD(isBackgroundKnownToBeObscured, IsBackgroundKnownToBeObscured); Member should be called backgroundIsKnownToBeObscured or just backgroundKnownToBeObscured
WebKit Review Bot
Comment 3 2013-03-15 16:44:50 PDT
Comment on attachment 193352 [details] patch Attachment 193352 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17118306 New failing tests: fast/replaced/border-radius-clip-content-edge.html
Antti Koivisto
Comment 4 2013-03-15 18:48:44 PDT
Created attachment 193412 [details] patch 2 Re-check the visibility on repaint-only style change. Add a test.
WebKit Review Bot
Comment 5 2013-03-15 19:44:58 PDT
Comment on attachment 193412 [details] patch 2 Attachment 193412 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17181352 New failing tests: fast/replaced/border-radius-clip-content-edge.html
Antti Koivisto
Comment 6 2013-03-18 10:05:23 PDT
Comment on attachment 193412 [details] patch 2 This is actually somewhat problematic. BitmapImage::frameHasAlphaAtIndex needs a decoded frame to know if it is transparent but we don't want to decode during layout (as it may never get painted). This is made less clear-cut but the fact that frameHasAlphaAtIndex() does not actually care about frames as the current implementation just checks if the image is jpg or something else.
Antti Koivisto
Comment 7 2013-03-18 17:34:34 PDT
Created attachment 193704 [details] Caching and invalidation of obscuration status
WebKit Review Bot
Comment 8 2013-03-18 17:37:40 PDT
Attachment 193704 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/css/obscured-background-child-style-change-expected.html', u'LayoutTests/fast/css/obscured-background-child-style-change.html', u'LayoutTests/fast/repaint/obscured-background-no-repaint.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderBlock.cpp', u'Source/WebCore/rendering/RenderBox.cpp', u'Source/WebCore/rendering/RenderBox.h', u'Source/WebCore/rendering/RenderImage.cpp', u'Source/WebCore/rendering/RenderImage.h', u'Source/WebCore/rendering/RenderObject.cpp', u'Source/WebCore/rendering/RenderObject.h', u'Source/WebCore/rendering/RenderReplaced.cpp']" exit_code: 1 Source/WebCore/rendering/RenderObject.h:697: The parameter name "b" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 9 2013-03-18 18:27:48 PDT
Comment on attachment 193704 [details] Caching and invalidation of obscuration status Attachment 193704 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16997997 New failing tests: fast/replaced/border-radius-clip-content-edge.html
Antti Koivisto
Comment 10 2013-03-18 18:44:16 PDT
(In reply to comment #9) > New failing tests: > fast/replaced/border-radius-clip-content-edge.html I think this just needs baseline update. The background color leaks under the shape in pixel results on chromium.
Antti Koivisto
Comment 11 2013-03-19 17:20:33 PDT
Note You need to log in before you can comment on or make changes to this bug.