Bug 68777 – Absolute child is not repainted when parent opacity changes

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

[Philip Rogers]

Created attachment 108608 [details]
Repro case showing incorrect child repaint

Changing the opacity of a parent should trigger a repaint on all children but absolutely positioned children are not repainted correctly. Instead of repainting the entire child, only the parts of the child that are in the parent's bounding box are repainted (see attached repro).

Tested on WebKit nightly @r95921/Mac, Chrome 16.0.889.0/Mac, and Chrome 14.0.835.186/Linux

As a workaround users can force a repaint by setting opacity=0.999 on the child.

[Julien Chaffraix]

I couldn't reproduce it on the latest WebKit nightly (96162) but I could see it on ToT Qt and Chromium Canary build.

[Philip Rogers]

(In reply to comment #1)
> I couldn't reproduce it on the latest WebKit nightly (96162) but I could see it on ToT Qt and Chromium Canary build.

I am still able to repro it on WebKit nightly @r96196 on Mac OSX 1.6.8.

[Konstantin Shcheglov]

I can reproduce this with Chromium, with USE(ACCELERATED_COMPOSITING).

Trick is that first change of opacity is not propagated to children.
Problem is in order of style[Will/Did]Change and diff calculation in RenderObject::setStyle().

1. Before styleWillChange().
RenderObject::adjustStyleDifference() correctly determines that diff=StyleDifferenceRepaintLayer. However at this point there are no layer (i.e. !hasLayer()). So, diff=StyleDifferenceRepaint used instead.

2. In styleWillChange().
RenderBoxModelObject::styleWillChange() for parent, sees diff=StyleDifferenceRepaint and calls repaint(). In contrast, second time, there is already layer, so layer()->repaintIncludingDescendants() is used.

3. In styleDidChange().
New RenderLayer is created for parent (and child layer moved to this new layer). So, now parent has layer.

4. After styleDidChange().
RenderObject::adjustStyleDifference() is called again, with all layers ready, so now diff=StyleDifferenceRepaintLayer. But this diff handled as just StyleDifferenceRepaint (at very end of RenderObject::setStyle()) and causes just repaint(), i.e. again not enough to repaint child (its layer).

[Konstantin Shcheglov]

Created attachment 111304 [details]
Handle StyleDifferenceRepaintLayer at the end of setStyle()

[WebKit Review Bot]

Attachment 111304 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
From git://git.webkit.org/WebKit
   a24babb..abfc01f  master     -> origin/master
	M	Source/WebCore/dom/Element.cpp
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/rendering/svg/RenderSVGResource.cpp
	M	Source/WebCore/rendering/style/StyleRareNonInheritedData.h
	M	Source/WebCore/rendering/style/StyleInheritedData.cpp
	M	Source/WebCore/rendering/style/StyleRareInheritedData.cpp
	M	Source/WebCore/rendering/style/StyleMultiColData.h
	M	Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp
	M	Source/WebCore/rendering/style/SVGRenderStyleDefs.cpp
	M	Source/WebCore/rendering/style/StyleInheritedData.h
	M	Source/WebCore/rendering/style/RenderStyle.cpp
	M	Source/WebCore/rendering/style/StyleMultiColData.cpp
	M	Source/WebCore/rendering/style/StyleRareInheritedData.h
	M	Source/WebCore/rendering/style/SVGRenderStyle.h
	M	Source/WebCore/rendering/style/SVGRenderStyleDefs.h
	M	Source/WebCore/rendering/style/RenderStyleConstants.h
	M	Source/WebCore/rendering/style/RenderStyle.h
	M	Source/WebCore/css/CSSStyleSelector.cpp
	M	Source/WebCore/css/CSSStyleSelector.h
	M	Source/WebCore/css/SVGCSSStyleSelector.cpp
	M	Source/WebCore/css/CSSStyleApplyProperty.cpp
r97638 = abfc01f879080fed6c628f7a9ab659c8d234a4b2 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/trunk.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Konstantin Shcheglov]

Created attachment 111458 [details]
Handle StyleDifferenceRepaintLayer at the end of setStyle()

[David Barr]

Looks good to me, maybe Darin or Simon would like to review?

[WebKit Review Bot]

Comment on attachment 111458 [details]
Handle StyleDifferenceRepaintLayer at the end of setStyle()

Clearing flags on attachment: 111458

Committed r100693: <http://trac.webkit.org/changeset/100693>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

This caused bug 72770.

[David Barr]

(In reply to comment #10)
> This caused bug 72770.

:( My LGTM is not yet sufficient.
I guess we were both caught out - such an innocent looking patch.