Bug 68777 - Absolute child is not repainted when parent opacity changes
Summary: Absolute child is not repainted when parent opacity changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified All
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction
Depends on: 72779
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-25 11:44 PDT by Philip Rogers
Modified: 2011-11-18 17:40 PST (History)
8 users (show)

See Also:


Attachments
Repro case showing incorrect child repaint (958 bytes, text/html)
2011-09-25 11:44 PDT, Philip Rogers
no flags Details
Handle StyleDifferenceRepaintLayer at the end of setStyle() (6.55 KB, patch)
2011-10-17 12:58 PDT, Konstantin Shcheglov
no flags Details | Formatted Diff | Diff
Handle StyleDifferenceRepaintLayer at the end of setStyle() (6.55 KB, patch)
2011-10-18 10:00 PDT, Konstantin Shcheglov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2011-09-25 11:44:58 PDT
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.
Comment 1 Julien Chaffraix 2011-09-27 18:00:13 PDT
I couldn't reproduce it on the latest WebKit nightly (96162) but I could see it on ToT Qt and Chromium Canary build.
Comment 2 Philip Rogers 2011-09-28 07:27:12 PDT
(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.
Comment 3 Konstantin Shcheglov 2011-10-17 10:59:01 PDT
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).
Comment 4 Konstantin Shcheglov 2011-10-17 12:58:13 PDT
Created attachment 111304 [details]
Handle StyleDifferenceRepaintLayer at the end of setStyle()
Comment 5 WebKit Review Bot 2011-10-17 13:00:37 PDT
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.
Comment 6 Konstantin Shcheglov 2011-10-18 10:00:22 PDT
Created attachment 111458 [details]
Handle StyleDifferenceRepaintLayer at the end of setStyle()
Comment 7 David Barr 2011-11-17 14:30:57 PST
Looks good to me, maybe Darin or Simon would like to review?
Comment 8 WebKit Review Bot 2011-11-17 15:53:35 PST
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>
Comment 9 WebKit Review Bot 2011-11-17 15:53:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Simon Fraser (smfr) 2011-11-18 16:32:20 PST
This caused bug 72770.
Comment 11 David Barr 2011-11-18 16:35:31 PST
(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.