Use return-value for result from Path::PointAndNormalAtLength

Return the result of Path::PointAndNormalAtLength rather than using two
out arguments.

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

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

@@ -36,8 +36,7 @@ PathPositionMapper::PathPositionMapper(const Path& path,
 
 PathPositionMapper::PositionType PathPositionMapper::PointAndNormalAtLength(
     float length,
-    FloatPoint& point,
-    float& angle) {
+    PointAndTangent& point_and_tangent) {
   if (length < 0)
     return kBeforePath;
   if (length > path_length_)
@@ -45,7 +44,7 @@ PathPositionMapper::PositionType PathPositionMapper::PointAndNormalAtLength(
   DCHECK_GE(length, 0);
   DCHECK_LE(length, path_length_);
 
-  position_calculator_.PointAndNormalAtLength(length, point, angle);
+  point_and_tangent = position_calculator_.PointAndNormalAtLength(length);
   return kOnPath;
 }
 

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

@@ -28,6 +28,8 @@
 
 namespace blink {
 
+struct PointAndTangent;
+
 // This class maps a 1D location in the "path space"; [0, path length] to a
 // (2D) point on the path and provides the normal (angle from the x-axis) for
 // said point.
@@ -44,7 +46,7 @@ class PathPositionMapper {
     kBeforePath,
     kAfterPath,
   };
-  PositionType PointAndNormalAtLength(float length, FloatPoint&, float& angle);
+  PositionType PointAndNormalAtLength(float length, PointAndTangent&);
   float length() const { return path_length_; }
   float StartOffset() const { return path_start_offset_; }
 

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

@@ -441,8 +441,7 @@ void SVGTextLayoutEngine::LayoutTextOnLineOrPath(
     float spacing = spacing_layout.CalculateCSSSpacing(current_character);
 
     FloatPoint text_path_shift;
-    float angle = 0;
-    FloatPoint position;
+    PointAndTangent position;
     if (in_path_layout_) {
       float scaled_glyph_advance = glyph_advance * text_path_scaling_;
       // Setup translations that move to the glyph midpoint.
@@ -460,7 +459,7 @@ void SVGTextLayoutEngine::LayoutTextOnLineOrPath(
                                    spacing * text_path_scaling_;
 
       PathPositionMapper::PositionType position_type =
-          text_path_->PointAndNormalAtLength(text_path_offset, position, angle);
+          text_path_->PointAndNormalAtLength(text_path_offset, position);
 
       // Skip character, if we're before the path.
       if (position_type == PathPositionMapper::kBeforePath) {
@@ -473,24 +472,26 @@ void SVGTextLayoutEngine::LayoutTextOnLineOrPath(
       if (position_type == PathPositionMapper::kAfterPath)
         break;
 
-      text_position_ = position;
+      text_position_ = position.point;
 
       // For vertical text on path, the actual angle has to be rotated 90
       // degrees anti-clockwise, not the orientation angle!
       if (is_vertical_text_)
-        angle -= 90;
+        position.tangent_in_degrees -= 90;
     } else {
-      position = text_position_;
-      position += baseline_shift;
+      position.point = text_position_;
+      position.point += baseline_shift;
     }
 
     if (data.HasRotate())
-      angle += data.rotate;
+      position.tangent_in_degrees += data.rotate;
 
     // Determine whether we have to start a new fragment.
     bool should_start_new_fragment =
-        needs_fragment_per_glyph || has_relative_position || angle ||
-        angle != last_angle || apply_spacing_to_next_character;
+        needs_fragment_per_glyph || has_relative_position ||
+        position.tangent_in_degrees ||
+        position.tangent_in_degrees != last_angle ||
+        apply_spacing_to_next_character;
 
     // If we already started a fragment, close it now.
     if (did_start_text_fragment && should_start_new_fragment) {
@@ -508,16 +509,17 @@ void SVGTextLayoutEngine::LayoutTextOnLineOrPath(
           visual_metrics_iterator_.CharacterOffset();
       current_text_fragment_.metrics_list_offset =
           visual_metrics_iterator_.MetricsListOffset();
-      current_text_fragment_.x = position.X();
-      current_text_fragment_.y = position.Y();
+      current_text_fragment_.x = position.point.X();
+      current_text_fragment_.y = position.point.Y();
 
       // Build fragment transformation.
-      if (angle)
-        current_text_fragment_.transform.Rotate(angle);
+      if (position.tangent_in_degrees)
+        current_text_fragment_.transform.Rotate(position.tangent_in_degrees);
 
-      if (text_path_shift.X() || text_path_shift.Y())
+      if (text_path_shift.X() || text_path_shift.Y()) {
         current_text_fragment_.transform.Translate(text_path_shift.X(),
                                                    text_path_shift.Y());
+      }
 
       // For vertical text, always rotate by 90 degrees regardless of
       // fontOrientation.
@@ -543,7 +545,7 @@ void SVGTextLayoutEngine::LayoutTextOnLineOrPath(
 
     AdvanceToNextLogicalCharacter(logical_metrics);
     visual_metrics_iterator_.Next();
-    last_angle = angle;
+    last_angle = position.tangent_in_degrees;
   }
 
   if (!did_start_text_fragment)

third_party/blink/renderer/core/style/computed_style.cc

@@ -1265,15 +1265,16 @@ void ComputedStyle::ApplyMotionPathTransform(
   const BasicShape* path = OffsetPath();
   const StyleOffsetRotation& rotate = OffsetRotate();
 
-  FloatPoint point;
-  float angle;
+  PointAndTangent path_position;
   if (path->GetType() == BasicShape::kStyleRayType) {
     // TODO(ericwilligers): crbug.com/641245 Support <size> for ray paths.
     float float_distance = FloatValueForLength(distance, 0);
 
-    angle = To<StyleRay>(*path).Angle() - 90;
-    point.SetX(float_distance * cos(deg2rad(angle)));
-    point.SetY(float_distance * sin(deg2rad(angle)));
+    path_position.tangent_in_degrees = To<StyleRay>(*path).Angle() - 90;
+    path_position.point.SetX(float_distance *
+                             cos(deg2rad(path_position.tangent_in_degrees)));
+    path_position.point.SetY(float_distance *
+                             sin(deg2rad(path_position.tangent_in_degrees)));
   } else {
     float zoom = EffectiveZoom();
     const StylePath& motion_path = To<StylePath>(*path);
@@ -1289,14 +1290,13 @@ void ComputedStyle::ApplyMotionPathTransform(
       computed_distance = clampTo<float>(float_distance, 0, path_length);
     }
 
-    motion_path.GetPath().PointAndNormalAtLength(computed_distance, point,
-                                                 angle);
-
-    point.Scale(zoom, zoom);
+    path_position =
+        motion_path.GetPath().PointAndNormalAtLength(computed_distance);
+    path_position.point.Scale(zoom, zoom);
   }
 
   if (rotate.type == OffsetRotationType::kFixed)
-    angle = 0;
+    path_position.tangent_in_degrees = 0;
 
   float origin_shift_x = 0;
   float origin_shift_y = 0;
@@ -1312,9 +1312,10 @@ void ComputedStyle::ApplyMotionPathTransform(
     origin_shift_y = anchor_point.Y() - origin_y;
   }
 
-  transform.Translate(point.X() - anchor_point.X() + origin_shift_x,
-                      point.Y() - anchor_point.Y() + origin_shift_y);
-  transform.Rotate(angle + rotate.angle);
+  transform.Translate(
+      path_position.point.X() - anchor_point.X() + origin_shift_x,
+      path_position.point.Y() - anchor_point.Y() + origin_shift_y);
+  transform.Rotate(path_position.tangent_in_degrees + rotate.angle);
 
   if (!position.X().IsAuto() || !anchor.X().IsAuto())
     // Shift the origin back to transform-origin.

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

@@ -219,26 +219,25 @@ void SVGAnimateMotionElement::CalculateAnimationValue(
 
   DCHECK(!animation_path_.IsEmpty());
 
-  float position_on_path = animation_path_.length() * percentage;
-  FloatPoint position;
-  float angle;
-  animation_path_.PointAndNormalAtLength(position_on_path, position, angle);
+  const float path_length = animation_path_.length();
+  float position_on_path = path_length * percentage;
+  PointAndTangent position =
+      animation_path_.PointAndNormalAtLength(position_on_path);
 
   // Handle accumulate="sum".
   if (repeat_count && parameters.is_cumulative) {
     FloatPoint position_at_end_of_duration =
-        animation_path_.PointAtLength(animation_path_.length());
-    position.Move(position_at_end_of_duration.X() * repeat_count,
-                  position_at_end_of_duration.Y() * repeat_count);
+        animation_path_.PointAtLength(path_length);
+    position.point.MoveBy(position_at_end_of_duration.ScaledBy(repeat_count));
   }
 
-  transform->Translate(position.X(), position.Y());
+  transform->Translate(position.point.X(), position.point.Y());
   RotateMode rotate_mode = GetRotateMode();
   if (rotate_mode != kRotateAuto && rotate_mode != kRotateAutoReverse)
     return;
   if (rotate_mode == kRotateAutoReverse)
-    angle += 180;
-  transform->Rotate(angle);
+    position.tangent_in_degrees += 180;
+  transform->Rotate(position.tangent_in_degrees);
 }
 
 void SVGAnimateMotionElement::ApplyResultsToTarget(

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

@@ -201,17 +201,13 @@ float Path::length() const {
 }
 
 FloatPoint Path::PointAtLength(float length) const {
-  FloatPoint point;
-  float normal;
-  PointAndNormalAtLength(length, point, normal);
-  return point;
+  return PointAndNormalAtLength(length).point;
 }
 
-static bool CalculatePointAndNormalOnPath(SkPathMeasure& measure,
-                                          SkScalar& contour_start,
-                                          SkScalar length,
-                                          FloatPoint& point,
-                                          float& normal_angle) {
+static base::Optional<PointAndTangent> CalculatePointAndNormalOnPath(
+    SkPathMeasure& measure,
+    SkScalar& contour_start,
+    SkScalar length) {
   do {
     SkScalar contour_end = contour_start + measure.getLength();
     if (length <= contour_end) {
@@ -220,31 +216,25 @@ static bool CalculatePointAndNormalOnPath(SkPathMeasure& measure,
 
       SkScalar pos_in_contour = length - contour_start;
       if (measure.getPosTan(pos_in_contour, &position, &tangent)) {
-        normal_angle =
+        PointAndTangent result;
+        result.point = FloatPoint(position);
+        result.tangent_in_degrees =
             rad2deg(SkScalarToFloat(SkScalarATan2(tangent.fY, tangent.fX)));
-        point = FloatPoint(SkScalarToFloat(position.fX),
-                           SkScalarToFloat(position.fY));
-        return true;
+        return result;
       }
     }
     contour_start = contour_end;
   } while (measure.nextContour());
-  return false;
+  return base::nullopt;
 }
 
-void Path::PointAndNormalAtLength(float length,
-                                  FloatPoint& point,
-                                  float& normal) const {
+PointAndTangent Path::PointAndNormalAtLength(float length) const {
   SkPathMeasure measure(path_, false);
   SkScalar start = 0;
-  if (CalculatePointAndNormalOnPath(
-          measure, start, WebCoreFloatToSkScalar(length), point, normal))
-    return;
-
-  SkPoint position = path_.getPoint(0);
-  point =
-      FloatPoint(SkScalarToFloat(position.fX), SkScalarToFloat(position.fY));
-  normal = 0;
+  if (base::Optional<PointAndTangent> result = CalculatePointAndNormalOnPath(
+          measure, start, WebCoreFloatToSkScalar(length)))
+    return *result;
+  return {FloatPoint(path_.getPoint(0)), 0};
 }
 
 Path::PositionCalculator::PositionCalculator(const Path& path)
@@ -252,9 +242,7 @@ Path::PositionCalculator::PositionCalculator(const Path& path)
       path_measure_(path.GetSkPath(), false),
       accumulated_length_(0) {}
 
-void Path::PositionCalculator::PointAndNormalAtLength(float length,
-                                                      FloatPoint& point,
-                                                      float& normal_angle) {
+PointAndTangent Path::PositionCalculator::PointAndNormalAtLength(float length) {
   SkScalar sk_length = WebCoreFloatToSkScalar(length);
   if (sk_length >= 0) {
     if (sk_length < accumulated_length_) {
@@ -263,15 +251,12 @@ void Path::PositionCalculator::PointAndNormalAtLength(float length,
       accumulated_length_ = 0;
     }
 
-    if (CalculatePointAndNormalOnPath(path_measure_, accumulated_length_,
-                                      sk_length, point, normal_angle))
-      return;
+    base::Optional<PointAndTangent> result = CalculatePointAndNormalOnPath(
+        path_measure_, accumulated_length_, sk_length);
+    if (result)
+      return *result;
   }
-
-  SkPoint position = path_.getPoint(0);
-  point =
-      FloatPoint(SkScalarToFloat(position.fX), SkScalarToFloat(position.fY));
-  normal_angle = 0;
+  return {FloatPoint(path_.getPoint(0)), 0};
 }
 
 void Path::Clear() {

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

@@ -65,6 +65,12 @@ struct PathElement {
   FloatPoint* points;
 };
 
+// Result structure from Path::PointAndNormalAtLength() (and similar).
+struct PointAndTangent {
+  FloatPoint point;
+  float tangent_in_degrees = 0;
+};
+
 typedef void (*PathApplierFunction)(void* info, const PathElement*);
 
 class PLATFORM_EXPORT Path {
@@ -97,7 +103,7 @@ class PLATFORM_EXPORT Path {
 
   float length() const;
   FloatPoint PointAtLength(float length) const;
-  void PointAndNormalAtLength(float length, FloatPoint&, float&) const;
+  PointAndTangent PointAndNormalAtLength(float length) const;
 
   // Helper for computing a sequence of positions and normals (normal angles) on
   // a path. The best possible access pattern will be one where the |length|
@@ -111,7 +117,7 @@ class PLATFORM_EXPORT Path {
    public:
     explicit PositionCalculator(const Path&);
 
-    void PointAndNormalAtLength(float length, FloatPoint&, float&);
+    PointAndTangent PointAndNormalAtLength(float length);
 
    private:
     SkPath path_;

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

@@ -17,10 +17,9 @@ TEST(PathTest, PointAtEndOfPath) {
   path.AddBezierCurveTo(FloatPoint(324, 196), FloatPoint(472, 370),
                         FloatPoint(460, 470));
 
-  FloatPoint point;
-  float angle;
-  path.PointAndNormalAtLength(path.length(), point, angle);
-  EXPECT_EQ(point, FloatPoint(460, 470));
+  PointAndTangent point_and_tangent =
+      path.PointAndNormalAtLength(path.length());
+  EXPECT_EQ(point_and_tangent.point, FloatPoint(460, 470));
 }
 
 }  // namespace blink