[webkit-reviews] review denied: [Bug 121828] [CSS Regions] Activate all regions to have layers, as CSS Regions create a new stacking context : [Attachment 213002] patch. stacking contexts are not related to having backing.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Sep 30 09:53:58 PDT 2013
Alexandru Chiculita <achicu at adobe.com> has denied Mihai Maerean
<mmaerean at adobe.com>'s request for review:
Bug 121828: [CSS Regions] Activate all regions to have layers, as CSS Regions
create a new stacking context
https://bugs.webkit.org/show_bug.cgi?id=121828
Attachment 213002: patch. stacking contexts are not related to having backing.
https://bugs.webkit.org/attachment.cgi?id=213002&action=review
------- Additional Comments from Alexandru Chiculita <achicu at adobe.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213002&action=review
> LayoutTests/ChangeLog:6
> + Reviewed by NOBODY (OOPS!).
Please explain why you had to change the tests below.
> LayoutTests/fast/repaint/region-painting-composited-element-expected.html:13
> + position: relative; z-index: 1; /*create Stacking Context*/
nit: use two lines.
> Source/WebCore/ChangeLog:8
> + Test: fast/regions/stacking-context-paint-order.html
Please add more details on why the change is actually needed as we already
created layers in some cases before.
> Source/WebCore/rendering/RenderFlowThread.cpp:323
> + region->setRequiresLayerForCompositing();
It is not required to call setRequiresLayerForCompositing anymore as
requiresLayer is always going to be true.
> Source/WebCore/rendering/RenderRegion.cpp:722
> +void RenderRegion::setRequiresLayerForCompositing()
Please remove this function and all its friends:
RenderBox::updateLayerIfNeeded
RenderBox::updatePaintingContainerForFloatingObject
RenderBlock::updateFloatingObjectsPaintingContainer
RenderBlock::updateLocalFloatingObjectsForPaintingContainer
RenderBlock::updateAllDescendantsFloatingObjectsPaintingContainer
> Source/WebCore/rendering/RenderRegion.h:145
> + virtual bool requiresLayer() const { return true; } // Because CSS
Regions create Stacking Contexts.
I would rewrite this to handle the isRenderRegionSet() instead of overwriting
it again below.
More information about the webkit-reviews
mailing list