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

• Component: Layout and Rendering
• Status: NEW

[Philip Rogers]

Changing the opacity of a parent should trigger a repaint of 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 https://bugs.webkit.org/show_bug.cgi?id=68777 for a repro case.

This was originally fixed in https://bugs.webkit.org/show_bug.cgi?id=68777 but it caused a regression and was rolled back. I will post a new patch shortly which should fix this bug.

[Philip Rogers]

Created attachment 116664 [details]
Fix Absolute child is not repainted when parent opacity changes

The previous patch (https://bugs.webkit.org/show_bug.cgi?id=68777) caused a regression because it affected both the composited and the non-composited code paths. Stackoverflow (sometimes) kicks the renderer into a composited path, which repainted incorrectly with the previous patch.

The actual bug only occurs when non-composited layers are encountered. To see this, add -webkit-transform:translateZ(0) to the test case (which creates composited layers) and the problem will go away. This new patch is updated to only affect the non-composited case.

Some details about what's actually going on...
The problem is in the order of style[Will/Did]Change and the diff calculation in RenderObject::setStyle().

1. Before styleWillChange().
RenderObject::adjustStyleDifference() should determine that the diff = StyleDifferenceLayout because there is no layer but one needs to be created.

2. In styleWillChange().
RenderBoxModelObject::styleWillChange() for the parent sees diff=StyleDifferenceLayout and calls repaint().

3. In styleDidChange().
A new RenderLayer is created for the parent.

4. After styleDidChange().
RenderObject::adjustStyleDifference() is called again, with all layers ready. If the layer is not composited, we set diff = StyleDifferenceRepaintLayer and force a full repaint including descendants. The previous patch would do this even for composited layers, which leaves dirty areas during scrolling.

[Julien Chaffraix]

Comment on attachment 116664 [details]
Fix Absolute child is not repainted when parent opacity changes

View in context: https://bugs.webkit.org/attachment.cgi?id=116664&action=review

r- for the absence of a proper ChangeLog and the current change which needs some refinements.

Note: we usually reopen and reuse bugs when a change was rolled out as it makes it easier to follow the patch iterations.

> Source/WebCore/ChangeLog:9
> +

Your explanations from the bug should be here. The ChangeLog should - at least - explain why you are doing your change. Detailed explanations about each block of code changed is better but not mandatory. Here I had to tab between the bug and your patch as your ChangeLog is lagging.

> Source/WebCore/rendering/RenderObject.cpp:1703
> +                diff = StyleDifferenceLayout;

This looks like a bad change as you may force a layout when a repaint is just what is needed. Your explanations don't explain why we do need to fake a layout (changing opacity really does not mandate doing a layout). It looks like this was just a means for you to get your repaint later in setStyle().

The styleWillChange / styleDidChange logic should handle a proper repaint in this case without having to abuse the |diff|.

[Simon Fraser (smfr)]

Did this ever work properly?