Bug 1870200 - Break the cyclic dependency if any of the desendants us…

…es non-scaling-stroke. r=emilio

While we are computing the transform-box:stroke-box, for CSS Transforms or
Motion path Transforms, we have to avoid the cyclic dependency not only
for the SVG geometry frame itself, but also for the desendants of
the SVG container frame. Therefore, we just compute its fill-box (i.e.
make |getStroke| be false).

w3c/csswg-drafts#9640

Differential Revision: https://phabricator.services.mozilla.com/D203184

layout/base/MotionPathUtils.cpp

@@ -111,9 +111,11 @@ CSSCoord MotionPathUtils::GetRayContainReferenceSize(nsIFrame* aFrame) {
   const auto size = CSSSize::FromAppUnits(
       (aFrame->HasAnyStateBits(NS_FRAME_SVG_LAYOUT)
            ? nsLayoutUtils::ComputeSVGReferenceRect(
-                 aFrame, aFrame->StyleSVGReset()->HasNonScalingStroke()
-                             ? StyleGeometryBox::FillBox
-                             : StyleGeometryBox::StrokeBox)
+                 aFrame,
+                 aFrame->StyleSVGReset()->HasNonScalingStroke()
+                     ? StyleGeometryBox::FillBox
+                     : StyleGeometryBox::StrokeBox,
+                 nsLayoutUtils::MayHaveNonScalingStrokeCyclicDependency::Yes)
            : nsLayoutUtils::ComputeHTMLReferenceRect(
                  aFrame, StyleGeometryBox::BorderBox))
           .Size());

layout/base/nsLayoutUtils.cpp

@@ -9492,8 +9492,9 @@ nsRect nsLayoutUtils::ComputeSVGOriginBox(SVGViewportElement* aElement) {
 }
 
 /* static */
-nsRect nsLayoutUtils::ComputeSVGReferenceRect(nsIFrame* aFrame,
-                                              StyleGeometryBox aGeometryBox) {
+nsRect nsLayoutUtils::ComputeSVGReferenceRect(
+    nsIFrame* aFrame, StyleGeometryBox aGeometryBox,
+    MayHaveNonScalingStrokeCyclicDependency aMayHaveCyclicDependency) {
   MOZ_ASSERT(aFrame->GetContent()->IsSVGElement());
   nsRect r;
 
@@ -9502,9 +9503,12 @@ nsRect nsLayoutUtils::ComputeSVGReferenceRect(nsIFrame* aFrame,
       // XXX Bug 1299876
       // The size of stroke-box is not correct if this graphic element has
       // specific stroke-linejoin or stroke-linecap.
-      gfxRect bbox =
-          SVGUtils::GetBBox(aFrame, SVGUtils::eBBoxIncludeFillGeometry |
-                                        SVGUtils::eBBoxIncludeStroke);
+      const uint32_t flags = SVGUtils::eBBoxIncludeFillGeometry |
+                             SVGUtils::eBBoxIncludeStroke |
+                             (bool(aMayHaveCyclicDependency)
+                                  ? SVGUtils::eAvoidCycleIfNonScalingStroke
+                                  : 0);
+      gfxRect bbox = SVGUtils::GetBBox(aFrame, flags);
       r = nsLayoutUtils::RoundGfxRectToAppRect(bbox, AppUnitsPerCSSPixel());
       break;
     }

layout/base/nsLayoutUtils.h

@@ -2913,7 +2913,13 @@ class nsLayoutUtils {
 
   // Compute the geometry box for SVG layout. The caller should map the CSS box
   // into the proper SVG box.
-  static nsRect ComputeSVGReferenceRect(nsIFrame*, StyleGeometryBox);
+  // |aMayHaveCyclicDependency| is used for stroke-box to avoid the cyclic
+  // dependency if any of its descendants uses non-scaling-stroke.
+  enum class MayHaveNonScalingStrokeCyclicDependency : bool { No, Yes };
+  static nsRect ComputeSVGReferenceRect(
+      nsIFrame*, StyleGeometryBox,
+      MayHaveNonScalingStrokeCyclicDependency =
+          MayHaveNonScalingStrokeCyclicDependency::No);
 
   // Compute the geometry box for CSS layout. The caller should map the SVG box
   // into the proper CSS box.

layout/style/nsStyleTransformMatrix.cpp

@@ -101,7 +101,8 @@ static nsRect GetSVGBox(const nsIFrame* aFrame) {
       // FIXME: Bug 1849054. We may have to update
       // SVGGeometryFrame::GetBBoxContribution() to get tighter stroke bounds.
       nsRect strokeBox = nsLayoutUtils::ComputeSVGReferenceRect(
-          const_cast<nsIFrame*>(aFrame), StyleGeometryBox::StrokeBox);
+          const_cast<nsIFrame*>(aFrame), StyleGeometryBox::StrokeBox,
+          nsLayoutUtils::MayHaveNonScalingStrokeCyclicDependency::Yes);
       // The |nsIFrame::mRect| includes markers, so we have to compute the
       // offsets without markers.
       return nsRect{strokeBox.x - aFrame->GetPosition().x,

layout/svg/SVGGeometryFrame.cpp

@@ -358,13 +358,31 @@ SVGBBox SVGGeometryFrame::GetBBoxContribution(const Matrix& aToBBoxUserspace,
 
   SVGGeometryElement* element = static_cast<SVGGeometryElement*>(GetContent());
 
-  bool getFill = (aFlags & SVGUtils::eBBoxIncludeFillGeometry) ||
-                 ((aFlags & SVGUtils::eBBoxIncludeFill) &&
-                  !StyleSVG()->mFill.kind.IsNone());
-
-  bool getStroke =
-      (aFlags & SVGUtils::eBBoxIncludeStrokeGeometry) ||
-      ((aFlags & SVGUtils::eBBoxIncludeStroke) && SVGUtils::HasStroke(this));
+  const bool getFill = (aFlags & SVGUtils::eBBoxIncludeFillGeometry) ||
+                       ((aFlags & SVGUtils::eBBoxIncludeFill) &&
+                        !StyleSVG()->mFill.kind.IsNone());
+
+  const bool getStroke =
+      ((aFlags & SVGUtils::eBBoxIncludeStrokeGeometry) ||
+       ((aFlags & SVGUtils::eBBoxIncludeStroke) &&
+        SVGUtils::HasStroke(this))) &&
+      // If this frame has non-scaling-stroke and we would like to compute its
+      // stroke, it may cause a potential cyclical dependency if the caller is
+      // for transform. In this case, we have to fall back to fill-box, so make
+      // |getStroke| be false.
+      // https://github.com/w3c/csswg-drafts/issues/9640
+      //
+      // Note:
+      // 1. We don't care about the computation of the markers below in this
+      //    function because we know the callers don't set
+      //    SVGUtils::eBBoxIncludeMarkers.
+      //    See nsStyleTransformMatrix::GetSVGBox() and
+      //    MotionPathUtils::GetRayContainReferenceSize() for more details.
+      // 2. We have to break the dependency here *again* because the geometry
+      //    frame may be in the subtree of a SVGContainerFrame, which may not
+      //    set non-scaling-stroke.
+      !(StyleSVGReset()->HasNonScalingStroke() &&
+        (aFlags & SVGUtils::eAvoidCycleIfNonScalingStroke));
 
   SVGContentUtils::AutoStrokeOptions strokeOptions;
   if (getStroke) {

layout/svg/SVGUtils.h

@@ -346,6 +346,12 @@ class SVGUtils final {
     // For a frame with a clip-path, if this flag is set then the result
     // will not be clipped to the bbox of the content inside the clip-path.
     eDoNotClipToBBoxOfContentInsideClipPath = 1 << 10,
+    // For some cases, e.g. when using transform-box: stroke-box, we may have
+    // the cyclical dependency if any of the elements in the subtree has
+    // non-scaling-stroke. In this case, we should break it and use
+    // transform-box:fill-box instead.
+    // https://github.com/w3c/csswg-drafts/issues/9640
+    eAvoidCycleIfNonScalingStroke = 1 << 11,
   };
   /**
    * This function in primarily for implementing the SVG DOM function getBBox()

testing/web-platform/tests/css/css-transforms/transform-box/svgbox-stroke-box-005.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<title>transform-box: border-box, stroke with vector-effect: non-scaling-stroke</title>
+<link rel="match" href="reference/svgbox-rect-ref.html">
+<link rel="help" href="https://drafts.csswg.org/css-transforms-1/#transform-box">
+<link rel="help" href="https://svgwg.org/svg2-draft/coords.html#VectorEffects">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/9640">
+<meta name="assert" content="The used value of transform-box is fill-box on SVG with non-scaling-stroke"/>
+<style>
+#container {
+  fill: green;
+  stroke: black;
+  stroke-width: 20;
+  transform-box: border-box;
+  transform: scale(0.5) translateY(-100%);
+}
+#inner {
+  vector-effect: non-scaling-stroke;
+}
+</style>
+<svg width="400" height="300">
+  <a id="container">
+    <rect id="inner" width="100" height="200" x="50" y="180"/>
+  </a>
+</svg>