SVG: Increase hit-testing precision on strokes

Skia takes a precision value when generating strokes as an optimization.
When rendering, we pass a value derived from the scale-factor in the
CTM, but when hit-testing we were passing a hard-coded value.  This
worked reasonably well for solid strokes at moderate scales.  However,
at sufficiently high scales and especially for dashed strokes, the
stroke generated for hit-testing was noticeably different from the
rendered stroke.

This change passes the CTM through to the hit-testing code, so that we
can use the same high precision for hit testing as rendering.

Bug: 964614
Change-Id: I8dfcb3ade86b9e7b63c7415baab5814ac2286ce8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2112955Reviewed-by: Ian Prest <iapres@microsoft.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Ian Prest <iapres@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#759761}

third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc

@@ -187,11 +187,18 @@ bool LayoutSVGShape::ShapeDependentStrokeContains(
       // The reason is similar to the above code about HasPath().
       if (!rare_data_)
         UpdateNonScalingStrokeData();
+
+      // Un-scale to get back to the root-transform (cheaper than re-computing
+      // the root transform from scratch).
+      AffineTransform root_transform;
+      root_transform.Scale(StyleRef().EffectiveZoom())
+          .Multiply(NonScalingStrokeTransform());
+
       stroke_path_cache_ = std::make_unique<Path>(
-          NonScalingStrokePath().StrokePath(stroke_data));
+          NonScalingStrokePath().StrokePath(stroke_data, root_transform));
     } else {
-      stroke_path_cache_ =
-          std::make_unique<Path>(path_->StrokePath(stroke_data));
+      stroke_path_cache_ = std::make_unique<Path>(
+          path_->StrokePath(stroke_data, ComputeRootTransform()));
     }
   }
 
@@ -300,19 +307,23 @@ void LayoutSVGShape::UpdateLayout() {
   ClearNeedsLayout();
 }
 
+AffineTransform LayoutSVGShape::ComputeRootTransform() const {
+  const LayoutObject* root = this;
+  while (root && !root->IsSVGRoot())
+    root = root->Parent();
+  return LocalToAncestorTransform(ToLayoutSVGRoot(root)).ToAffineTransform();
+}
+
 AffineTransform LayoutSVGShape::ComputeNonScalingStrokeTransform() const {
   // Compute the CTM to the SVG root. This should probably be the CTM all the
   // way to the "canvas" of the page ("host" coordinate system), but with our
   // current approach of applying/painting non-scaling-stroke, that can break in
   // unpleasant ways (see crbug.com/747708 for an example.) Maybe it would be
   // better to apply this effect during rasterization?
-  const LayoutObject* root = this;
-  while (root && !root->IsSVGRoot())
-    root = root->Parent();
   AffineTransform host_transform;
   host_transform.Scale(1 / StyleRef().EffectiveZoom())
-      .Multiply(
-          LocalToAncestorTransform(ToLayoutSVGRoot(root)).ToAffineTransform());
+      .Multiply(ComputeRootTransform());
+
   // Width of non-scaling stroke is independent of translation, so zero it out
   // here.
   host_transform.SetE(0);

third_party/blink/renderer/core/layout/svg/layout_svg_shape.h

@@ -93,6 +93,7 @@ class LayoutSVGShape : public LayoutSVGModelObject {
     return rare_data_->non_scaling_stroke_transform_;
   }
 
+  AffineTransform ComputeRootTransform() const;
   AffineTransform ComputeNonScalingStrokeTransform() const;
   AffineTransform LocalSVGTransform() const final { return local_transform_; }
 

third_party/blink/renderer/core/svg/svg_geometry_element.cc

@@ -118,9 +118,17 @@ bool SVGGeometryElement::isPointInStroke(SVGPointTearOff* point) const {
         layout_shape.ComputeNonScalingStrokeTransform();
     path.Transform(transform);
     local_point = transform.MapPoint(local_point);
+
+    // Un-scale to get back to the root-transform (cheaper than re-computing
+    // the root transform from scratch).
+    AffineTransform root_transform;
+    root_transform.Scale(layout_shape.StyleRef().EffectiveZoom())
+        .Multiply(transform);
+    return path.StrokeContains(local_point, stroke_data, root_transform);
   }
   // Path::StrokeContains will reject points with a non-finite component.
-  return path.StrokeContains(local_point, stroke_data);
+  return path.StrokeContains(local_point, stroke_data,
+                             layout_shape.ComputeRootTransform());
 }
 
 Path SVGGeometryElement::ToClipPath() const {

third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc

@@ -852,7 +852,7 @@ bool BaseRenderingContext2D::IsPointInStrokeInternal(const Path& path,
   std::copy(GetState().LineDash().begin(), GetState().LineDash().end(),
             line_dash.begin());
   stroke_data.SetLineDash(line_dash, GetState().LineDashOffset());
-  return path.StrokeContains(transformed_point, stroke_data);
+  return path.StrokeContains(transformed_point, stroke_data, ctm);
 }
 
 void BaseRenderingContext2D::clearRect(double x,

third_party/blink/renderer/platform/graphics/path.cc

@@ -82,27 +82,30 @@ bool Path::Contains(const FloatPoint& point, WindRule rule) const {
   return path_.contains(x, y);
 }
 
-// FIXME: this method ignores the CTM and may yield inaccurate results for large
-// scales.
-SkPath Path::StrokePath(const StrokeData& stroke_data) const {
+SkPath Path::StrokePath(const StrokeData& stroke_data,
+                        const AffineTransform& transform) const {
+  float stroke_precision = clampTo<float>(
+      sqrt(std::max(transform.XScaleSquared(), transform.YScaleSquared())));
+  return StrokePath(stroke_data, stroke_precision);
+}
+
+SkPath Path::StrokePath(const StrokeData& stroke_data,
+                        float stroke_precision) const {
   PaintFlags flags;
   stroke_data.SetupPaint(&flags);
 
-  // Skia stroke resolution scale. This is multiplied by 4 internally
-  // (i.e. 1.0 corresponds to 1/4 pixel res).
-  static const SkScalar kResScale = 0.3f;
-
   SkPath stroke_path;
-  flags.getFillPath(path_, &stroke_path, nullptr, kResScale);
+  flags.getFillPath(path_, &stroke_path, nullptr, stroke_precision);
 
   return stroke_path;
 }
 
 bool Path::StrokeContains(const FloatPoint& point,
-                          const StrokeData& stroke_data) const {
+                          const StrokeData& stroke_data,
+                          const AffineTransform& transform) const {
   if (!std::isfinite(point.X()) || !std::isfinite(point.Y()))
     return false;
-  return StrokePath(stroke_data)
+  return StrokePath(stroke_data, transform)
       .contains(SkScalar(point.X()), SkScalar(point.Y()));
 }
 
@@ -111,7 +114,9 @@ FloatRect Path::BoundingRect() const {
 }
 
 FloatRect Path::StrokeBoundingRect(const StrokeData& stroke_data) const {
-  return StrokePath(stroke_data).computeTightBounds();
+  // Skia stroke resolution scale for reduced-precision requirements.
+  constexpr float kStrokePrecision = 0.3f;
+  return StrokePath(stroke_data, kStrokePrecision).computeTightBounds();
 }
 
 static FloatPoint* ConvertPathPoints(FloatPoint dst[],

third_party/blink/renderer/platform/graphics/path.h

@@ -81,8 +81,14 @@ class PLATFORM_EXPORT Path {
 
   bool Contains(const FloatPoint&) const;
   bool Contains(const FloatPoint&, WindRule) const;
-  bool StrokeContains(const FloatPoint&, const StrokeData&) const;
-  SkPath StrokePath(const StrokeData&) const;
+
+  // Determine if the path's stroke contains the point.  The transform is used
+  // only to determine the precision factor when analyzing the stroke, so that
+  // we return accurate results in high-zoom scenarios.
+  bool StrokeContains(const FloatPoint&,
+                      const StrokeData&,
+                      const AffineTransform&) const;
+  SkPath StrokePath(const StrokeData&, const AffineTransform&) const;
 
   FloatRect BoundingRect() const;
   FloatRect StrokeBoundingRect(const StrokeData&) const;
@@ -193,6 +199,7 @@ class PLATFORM_EXPORT Path {
                   float radius_y,
                   float start_angle,
                   float end_angle);
+  SkPath StrokePath(const StrokeData&, float stroke_precision) const;
 
   SkPath path_;
 };

third_party/blink/web_tests/external/wpt/2dcontext/path-objects/2d.path.isPointInStroke.scaleddashes.html

+<!DOCTYPE html>
+<!-- DO NOT EDIT! This test has been generated by /2dcontext/tools/gentest.py. -->
+<title>Canvas test: 2d.path.isPointInStroke.scaleddashes</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/2dcontext/resources/canvas-tests.js"></script>
+<link rel="stylesheet" href="/2dcontext/resources/canvas-tests.css">
+<body class="show_output">
+
+<h1>2d.path.isPointInStroke.scaleddashes</h1>
+<p class="desc">isPointInStroke() should return correct results on dashed paths at high scale factors</p>
+
+
+<p class="output">Actual output:</p>
+<canvas id="c" class="output" width="100" height="50"><p class="fallback">FAIL (fallback content)</p></canvas>
+
+<ul id="d"></ul>
+<script>
+var t = async_test("isPointInStroke() should return correct results on dashed paths at high scale factors");
+_addTest(function(canvas, ctx) {
+
+var scale = 20;
+ctx.setLineDash([10, 21.4159]); // dash from t=0 to t=10 along the circle
+ctx.scale(scale, scale);
+ctx.ellipse(6, 10, 5, 5, 0, 2*Math.PI, false);
+ctx.stroke();
+
+// hit-test the beginning of the dash (t=0)
+_assertSame(ctx.isPointInStroke(11*scale, 10*scale), true, "ctx.isPointInStroke(11*scale, 10*scale)", "true");
+// hit-test the middle of the dash (t=5)
+_assertSame(ctx.isPointInStroke(8.70*scale, 14.21*scale), true, "ctx.isPointInStroke(8.70*scale, 14.21*scale)", "true");
+// hit-test the end of the dash (t=9.8)
+_assertSame(ctx.isPointInStroke(4.10*scale, 14.63*scale), true, "ctx.isPointInStroke(4.10*scale, 14.63*scale)", "true");
+// hit-test past the end of the dash (t=10.2)
+_assertSame(ctx.isPointInStroke(3.74*scale, 14.46*scale), false, "ctx.isPointInStroke(3.74*scale, 14.46*scale)", "false");
+
+
+});
+</script>
+

third_party/blink/web_tests/external/wpt/2dcontext/tools/tests2d.yaml

@@ -8646,6 +8646,27 @@
     @assert ctx.isPointInPath(NaN, NaN) === false;
 
 
+- name: 2d.path.isPointInStroke.scaleddashes
+  desc: isPointInStroke() should return correct results on dashed paths at high scale factors
+  testing:
+    - 2d.path.isPointInStroke
+  code: |
+    var scale = 20;
+    ctx.setLineDash([10, 21.4159]); // dash from t=0 to t=10 along the circle
+    ctx.scale(scale, scale);
+    ctx.ellipse(6, 10, 5, 5, 0, 2*Math.PI, false);
+    ctx.stroke();
+
+    // hit-test the beginning of the dash (t=0)
+    @assert ctx.isPointInStroke(11*scale, 10*scale) === true;
+    // hit-test the middle of the dash (t=5)
+    @assert ctx.isPointInStroke(8.70*scale, 14.21*scale) === true;
+    // hit-test the end of the dash (t=9.8)
+    @assert ctx.isPointInStroke(4.10*scale, 14.63*scale) === true;
+    // hit-test past the end of the dash (t=10.2)
+    @assert ctx.isPointInStroke(3.74*scale, 14.46*scale) === false;
+
+
 - name: 2d.drawImage.3arg
   testing:
     - 2d.drawImage.defaultsource

third_party/blink/web_tests/external/wpt/svg/shapes/scripted/stroke-dashes-hit-at-high-scale.svg

+<svg id="svg" xmlns="http://www.w3.org/2000/svg" xmlns:h="http://www.w3.org/1999/xhtml" viewBox="0 0 36 36" width="600" height="600">
+  <title>Strokes w/dashes are properly hit-tested, even at large scale factors</title>
+  <h:script src="/resources/testharness.js"/>
+  <h:script src="/resources/testharnessreport.js"/>
+  <metadata>
+    <h:link rel="help" href="https://svgwg.org/svg2-draft/shapes.html#CircleElement"/>
+    <h:link rel="help" href="https://svgwg.org/svg2-draft/painting.html#StrokeProperties"/>
+  </metadata>
+  <circle id="circle" cx="6" cy="10" r="5" stroke="blue" stroke-width="1" stroke-dasharray="10 21.4159" fill="none"/>
+  <script>
+  <![CDATA[
+    test(function() {
+      let svg = document.getElementById("svg");
+      let circle = document.getElementById("circle");
+      let hitTest = function(x, y) {
+        return document.elementFromPoint(
+                x * svg.width.baseVal.value / svg.viewBox.baseVal.width,
+                y * svg.height.baseVal.value / svg.viewBox.baseVal.height);
+      }
+      assert_equals(hitTest(11, 10), circle, "hit-test the beginning of the dash (t=0)");
+      assert_equals(hitTest(8.70, 14.21), circle, "hit-test the middle of the dash (t=5)");
+      assert_equals(hitTest(4.10, 14.63), circle, "hit-test the end of the dash (t=9.8)");
+      assert_equals(hitTest(3.74, 14.46), svg, "hit-test past the end of the dash (t=10.2)");
+    });
+  ]]>
+  </script>
+</svg>

third_party/blink/web_tests/external/wpt/svg/types/scripted/SVGGeometryElement.isPointInStroke-02.svg

+<svg id="svg" xmlns="http://www.w3.org/2000/svg" xmlns:h="http://www.w3.org/1999/xhtml" viewBox="0 0 36 36" width="600" height="600">
+  <title>isPointInStroke w/dashes works properly at large scale factors</title>
+  <h:script src="/resources/testharness.js"/>
+  <h:script src="/resources/testharnessreport.js"/>
+  <metadata>
+    <h:link rel="help" href="https://svgwg.org/svg2-draft/types.html#InterfaceSVGGeometryElement"/>
+    <h:link rel="help" href="https://svgwg.org/svg2-draft/types.html#__svg__SVGGeometryElement__isPointInStroke"/>
+  </metadata>
+  <circle id="circle" cx="6" cy="10" r="5" stroke="blue" stroke-width="1" stroke-dasharray="10 21.4159" fill="none"/>
+  <script>
+  <![CDATA[
+    test(function() {
+      let svg = document.getElementById("svg");
+      let circle = document.getElementById("circle");
+      let pt = svg.createSVGPoint();
+
+      pt.x = 11;
+      pt.y = 10;
+      assert_true(circle.isPointInStroke(pt), "hit-test the beginning of the dash (t=0)");
+
+      pt.x = 8.70;
+      pt.y = 14.21;
+      assert_true(circle.isPointInStroke(pt), "hit-test the middle of the dash (t=5)");
+
+      pt.x = 4.10;
+      pt.y = 14.63;
+      assert_true(circle.isPointInStroke(pt), "hit-test the end of the dash (t=9.8)");
+
+      pt.x = 3.74;
+      pt.y = 14.46;
+      assert_false(circle.isPointInStroke(pt), "hit-test past the end of the dash (t=10.2)");
+    });
+  ]]>
+  </script>
+</svg>