- Revision
- 268898
- Author
- [email protected]
- Date
- 2020-10-22 16:15:41 -0700 (Thu, 22 Oct 2020)
Log Message
Twitter Photo gallery incorrectly rendered after opening another modal
https://bugs.webkit.org/show_bug.cgi?id=217737
<rdar://problem/70314822>
Reviewed by Zalan Bujtas.
Source/WebCore:
If a layer with a certain configuration of clipping layers had non-zero border-radius, then
changes to zero border-radius, we'd fail to undo the composited rounded-corner masking.
This was because RenderLayerBacking::updateGeometry() didn't call setMasksToBoundsRect()
in the non-rounded corner case.
The compositing code in general tries to separate "configuration" changes (i.e. setting
up the right layer hierarchy) from "geometry" changes (setting positions and sizes).
In this area, those responsibilities were muddied because the return value from
setMasksToBoundsRect() was used to know whether the platform-specific GraphicsLayer
supports rounded-corner clips. This forced RenderLayerBacking::updateChildClippingStrategy()
to do some geometry computation.
Clean that up by adding the static GraphicsLayer::supportsRoundedClip(), and use
that to know if the platform needs a m_childClippingMaskLayer (all non-CA platforms).
Now updateChildClippingStrategy() can be purely about hierarchy, and updateGeometry()
can focus on geometry. In updateGeometry(), we are sure to call setMasksToBoundsRect()
in the non-rounded corner case too.
Test: compositing/style-change/border-radius-change.html
* platform/graphics/GraphicsLayer.cpp:
(WebCore::GraphicsLayer::supportsRoundedClip):
(WebCore::GraphicsLayer::setMasksToBoundsRect):
* platform/graphics/GraphicsLayer.h:
(WebCore::GraphicsLayer::setMasksToBoundsRect): Deleted.
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayer::supportsRoundedClip):
(WebCore::GraphicsLayerCA::setMasksToBoundsRect):
* platform/graphics/ca/GraphicsLayerCA.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateGeometry):
(WebCore::RenderLayerBacking::updateChildClippingStrategy):
LayoutTests:
* compositing/style-change/border-radius-change-expected.html: Added.
* compositing/style-change/border-radius-change.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (268897 => 268898)
--- trunk/LayoutTests/ChangeLog 2020-10-22 22:25:44 UTC (rev 268897)
+++ trunk/LayoutTests/ChangeLog 2020-10-22 23:15:41 UTC (rev 268898)
@@ -1,3 +1,14 @@
+2020-10-22 Simon Fraser <[email protected]>
+
+ Twitter Photo gallery incorrectly rendered after opening another modal
+ https://bugs.webkit.org/show_bug.cgi?id=217737
+ <rdar://problem/70314822>
+
+ Reviewed by Zalan Bujtas.
+
+ * compositing/style-change/border-radius-change-expected.html: Added.
+ * compositing/style-change/border-radius-change.html: Added.
+
2020-10-22 Lauro Moura <[email protected]>
[GLIB] Gardening some crashes
Added: trunk/LayoutTests/compositing/style-change/border-radius-change-expected.html (0 => 268898)
--- trunk/LayoutTests/compositing/style-change/border-radius-change-expected.html (rev 0)
+++ trunk/LayoutTests/compositing/style-change/border-radius-change-expected.html 2020-10-22 23:15:41 UTC (rev 268898)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .box {
+ width: 200px;
+ height: 200px;
+ margin: 10px;
+ background-color: silver;
+ }
+
+ .composited {
+ transform: translateZ(0);
+ }
+
+ .rounded {
+ margin: 20px;
+ overflow: hidden;
+ border-radius: 0px;
+ z-index: 0;
+ }
+
+ .rounded .composited {
+ background-color: blue;
+ width: 100%;
+ height: 100%;
+ }
+ </style>
+</head>
+<body>
+ <div class="composited rounded box">
+ <div class="composited">
+
+ </div>
+
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/compositing/style-change/border-radius-change.html (0 => 268898)
--- trunk/LayoutTests/compositing/style-change/border-radius-change.html (rev 0)
+++ trunk/LayoutTests/compositing/style-change/border-radius-change.html 2020-10-22 23:15:41 UTC (rev 268898)
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .box {
+ width: 200px;
+ height: 200px;
+ margin: 10px;
+ background-color: silver;
+ }
+
+ .composited {
+ transform: translateZ(0);
+ }
+
+ .rounded {
+ margin: 20px;
+ overflow: hidden;
+ border-radius: 50px;
+ z-index: 0;
+ }
+
+ .rounded .composited {
+ background-color: blue;
+ width: 100%;
+ height: 100%;
+ }
+
+ body.changed .rounded {
+ border-radius: 0px;
+ }
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ window.addEventListener('load', () => {
+ setTimeout(() => {
+ document.body.classList.add('changed');
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ }, false);
+ </script>
+</head>
+<body>
+ <div class="composited rounded box">
+ <div class="composited">
+
+ </div>
+
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (268897 => 268898)
--- trunk/Source/WebCore/ChangeLog 2020-10-22 22:25:44 UTC (rev 268897)
+++ trunk/Source/WebCore/ChangeLog 2020-10-22 23:15:41 UTC (rev 268898)
@@ -1,3 +1,44 @@
+2020-10-22 Simon Fraser <[email protected]>
+
+ Twitter Photo gallery incorrectly rendered after opening another modal
+ https://bugs.webkit.org/show_bug.cgi?id=217737
+ <rdar://problem/70314822>
+
+ Reviewed by Zalan Bujtas.
+
+ If a layer with a certain configuration of clipping layers had non-zero border-radius, then
+ changes to zero border-radius, we'd fail to undo the composited rounded-corner masking.
+ This was because RenderLayerBacking::updateGeometry() didn't call setMasksToBoundsRect()
+ in the non-rounded corner case.
+
+ The compositing code in general tries to separate "configuration" changes (i.e. setting
+ up the right layer hierarchy) from "geometry" changes (setting positions and sizes).
+ In this area, those responsibilities were muddied because the return value from
+ setMasksToBoundsRect() was used to know whether the platform-specific GraphicsLayer
+ supports rounded-corner clips. This forced RenderLayerBacking::updateChildClippingStrategy()
+ to do some geometry computation.
+
+ Clean that up by adding the static GraphicsLayer::supportsRoundedClip(), and use
+ that to know if the platform needs a m_childClippingMaskLayer (all non-CA platforms).
+ Now updateChildClippingStrategy() can be purely about hierarchy, and updateGeometry()
+ can focus on geometry. In updateGeometry(), we are sure to call setMasksToBoundsRect()
+ in the non-rounded corner case too.
+
+ Test: compositing/style-change/border-radius-change.html
+
+ * platform/graphics/GraphicsLayer.cpp:
+ (WebCore::GraphicsLayer::supportsRoundedClip):
+ (WebCore::GraphicsLayer::setMasksToBoundsRect):
+ * platform/graphics/GraphicsLayer.h:
+ (WebCore::GraphicsLayer::setMasksToBoundsRect): Deleted.
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayer::supportsRoundedClip):
+ (WebCore::GraphicsLayerCA::setMasksToBoundsRect):
+ * platform/graphics/ca/GraphicsLayerCA.h:
+ * rendering/RenderLayerBacking.cpp:
+ (WebCore::RenderLayerBacking::updateGeometry):
+ (WebCore::RenderLayerBacking::updateChildClippingStrategy):
+
2020-10-22 Chris Dumez <[email protected]>
Share more code between WorkerThread and AudioWorkletThread
Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp (268897 => 268898)
--- trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp 2020-10-22 22:25:44 UTC (rev 268897)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp 2020-10-22 23:15:41 UTC (rev 268898)
@@ -88,6 +88,11 @@
return false;
}
+bool GraphicsLayer::supportsRoundedClip()
+{
+ return false;
+}
+
bool GraphicsLayer::supportsBackgroundColorContent()
{
#if USE(TEXTURE_MAPPER)
@@ -382,6 +387,11 @@
m_maskLayer = WTFMove(layer);
}
+void GraphicsLayer::setMasksToBoundsRect(const FloatRoundedRect& roundedRect)
+{
+ m_masksToBoundsRect = roundedRect;
+}
+
Path GraphicsLayer::shapeLayerPath() const
{
#if USE(CA)
Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayer.h (268897 => 268898)
--- trunk/Source/WebCore/platform/graphics/GraphicsLayer.h 2020-10-22 22:25:44 UTC (rev 268897)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayer.h 2020-10-22 23:15:41 UTC (rev 268898)
@@ -465,9 +465,9 @@
virtual void setContentsRectClipsDescendants(bool b) { m_contentsRectClipsDescendants = b; }
// Set a rounded rect that is used to clip this layer and its descendants (implies setting masksToBounds).
- // Returns false if the platform can't support this rounded clip, and we should fall back to painting a mask.
+ // Consult supportsRoundedClip() to know whether non-zero radii are supported.
FloatRoundedRect maskToBoundsRect() const { return m_masksToBoundsRect; };
- virtual bool setMasksToBoundsRect(const FloatRoundedRect& roundedRect) { m_masksToBoundsRect = roundedRect; return false; }
+ virtual void setMasksToBoundsRect(const FloatRoundedRect&);
Path shapeLayerPath() const;
virtual void setShapeLayerPath(const Path&);
@@ -607,6 +607,7 @@
void resetTrackedRepaints();
void addRepaintRect(const FloatRect&);
+ static bool supportsRoundedClip();
static bool supportsBackgroundColorContent();
static bool supportsLayerType(Type);
static bool supportsContentsTiling();
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (268897 => 268898)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-10-22 22:25:44 UTC (rev 268897)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-10-22 23:15:41 UTC (rev 268898)
@@ -325,6 +325,11 @@
return true;
}
+bool GraphicsLayer::supportsRoundedClip()
+{
+ return true;
+}
+
bool GraphicsLayer::supportsSubpixelAntialiasedLayerText()
{
#if PLATFORM(MAC)
@@ -971,14 +976,13 @@
noteLayerPropertyChanged(ChildrenChanged | ContentsRectsChanged);
}
-bool GraphicsLayerCA::setMasksToBoundsRect(const FloatRoundedRect& roundedRect)
+void GraphicsLayerCA::setMasksToBoundsRect(const FloatRoundedRect& roundedRect)
{
if (roundedRect == m_masksToBoundsRect)
- return true;
+ return;
GraphicsLayer::setMasksToBoundsRect(roundedRect);
noteLayerPropertyChanged(MasksToBoundsRectChanged);
- return true;
}
void GraphicsLayerCA::setShapeLayerPath(const Path& path)
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (268897 => 268898)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h 2020-10-22 22:25:44 UTC (rev 268897)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h 2020-10-22 23:15:41 UTC (rev 268898)
@@ -122,7 +122,7 @@
WEBCORE_EXPORT void setContentsRect(const FloatRect&) override;
WEBCORE_EXPORT void setContentsClippingRect(const FloatRoundedRect&) override;
WEBCORE_EXPORT void setContentsRectClipsDescendants(bool) override;
- WEBCORE_EXPORT bool setMasksToBoundsRect(const FloatRoundedRect&) override;
+ WEBCORE_EXPORT void setMasksToBoundsRect(const FloatRoundedRect&) override;
WEBCORE_EXPORT void setShapeLayerPath(const Path&) override;
WEBCORE_EXPORT void setShapeLayerWindRule(WindRule) override;
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (268897 => 268898)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2020-10-22 22:25:44 UTC (rev 268897)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2020-10-22 23:15:41 UTC (rev 268898)
@@ -1322,12 +1322,18 @@
clipLayer->setSize(snappedClippingGraphicsLayer.m_snappedRect.size());
clipLayer->setOffsetFromRenderer(toLayoutSize(clippingBox.location() - snappedClippingGraphicsLayer.m_snapDelta));
- if ((renderer().style().clipPath() || renderer().style().hasBorderRadius()) && !m_childClippingMaskLayer) {
- FloatRoundedRect contentsClippingRect = renderBox.roundedBorderBoxRect().pixelSnappedRoundedRectForPainting(deviceScaleFactor);
- contentsClippingRect.move(LayoutSize(-clipLayer->offsetFromRenderer()));
- clipLayer->setMasksToBoundsRect(contentsClippingRect);
- }
+ auto computeMasksToBoundsRect = [&] {
+ if ((renderer().style().clipPath() || renderer().style().hasBorderRadius()) && !m_childClippingMaskLayer) {
+ FloatRoundedRect contentsClippingRect = renderBox.roundedBorderBoxRect().pixelSnappedRoundedRectForPainting(deviceScaleFactor);
+ contentsClippingRect.move(LayoutSize(-clipLayer->offsetFromRenderer()));
+ return contentsClippingRect;
+ }
+ return FloatRoundedRect { FloatRect { { }, snappedClippingGraphicsLayer.m_snappedRect.size() } };
+ };
+
+ clipLayer->setMasksToBoundsRect(computeMasksToBoundsRect());
+
if (m_childClippingMaskLayer && !m_scrollContainerLayer) {
m_childClippingMaskLayer->setSize(clipLayer->size());
m_childClippingMaskLayer->setPosition({ });
@@ -2210,33 +2216,21 @@
void RenderLayerBacking::updateChildClippingStrategy(bool needsDescendantsClippingLayer)
{
- if (hasClippingLayer() && needsDescendantsClippingLayer) {
- if (is<RenderBox>(renderer()) && (renderer().style().clipPath() || renderer().style().hasBorderRadius())) {
- auto* clipLayer = clippingLayer();
- FloatRoundedRect contentsClippingRect = downcast<RenderBox>(renderer()).roundedBorderBoxRect().pixelSnappedRoundedRectForPainting(deviceScaleFactor());
- contentsClippingRect.move(LayoutSize(-clipLayer->offsetFromRenderer()));
- // Note that we have to set this rounded rect again during the geometry update (clipLayer->offsetFromRenderer() may be stale here).
- if (clipLayer->setMasksToBoundsRect(contentsClippingRect)) {
- clipLayer->setMaskLayer(nullptr);
- GraphicsLayer::clear(m_childClippingMaskLayer);
- return;
- }
+ auto needsClipMaskLayer = [&] {
+ return needsDescendantsClippingLayer && !GraphicsLayer::supportsRoundedClip() && is<RenderBox>(renderer()) && (renderer().style().hasBorderRadius() || renderer().style().clipPath());
+ };
- if (!m_childClippingMaskLayer) {
- m_childClippingMaskLayer = createGraphicsLayer("child clipping mask");
- m_childClippingMaskLayer->setDrawsContent(true);
- m_childClippingMaskLayer->setPaintingPhase({ GraphicsLayerPaintingPhase::ChildClippingMask });
- clippingLayer()->setMaskLayer(m_childClippingMaskLayer.copyRef());
- }
- }
- } else {
- if (m_childClippingMaskLayer) {
- if (hasClippingLayer())
- clippingLayer()->setMaskLayer(nullptr);
- GraphicsLayer::clear(m_childClippingMaskLayer);
- } else
- if (hasClippingLayer())
- clippingLayer()->setMasksToBoundsRect(FloatRoundedRect(FloatRect({ }, clippingLayer()->size())));
+ auto* clippingLayer = this->clippingLayer();
+ if (needsClipMaskLayer()) {
+ m_childClippingMaskLayer = createGraphicsLayer("child clipping mask");
+ m_childClippingMaskLayer->setDrawsContent(true);
+ m_childClippingMaskLayer->setPaintingPhase({ GraphicsLayerPaintingPhase::ChildClippingMask });
+ if (clippingLayer)
+ clippingLayer->setMaskLayer(m_childClippingMaskLayer.copyRef());
+ } else if (m_childClippingMaskLayer) {
+ if (clippingLayer)
+ clippingLayer->setMaskLayer(nullptr);
+ GraphicsLayer::clear(m_childClippingMaskLayer);
}
}
