Fix getPointAtLength() for arcs that decompose to multiple beziers

When an arc segment was decomposed into more than one cubic bezier
segment, some "residue" (the length of the first part of the cubic that
was "flat enough") would accumulate from any segments that followed the
one that contained the point. This would make the computed |offset|
incorrect.
Switch to checking, for each bezier segment, if the we've passed the
point (length) that we're querying, and skip adding anything for that
bezier segment.

Bug: 719516
Change-Id: I0c54e1a07be5fb5edddaded542df0ffd5b0a087c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1939979
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720539}

third_party/blink/renderer/core/BUILD.gn

@@ -1554,6 +1554,7 @@ jumbo_source_set("unit_tests") {
     "svg/graphics/svg_image_test.cc",
     "svg/svg_foreign_object_element_test.cc",
     "svg/svg_path_parser_test.cc",
+    "svg/svg_path_query_test.cc",
     "svg/svg_text_content_element_test.cc",
     "svg/svg_use_element_test.cc",
     "svg/unsafe_svg_attribute_sanitization_test.cc",

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

@@ -45,12 +45,7 @@ class SVGPathTraversalState final : public SVGPathConsumer {
   float TotalLength() const { return traversal_state_.total_length_; }
   FloatPoint ComputedPoint() const { return traversal_state_.current_; }
 
-  bool ProcessSegment() {
-    traversal_state_.ProcessSegment();
-    if (traversal_state_.success_)
-      return true;
-    return false;
-  }
+  bool IsDone() const { return traversal_state_.success_; }
 
  private:
   void EmitSegment(const PathSegmentData&) override;
@@ -59,6 +54,10 @@ class SVGPathTraversalState final : public SVGPathConsumer {
 };
 
 void SVGPathTraversalState::EmitSegment(const PathSegmentData& segment) {
+  // Arcs normalize to one or more cubic bezier segments, so if we've already
+  // processed enough (sub)segments we need not continue.
+  if (traversal_state_.success_)
+    return;
   switch (segment.command) {
     case kPathSegMoveToAbs:
       traversal_state_.total_length_ +=
@@ -78,6 +77,7 @@ void SVGPathTraversalState::EmitSegment(const PathSegmentData& segment) {
     default:
       NOTREACHED();
   }
+  traversal_state_.ProcessSegment();
 }
 
 void ExecuteQuery(const SVGPathByteStream& path_byte_stream,
@@ -93,7 +93,7 @@ void ExecuteQuery(const SVGPathByteStream& path_byte_stream,
     normalizer.EmitSegment(segment);
 
     has_more_data = source.HasMoreData();
-    if (traversal_state.ProcessSegment())
+    if (traversal_state.IsDone())
       break;
   }
 }

third_party/blink/renderer/core/svg/svg_path_query.h

@@ -21,14 +21,15 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_PATH_QUERY_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_SVG_SVG_PATH_QUERY_H_
 
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 
 namespace blink {
 
 class FloatPoint;
 class SVGPathByteStream;
 
-class SVGPathQuery {
+class CORE_EXPORT SVGPathQuery {
   STACK_ALLOCATED();
 
  public:

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

@@ -0,0 +1,40 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/svg/svg_path_query.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/svg/svg_path_byte_stream.h"
+#include "third_party/blink/renderer/core/svg/svg_path_utilities.h"
+#include "third_party/blink/renderer/platform/geometry/float_point.h"
+
+namespace blink {
+namespace {
+
+void PointsApproximatelyEqual(const FloatPoint& p1,
+                              const FloatPoint& p2,
+                              float epsilon) {
+  EXPECT_NEAR(p1.X(), p2.X(), epsilon);
+  EXPECT_NEAR(p1.Y(), p2.Y(), epsilon);
+}
+
+TEST(SVGPathQueryTest, PointAtLength_ArcDecomposedToMultipleCubics) {
+  SVGPathByteStream path_stream;
+  ASSERT_EQ(BuildByteStreamFromString("M56.2,66.2a174.8,174.8,0,1,0,276.0,-2.0",
+                                      path_stream),
+            SVGParseStatus::kNoError);
+
+  const float step = 7.80249691f;
+  PointsApproximatelyEqual(SVGPathQuery(path_stream).GetPointAtLength(0),
+                           FloatPoint(56.200f, 66.200f), 0.0005f);
+  PointsApproximatelyEqual(SVGPathQuery(path_stream).GetPointAtLength(step),
+                           FloatPoint(51.594f, 72.497f), 0.0005f);
+  PointsApproximatelyEqual(SVGPathQuery(path_stream).GetPointAtLength(2 * step),
+                           FloatPoint(47.270f, 78.991f), 0.0005f);
+  PointsApproximatelyEqual(SVGPathQuery(path_stream).GetPointAtLength(3 * step),
+                           FloatPoint(43.239f, 85.671f), 0.0005f);
+}
+
+}  // namespace
+}  // namespace blink

third_party/blink/renderer/core/svg/svg_path_utilities.h

@@ -34,8 +34,8 @@ bool CORE_EXPORT BuildPathFromString(const StringView&, Path&);
 bool BuildPathFromByteStream(const SVGPathByteStream&, Path&);
 
 // StringView -> SVGPathByteStream
-SVGParsingError BuildByteStreamFromString(const StringView&,
-                                          SVGPathByteStream&);
+SVGParsingError CORE_EXPORT BuildByteStreamFromString(const StringView&,
+                                                      SVGPathByteStream&);
 
 // SVGPathByteStream -> String
 enum PathSerializationFormat { kNoTransformation, kTransformToAbsolute };