[LBSE] Outermost <svg> elements are not device-pixel aligned

https://bugs.webkit.org/show_bug.cgi?id=244966 Reviewed by Rob Buis. RenderLayer::paintLayerByApplingTransform() already contains all the code necessary to place outer <svg> elements on device-pixel aligned boundaries. The subpixelOffset is applied as transformation once for the outermost <svg> and thus to all descendants, since the outermost <svg> establishes a stacking context. The subpixelOffset is not relevant below RenderSVGRoot, since the rest of the SVG render tree is not applying device-pixel alignment. This fixes a few rendering differences between LBSE / legacy, as indicated by the LBSE specific expected.png removal. Add two new tests covering inline SVG positioning non non-integer coordinates, probing the sub-pixel / device-pixel alignment is done correctly. A follow-up patch will revisit this topic, considering also composited elements, testing their behaviour with respect to sub-pixel positioning + SVG + overflow + 3D transforms... * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/absolute-sized-svg-in-xhtml-expected.png: Removed. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/createImageElement2-expected.png: Removed. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/embedding-external-svgs-expected.png: Removed. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/image-parent-translation-expected.png: Removed. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/junk-data-expected.png: Removed. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/path-bad-data-expected.png: Removed. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/rootmost-svg-xy-attrs-expected.png: * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/hixie/error/012-expected.png: Removed. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/transforms/animated-path-inside-transformed-html-expected.png: Added. * LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/transforms/svg-css-transforms-expected.png: Removed. * LayoutTests/svg/compositing/outermost-svg-with-border-padding.html: Allow 1px more difference (seen by ews-mac-debug-wk2). * LayoutTests/svg/in-html/inline-svg-non-integer-position-display-block-expected.html: Added. * LayoutTests/svg/in-html/inline-svg-non-integer-position-display-block.html: Added. * LayoutTests/svg/in-html/inline-svg-non-integer-position-display-inline-expected.html: Added. * LayoutTests/svg/in-html/inline-svg-non-integer-position-display-inline.html: Added. * Source/WebCore/rendering/RenderLayerBacking.cpp: (WebCore::snappedGraphicsLayer): (WebCore::RenderLayerBacking::computeParentGraphicsLayerRect const): (WebCore::RenderLayerBacking::updateGeometry): (WebCore::RenderLayerBacking::adjustOverflowControlsPositionRelativeToAncestor): (WebCore::RenderLayerBacking::updateMaskingLayerGeometry): (WebCore::RenderLayerBacking::updateContentsRects): (WebCore::RenderLayerBacking::updateClippingStackLayerGeometry): (WebCore::RenderLayerBacking::setContentsNeedDisplayInRect): * Source/WebCore/rendering/svg/RenderSVGRoot.cpp: (WebCore::RenderSVGRoot::paintingAffectedByExternalOffset const): (WebCore::RenderSVGRoot::updateFromStyle): * Source/WebCore/rendering/svg/RenderSVGRoot.h: Canonical link: https://commits.webkit.org/254558@main
WebKit · Sep 16, 2022 · de2323d7e2c84bdeb40158ec03bf6130ed7584e6 · de2323d
1 parent 65202ec
commit de2323d7e2c84bdeb40158ec03bf6130ed7584e6
Show file tree

Hide file tree

Showing 17 changed files with 114 additions and 1 deletion.
diff --git a/.../mac-monterey-wk2-lbse-text/svg/custom/absolute-sized-svg-in-xhtml-expected.png b/.../mac-monterey-wk2-lbse-text/svg/custom/absolute-sized-svg-in-xhtml-expected.png
diff --git a/...platform/mac-monterey-wk2-lbse-text/svg/custom/createImageElement2-expected.png b/...platform/mac-monterey-wk2-lbse-text/svg/custom/createImageElement2-expected.png
diff --git a/...form/mac-monterey-wk2-lbse-text/svg/custom/embedding-external-svgs-expected.png b/...form/mac-monterey-wk2-lbse-text/svg/custom/embedding-external-svgs-expected.png
diff --git a/...orm/mac-monterey-wk2-lbse-text/svg/custom/image-parent-translation-expected.png b/...orm/mac-monterey-wk2-lbse-text/svg/custom/image-parent-translation-expected.png
diff --git a/LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/junk-data-expected.png b/LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/custom/junk-data-expected.png
diff --git a/...Tests/platform/mac-monterey-wk2-lbse-text/svg/custom/path-bad-data-expected.png b/...Tests/platform/mac-monterey-wk2-lbse-text/svg/custom/path-bad-data-expected.png
diff --git a/...atform/mac-monterey-wk2-lbse-text/svg/custom/rootmost-svg-xy-attrs-expected.png b/...atform/mac-monterey-wk2-lbse-text/svg/custom/rootmost-svg-xy-attrs-expected.png
diff --git a/LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/hixie/error/012-expected.png b/LayoutTests/platform/mac-monterey-wk2-lbse-text/svg/hixie/error/012-expected.png
diff --git a/...wk2-lbse-text/svg/transforms/animated-path-inside-transformed-html-expected.png b/...wk2-lbse-text/svg/transforms/animated-path-inside-transformed-html-expected.png
diff --git a/...tform/mac-monterey-wk2-lbse-text/svg/transforms/svg-css-transforms-expected.png b/...tform/mac-monterey-wk2-lbse-text/svg/transforms/svg-css-transforms-expected.png
diff --git a/LayoutTests/svg/compositing/outermost-svg-with-border-padding.html b/LayoutTests/svg/compositing/outermost-svg-with-border-padding.html
@@ -2,7 +2,7 @@
 <html>
 <head>
 <!-- FIXME: SVG content slightly clipped (up to 1px) due to off-device-pixel-alignment issue, remove when fixed. -->
-<meta name="fuzzy" content="maxDifference=111; totalPixels=848-852" />
+<meta name="fuzzy" content="maxDifference=111; totalPixels=848-853" />
 <style>
     html, body {
         margin: 0;

diff --git a/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-block-expected.html b/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-block-expected.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+  <head>
+  <style>
+    .box {
+      display: block;
+      position: relative;
+      top: 0px;
+      left: 0px;
+      width: 100px;
+      height: 100px;
+      background-color: green;
+    }
+  </style>
+  <body>
+    <h1>Enforce non-integer SVG location</h1>
+    <div class="box"></div>
+    <h1>Effect of overflow</h1>
+    <div class="box"></div>
+    <h1>With transformation</h1>
+    <div class="box"></div>
+  </body>
+</html>
diff --git a/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-block.html b/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-block.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <h1>Enforce non-integer SVG location</h1>
+    <svg width="100" height="100" style="overflow: visible; display: block;">
+      <rect x="0" y="0" width="100" height="100" fill="green"/>
+    </svg>
+    <h1>Effect of overflow</h1>
+    <svg width="100" height="100" style="overflow: hidden; display: block;">
+      <rect x="0" y="0" width="100" height="100" fill="green"/>
+    </svg>
+    <h1>With transformation</h1>
+    <svg width="50" height="50" style="overflow: visible; transform: scale(2); transform-origin: 0 0; display: block;">
+      <rect x="0" y="0" width="50" height="50" fill="green"/>
+    </svg>
+  </body>
+</html>
diff --git a/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-inline-expected.html b/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-inline-expected.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+  <head>
+  <style>
+    .box {
+      display: inline-block;
+      position: relative;
+      top: 0px;
+      left: 0px;
+      width: 100px;
+      height: 100px;
+      background-color: green;
+    }
+  </style>
+  <body>
+    <h1>Enforce non-integer SVG location</h1>
+    <div class="box"></div>
+    <h1>Effect of overflow</h1>
+    <div class="box"></div>
+    <h1>With transformation</h1>
+    <div class="box"></div>
+  </body>
+</html>
diff --git a/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-inline.html b/LayoutTests/svg/in-html/inline-svg-non-integer-position-display-inline.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <h1>Enforce non-integer SVG location</h1>
+    <svg width="100" height="100" style="overflow: visible; display: inline;">
+      <rect x="0" y="0" width="100" height="100" fill="green"/>
+    </svg>
+    <h1>Effect of overflow</h1>
+    <svg width="100" height="100" style="overflow: hidden; display: inline;">
+      <rect x="0" y="0" width="100" height="100" fill="green"/>
+    </svg>
+    <h1>With transformation</h1>
+    <svg width="50" height="50" style="overflow: visible; transform: scale(2); transform-origin: 0 0; display: inline;">
+      <rect x="0" y="0" width="50" height="50" fill="green"/>
+    </svg>
+  </body>
+</html>
diff --git a/Source/WebCore/rendering/svg/RenderSVGRoot.cpp b/Source/WebCore/rendering/svg/RenderSVGRoot.cpp
@@ -388,12 +388,44 @@ void RenderSVGRoot::styleDidChange(StyleDifference diff, const RenderStyle* oldS
     SVGResourcesCache::clientStyleChanged(*this, diff, oldStyle, style());
 }
 
+bool RenderSVGRoot::paintingAffectedByExternalOffset() const
+{
+    // Standalone SVGs have no parent above the outermost <svg> that could affect the positioning.
+    if (isDocumentElementRenderer())
+        return false;
+
+    // SVGs embedded via 'SVGImage' paint at (0, 0) by construction.
+    if (isEmbeddedThroughSVGImage())
+        return false;
+
+    // <object> / <embed> might receive a non-zero paintOffset.
+    if (isEmbeddedThroughFrameContainingSVGDocument())
+        return true;
+
+    // Inline SVG. A non-SVG ancestor might induce a non-zero paintOffset.
+    if (auto* parentNode = svgSVGElement().parentNode())
+        return !is<SVGElement>(parentNode);
+
+    ASSERT_NOT_REACHED();
+    return false;
+}
+
 void RenderSVGRoot::updateFromStyle()
 {
     RenderReplaced::updateFromStyle();
 
     if (shouldApplyViewportClip())
         setHasNonVisibleOverflow();
+
+    // Only mark us as transformed if really needed. Whenver a non-zero paintOffset could reach
+    // RenderSVGRoot from an ancestor, the pixel snapping logic needs to be applied. Since the rest
+    // of the SVG subtree doesn't know anything about subpixel offsets, we'll have to stop use/set
+    // 'adjustedSubpixelOffset' starting at the RenderSVGRoot boundary. This mostly affects inline
+    // SVG documents and SVGs embedded via <object> / <embed>.
+    if (!hasSVGTransform() && paintingAffectedByExternalOffset()) {
+        setHasTransformRelatedProperty();
+        setHasSVGTransform();
+    }
 }
 
 void RenderSVGRoot::updateLayerTransform()

diff --git a/Source/WebCore/rendering/svg/RenderSVGRoot.h b/Source/WebCore/rendering/svg/RenderSVGRoot.h
@@ -82,6 +82,7 @@ class RenderSVGRoot final : public RenderReplaced {
     bool requiresLayer() const final { return true; }
 
     bool updateLayoutSizeIfNeeded();
+    bool paintingAffectedByExternalOffset() const;
 
     // To prevent certain legacy code paths to hit assertions in debug builds, when switching off LBSE (during the teardown of the LBSE tree).
     std::optional<FloatRect> computeFloatVisibleRectInContainer(const FloatRect&, const RenderLayerModelObject*, VisibleRectContext) const final { return std::nullopt; }