Diff
Modified: trunk/LayoutTests/ChangeLog (284335 => 284336)
--- trunk/LayoutTests/ChangeLog 2021-10-17 16:38:27 UTC (rev 284335)
+++ trunk/LayoutTests/ChangeLog 2021-10-17 16:54:24 UTC (rev 284336)
@@ -1,3 +1,20 @@
+2021-10-17 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (r270850): Reference clip path clips in the wrong place when inside non-visible overflow
+ https://bugs.webkit.org/show_bug.cgi?id=231852
+ <rdar://83186229>
+
+ Reviewed by Tim Horton.
+
+ Tests for reference clip-path with box-shadows and enclosing oveflow:hidden.
+
+ * css3/masking/reference-clip-path-bounds-expected.html: Added.
+ * css3/masking/reference-clip-path-bounds.html: Added.
+ * css3/masking/reference-clip-path-objectBoundingBox-buffer-clip-expected.html: Added.
+ * css3/masking/reference-clip-path-objectBoundingBox-buffer-clip.html: Added.
+ * css3/masking/reference-clip-path-objectBoundingBox-path-clip-expected.html: Added.
+ * css3/masking/reference-clip-path-objectBoundingBox-path-clip.html: Added.
+
2021-10-17 Tyler Wilcock <tyle...@apple.com>
AX: WebKit should not expose redundant AXGroups with missing role when the label is the same as the contents
Added: trunk/LayoutTests/css3/masking/reference-clip-path-bounds-expected.html (0 => 284336)
--- trunk/LayoutTests/css3/masking/reference-clip-path-bounds-expected.html (rev 0)
+++ trunk/LayoutTests/css3/masking/reference-clip-path-bounds-expected.html 2021-10-17 16:54:24 UTC (rev 284336)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .container {
+ position: absolute;
+ top: 20px;
+ border: 2px solid black;
+ width: 300px;
+ height: 300px;
+ overflow: hidden;
+ }
+
+ .clipped {
+ margin-left: 50px;
+ width: 200px;
+ height: 200px;
+ clip-path: inset(10px 40px 40px 10px);
+ background-color: green;
+ }
+ </style>
+</head>
+<body>
+ <div class="container">
+ <div class="clipped"></div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/css3/masking/reference-clip-path-bounds.html (0 => 284336)
--- trunk/LayoutTests/css3/masking/reference-clip-path-bounds.html (rev 0)
+++ trunk/LayoutTests/css3/masking/reference-clip-path-bounds.html 2021-10-17 16:54:24 UTC (rev 284336)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .container {
+ position: absolute;
+ top: 20px;
+ border: 2px solid black;
+ width: 300px;
+ height: 300px;
+ overflow: hidden;
+ }
+
+ .clipped {
+ margin-left: 50px;
+ width: 200px;
+ height: 200px;
+ clip-path: url(#rect-clip);
+ background-color: green;
+ }
+ </style>
+</head>
+<body>
+ <svg width="0" height="0">
+ <defs>
+ <clippath id="rect-clip">
+ <rect x="10" y="10" width="150" height="150"/>
+ </clippath>
+ </defs>
+ </svg>
+ <div class="container">
+ <div class="clipped"></div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-buffer-clip-expected.html (0 => 284336)
--- trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-buffer-clip-expected.html (rev 0)
+++ trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-buffer-clip-expected.html 2021-10-17 16:54:24 UTC (rev 284336)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .clipper {
+ position: absolute;
+ top: 150px;
+ left: 150px;
+ width: 100px;
+ height: 150px;
+ background: lime;
+ }
+
+ .clipper > div {
+ position: absolute;
+ width: 50px;
+ height: 200px;
+ background-color: blue;
+ }
+
+ .clipper .above {
+ top: -100px;
+ left: 0px;
+ height: 150px;
+ }
+
+ .clipper .below {
+ top: 50px;
+ left: 50px;
+ }
+ </style>
+</head>
+<body>
+ <div class="clipper">
+ <div class="below"></div>
+ <div class="above"></div>
+ </div>
+ <svg height="0" width="0">
+ <clipPath id="clipper" clipPathUnits="objectBoundingBox">
+ <!--Two rects triggers the buffer-based clipping in RenderSVGResourceClipper::applyClippingToContext() -->
+ <rect x="0.25" y="-0.25" width="0.25" height="1"/>
+ <rect x="0.25" y="0.25" width="0.5" height="1.5"/>
+ </clipPath>
+ </svg>
+</body>
+</html>
Added: trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-buffer-clip.html (0 => 284336)
--- trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-buffer-clip.html (rev 0)
+++ trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-buffer-clip.html 2021-10-17 16:54:24 UTC (rev 284336)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .clipper {
+ position: absolute;
+ top: 100px;
+ left: 100px;
+ width: 200px;
+ height: 200px;
+ clip-path: url(#clipper);
+ background: lime;
+ }
+
+ .clipper > div {
+ position: absolute;
+ width: 200px;
+ height: 200px;
+ background-color: blue;
+ }
+
+ .above {
+ top: -100px;
+ left: -100px;
+ }
+
+ .below {
+ top: 100px;
+ left: 100px;
+ }
+ </style>
+</head>
+<body>
+ <div class="clipper">
+ <div class="below"></div>
+ <div class="above"></div>
+ </div>
+ <svg height="0" width="0">
+ <clipPath id="clipper" clipPathUnits="objectBoundingBox">
+ <!--Two rects triggers the buffer-based clipping in RenderSVGResourceClipper::applyClippingToContext() -->
+ <rect x="0.25" y="-0.25" width="0.25" height="1"/>
+ <rect x="0.25" y="0.25" width="0.5" height="1.5"/>
+ </clipPath>
+ </svg>
+</body>
+</html>
Added: trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-path-clip-expected.html (0 => 284336)
--- trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-path-clip-expected.html (rev 0)
+++ trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-path-clip-expected.html 2021-10-17 16:54:24 UTC (rev 284336)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .clipper {
+ position: absolute;
+ top: 120px;
+ left: 170px;
+ width: 100px;
+ height: 200px;
+ background: lime;
+ }
+
+ .clipper > div {
+ position: absolute;
+ top: 100px;
+ left: 50px;
+ width: 50px;
+ height: 200px;
+ background-color: blue;
+ }
+ </style>
+</head>
+<body>
+ <div class="clipper">
+ <div></div>
+ </div>
+ <svg height="0" width="0">
+ <clipPath id="clipper" clipPathUnits="objectBoundingBox">
+ <rect x="0.25" y="-0.25" width="0.5" height="1.5"/>
+ </clipPath>
+ </svg>
+</body>
+</html>
Added: trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-path-clip.html (0 => 284336)
--- trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-path-clip.html (rev 0)
+++ trunk/LayoutTests/css3/masking/reference-clip-path-objectBoundingBox-path-clip.html 2021-10-17 16:54:24 UTC (rev 284336)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ .clipper {
+ position: absolute;
+ top: 100px;
+ left: 100px;
+ width: 200px;
+ height: 200px;
+ clip-path: url(#clipper);
+ background: lime;
+ margin: 20px;
+ }
+
+ .clipper > div {
+ position: absolute;
+ top: 100px;
+ left: 100px;
+ width: 200px;
+ height: 200px;
+ background-color: blue;
+ }
+ </style>
+</head>
+<body>
+ <div class="clipper">
+ <div></div>
+ </div>
+ <svg height="0" width="0">
+ <clipPath id="clipper" clipPathUnits="objectBoundingBox">
+ <rect x="0.25" y="0" width="0.5" height="1.5"/>
+ </clipPath>
+ </svg>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (284335 => 284336)
--- trunk/Source/WebCore/ChangeLog 2021-10-17 16:38:27 UTC (rev 284335)
+++ trunk/Source/WebCore/ChangeLog 2021-10-17 16:54:24 UTC (rev 284336)
@@ -1,3 +1,29 @@
+2021-10-17 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (r270850): Reference clip path clips in the wrong place when inside non-visible overflow
+ https://bugs.webkit.org/show_bug.cgi?id=231852
+ <rdar://83186229>
+
+ Reviewed by Tim Horton.
+
+ When clipping CSS boxes with reference clip paths we need to give RenderSVGResourceClipper two
+ different rectangles; the objectBoundingBox that is used to compute clip path geometry,
+ and the bounds of the clipped content, so that the buffer-based clipping code path
+ can create an ImageBuffer of the appropriate size.
+
+ Previously, we confounded these two rectangles, resulting in various bugs.
+
+ Tests: css3/masking/reference-clip-path-bounds.html
+ css3/masking/reference-clip-path-objectBoundingBox-buffer-clip.html
+ css3/masking/reference-clip-path-objectBoundingBox-path-clip.html
+
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::setupClipPath):
+ * rendering/svg/RenderSVGResourceClipper.cpp:
+ (WebCore::RenderSVGResourceClipper::applyResource):
+ (WebCore::RenderSVGResourceClipper::applyClippingToContext):
+ * rendering/svg/RenderSVGResourceClipper.h:
+
2021-10-17 Tyler Wilcock <tyle...@apple.com>
AX: WebKit should not expose redundant AXGroups with missing role when the label is the same as the contents
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (284335 => 284336)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2021-10-17 16:38:27 UTC (rev 284335)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2021-10-17 16:54:24 UTC (rev 284336)
@@ -3182,8 +3182,11 @@
if (is<RenderSVGRoot>(renderer()))
return false;
+ if (!is<RenderBox>(renderer()))
+ return false;
+
// It's not clear that this geometry is correct: https://github.com/w3c/csswg-drafts/issues/5786
- auto rootRelativeBounds = calculateLayerBounds(paintingInfo.rootLayer, offsetFromRoot, { UseLocalClipRectIfPossible });
+ auto clipPathObjectBoundingBox = computeReferenceRectFromBox(downcast<RenderBox>(renderer()), CSSBoxType::BorderBox, offsetFromRoot);
auto& style = renderer().style();
LayoutSize paintingOffsetFromRoot = LayoutSize(snapSizeToDevicePixel(offsetFromRoot + paintingInfo.subpixelOffset, LayoutPoint(), renderer().document().deviceScaleFactor()));
@@ -3190,7 +3193,8 @@
ASSERT(style.clipPath());
if (is<ShapeClipPathOperation>(*style.clipPath()) || (is<BoxClipPathOperation>(*style.clipPath()) && is<RenderBox>(renderer()))) {
WindRule windRule;
- Path path = computeClipPath(paintingOffsetFromRoot, rootRelativeBounds, windRule);
+ // FIXME: Should probably pixel-snap clipPathObjectBoundingBox here.
+ Path path = computeClipPath(paintingOffsetFromRoot, clipPathObjectBoundingBox, windRule);
context.save();
context.clipPath(path, windRule);
return true;
@@ -3200,11 +3204,15 @@
auto& referenceClipPathOperation = downcast<ReferenceClipPathOperation>(*style.clipPath());
if (auto* clipperRenderer = renderer().ensureReferencedSVGResources().referencedClipperRenderer(renderer().document(), referenceClipPathOperation)) {
context.save();
- auto referenceBox = snapRectToDevicePixels(rootRelativeBounds, renderer().document().deviceScaleFactor());
+ auto referenceBox = snapRectToDevicePixels(clipPathObjectBoundingBox, renderer().document().deviceScaleFactor());
auto offset = referenceBox.location();
+
+ auto clippedContentBounds = FloatRect(calculateLayerBounds(paintingInfo.rootLayer, offsetFromRoot, { UseLocalClipRectIfPossible }));
+ clippedContentBounds.moveBy(-offset);
+
context.translate(offset);
- FloatRect svgReferenceBox { {}, referenceBox.size() };
- clipperRenderer->applyClippingToContext(context, renderer(), svgReferenceBox, renderer().style().effectiveZoom());
+ FloatRect clipPathReferenceBox { { }, referenceBox.size() };
+ clipperRenderer->applyClippingToContext(context, renderer(), clipPathReferenceBox, clippedContentBounds, renderer().style().effectiveZoom());
context.translate(-offset);
return true;
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp (284335 => 284336)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp 2021-10-17 16:38:27 UTC (rev 284335)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp 2021-10-17 16:54:24 UTC (rev 284336)
@@ -79,7 +79,8 @@
if (repaintRect.isEmpty())
return true;
- return applyClippingToContext(*context, renderer, renderer.objectBoundingBox());
+ auto boundingBox = renderer.objectBoundingBox();
+ return applyClippingToContext(*context, renderer, boundingBox, boundingBox);
}
bool RenderSVGResourceClipper::pathOnlyClipping(GraphicsContext& context, const AffineTransform& animatedLocalTransform, const FloatRect& objectBoundingBox, float effectiveZoom)
@@ -139,11 +140,11 @@
return true;
}
-bool RenderSVGResourceClipper::applyClippingToContext(GraphicsContext& context, RenderElement& renderer, const FloatRect& objectBoundingBox, float effectiveZoom)
+bool RenderSVGResourceClipper::applyClippingToContext(GraphicsContext& context, RenderElement& renderer, const FloatRect& objectBoundingBox, const FloatRect& clippedContentBounds, float effectiveZoom)
{
ClipperData& clipperData = addRendererToClipper(renderer);
- LOG_WITH_STREAM(SVG, stream << "RenderSVGResourceClipper " << this << " applyClippingToContext: renderer " << &renderer << " objectBoundingBox " << objectBoundingBox << " (existing image buffer " << clipperData.imageBuffer.get() << ")");
+ LOG_WITH_STREAM(SVG, stream << "RenderSVGResourceClipper " << this << " applyClippingToContext: renderer " << &renderer << " objectBoundingBox " << objectBoundingBox << " clippedContentBounds " << clippedContentBounds << " (existing image buffer " << clipperData.imageBuffer.get() << ")");
AffineTransform animatedLocalTransform = clipPathElement().animatedLocalTransform();
@@ -151,13 +152,13 @@
return true;
AffineTransform absoluteTransform = SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(renderer);
- if (!clipperData.isValidForGeometry(objectBoundingBox, absoluteTransform)) {
+ if (!clipperData.isValidForGeometry(objectBoundingBox, clippedContentBounds, absoluteTransform)) {
// FIXME (149469): This image buffer should not be unconditionally unaccelerated. Making it match the context breaks nested clipping, though.
- auto maskImage = SVGRenderingContext::createImageBuffer(objectBoundingBox, absoluteTransform, DestinationColorSpace::SRGB(), RenderingMode::Unaccelerated, &context);
+ auto maskImage = SVGRenderingContext::createImageBuffer(clippedContentBounds, absoluteTransform, DestinationColorSpace::SRGB(), RenderingMode::Unaccelerated, &context);
if (!maskImage)
return false;
- clipperData = { WTFMove(maskImage), objectBoundingBox, absoluteTransform };
+ clipperData = { WTFMove(maskImage), objectBoundingBox, clippedContentBounds, absoluteTransform };
GraphicsContext& maskContext = clipperData.imageBuffer->context();
maskContext.concatCTM(animatedLocalTransform);
@@ -169,7 +170,7 @@
if (resources && (clipper = resources->clipper())) {
GraphicsContextStateSaver stateSaver(maskContext);
- if (!clipper->applyClippingToContext(maskContext, *this, objectBoundingBox))
+ if (!clipper->applyClippingToContext(maskContext, *this, objectBoundingBox, clippedContentBounds))
return false;
succeeded = drawContentIntoMaskImage(*clipperData.imageBuffer, objectBoundingBox, effectiveZoom);
@@ -184,7 +185,7 @@
if (!clipperData.imageBuffer)
return false;
- SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, objectBoundingBox, clipperData.imageBuffer, true);
+ SVGRenderingContext::clipToImageBuffer(context, absoluteTransform, clippedContentBounds, clipperData.imageBuffer, true);
return true;
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h (284335 => 284336)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h 2021-10-17 16:38:27 UTC (rev 284335)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.h 2021-10-17 16:54:24 UTC (rev 284336)
@@ -45,7 +45,9 @@
// clipPath can be clipped too, but don't have a boundingBox or repaintRect. So we can't call
// applyResource directly and use the rects from the object, since they are empty for RenderSVGResources
// FIXME: We made applyClippingToContext public because we cannot call applyResource on HTML elements (it asserts on RenderObject::objectBoundingBox)
- bool applyClippingToContext(GraphicsContext&, RenderElement&, const FloatRect&, float effectiveZoom = 1);
+ // objectBoundingBox ia used to compute clip path geometry when clipPathUnits="objectBoundingBox".
+ // clippedContentBounds is the bounds of the content to which clipping is being applied.
+ bool applyClippingToContext(GraphicsContext&, RenderElement&, const FloatRect& objectBoundingBox, const FloatRect& clippedContentBounds, float effectiveZoom = 1);
FloatRect resourceBoundingBox(const RenderObject&) override;
RenderSVGResourceType resourceType() const override { return ClipperResourceType; }
@@ -59,20 +61,22 @@
struct ClipperData {
FloatRect objectBoundingBox;
+ FloatRect clippedContentBounds;
AffineTransform absoluteTransform;
RefPtr<ImageBuffer> imageBuffer;
ClipperData() = default;
- ClipperData(RefPtr<ImageBuffer>&& buffer, const FloatRect& boundingBox, const AffineTransform& transform)
+ ClipperData(RefPtr<ImageBuffer>&& buffer, const FloatRect& boundingBox, const FloatRect& clippedBounds, const AffineTransform& transform)
: objectBoundingBox(boundingBox)
+ , clippedContentBounds(clippedBounds)
, absoluteTransform(transform)
, imageBuffer(WTFMove(buffer))
{
}
- bool isValidForGeometry(const FloatRect& boundingBox, const AffineTransform& transform) const
+ bool isValidForGeometry(const FloatRect& boundingBox, const FloatRect& clippedBounds, const AffineTransform& transform) const
{
- return imageBuffer && objectBoundingBox == boundingBox && absoluteTransform == transform;
+ return imageBuffer && objectBoundingBox == boundingBox && clippedContentBounds == clippedBounds && absoluteTransform == transform;
}
};
