Bug 112457 – Don't compute background obscuration on every repaint

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Antti Koivisto]

There are less layouts than paints.

[Antti Koivisto]

Created attachment 193352 [details]
patch

[Simon Fraser (smfr)]

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 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]

Created attachment 193412 [details]
patch 2

Re-check the visibility on repaint-only style change. Add a test.

[WebKit Review Bot]

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 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]

Created attachment 193704 [details]
Caching and invalidation of obscuration status

[WebKit Review Bot]

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 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]

(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]

http://trac.webkit.org/changeset/146279