Return an error when computing the angle between non-finite vectors.

As currently written, `AbsoluteAngleTo` can produce a NaN angle under a variety of circumstances; what they all have in common is that one or more components is NaN or infinite. (The converse doesn't hold; some very specific infinite inputs can actually produce non-NaN outputs. See https://en.cppreference.com/w/c/numeric/math/atan2.) This change handles them all by testing for non-finite inputs in `AbsoluteAngleTo` and making `NumberOfStepsBetweenInputs` return an error status for inputs that create non-finite intermediate states.

PiperOrigin-RevId: 603475993

7 files changed, 166 insertions, 19 deletions

diff --git a/ink_stroke_modeler/BUILD.bazel b/ink_stroke_modeler/BUILD.bazel
index 490aefa..79e5fc7 100644
--- a/ink_stroke_modeler/BUILD.bazel
+++ b/ink_stroke_modeler/BUILD.bazel
@@ -50,6 +50,7 @@ cc_library(
     deps = [
         "//ink_stroke_modeler/internal:validation",
         "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
     ],
 )
 
@@ -59,6 +60,7 @@ cc_test(
     deps = [
         ":types",
         "//ink_stroke_modeler/internal:type_matchers",
+        "@com_google_absl//absl/status",
         "@com_google_googletest//:gtest_main",
     ],
 )
diff --git a/ink_stroke_modeler/CMakeLists.txt b/ink_stroke_modeler/CMakeLists.txt
index 156ee71..c02f280 100644
--- a/ink_stroke_modeler/CMakeLists.txt
+++ b/ink_stroke_modeler/CMakeLists.txt
@@ -49,6 +49,7 @@ ink_cc_library(
   types.h
   DEPS
   absl::status
+  absl::statusor
   InkStrokeModeler::validation
 )
 
@@ -58,6 +59,7 @@ ink_cc_test(
   SRCS
   types_test.cc
   DEPS
+  absl::status
   InkStrokeModeler::types
   InkStrokeModeler::type_matchers
   GTest::gmock_main
diff --git a/ink_stroke_modeler/internal/position_modeler.cc b/ink_stroke_modeler/internal/position_modeler.cc
index c5b17d4..862a877 100644
--- a/ink_stroke_modeler/internal/position_modeler.cc
+++ b/ink_stroke_modeler/internal/position_modeler.cc
@@ -3,6 +3,7 @@
 #include <algorithm>
 #include <cmath>
 #include <limits>
+#include <sstream>
 
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
@@ -27,10 +28,19 @@ absl::StatusOr<int> NumberOfStepsBetweenInputs(
                            position_modeler_params.spring_mass_constant *
                            float_delta;
   Vec2 estimated_end_v = tip_state.velocity + estimated_delta_v;
-  float estimated_angle = tip_state.velocity.AbsoluteAngleTo(estimated_end_v);
+  absl::StatusOr<float> estimated_angle =
+      tip_state.velocity.AbsoluteAngleTo(estimated_end_v);
+  if (!estimated_angle.ok()) {
+    std::stringstream stream;
+    stream << "Non-finite or enormous inputs. tip_state.velocity="
+           << tip_state.velocity
+           << "; tip_state.position=" << tip_state.position
+           << "; end.position=" << end.position << ".";
+    return absl::InvalidArgumentError(stream.str());
+  }
   if (sampling_params.max_estimated_angle_to_traverse_per_input > 0) {
     int steps_for_angle = std::min(
-        std::ceil(estimated_angle /
+        std::ceil(*estimated_angle /
                   sampling_params.max_estimated_angle_to_traverse_per_input),
         static_cast<double>(std::numeric_limits<int>::max()));
     if (steps_for_angle > n_steps) {
@@ -39,7 +49,7 @@ absl::StatusOr<int> NumberOfStepsBetweenInputs(
   }
   if (n_steps > sampling_params.max_outputs_per_call) {
     return absl::InvalidArgumentError(absl::Substitute(
-        "Input events are too far apart, requested $0 > $1 samples.", n_steps,
+        "Input events are too far apart; requested $0 > $1 samples.", n_steps,
         sampling_params.max_outputs_per_call));
   }
   return n_steps;
diff --git a/ink_stroke_modeler/internal/position_modeler_test.cc b/ink_stroke_modeler/internal/position_modeler_test.cc
index acb4cb0..6cc1d87 100644
--- a/ink_stroke_modeler/internal/position_modeler_test.cc
+++ b/ink_stroke_modeler/internal/position_modeler_test.cc
@@ -427,6 +427,76 @@ TEST(NumberOfStepsBetweenInputsTest, OverMaxOutputsPerCall) {
   EXPECT_EQ(n_steps.status().code(), absl::StatusCode::kInvalidArgument);
 }
 
+TEST(NumberOfStepsBetweenInputsTest, NanTipPositionIsError) {
+  absl::StatusOr<int> n_steps = NumberOfStepsBetweenInputs(
+      TipState{{5, std::numeric_limits<float>::quiet_NaN()}, {1, 1}, Time{0}},
+      Input{.position = {5, 0}, .time = Time{0}},
+      Input{.position = {25, 20}, .time = Time{20}},
+      SamplingParams{.min_output_rate = 0.1, .max_outputs_per_call = 1},
+      PositionModelerParams{});
+  EXPECT_EQ(n_steps.status().code(), absl::StatusCode::kInvalidArgument);
+}
+
+TEST(NumberOfStepsBetweenInputsTest, InfiniteTipPositionIsError) {
+  absl::StatusOr<int> n_steps = NumberOfStepsBetweenInputs(
+      TipState{{5, std::numeric_limits<float>::infinity()}, {1, 1}, Time{0}},
+      Input{.position = {5, 0}, .time = Time{0}},
+      Input{.position = {25, 20}, .time = Time{20}},
+      SamplingParams{.min_output_rate = 0.1, .max_outputs_per_call = 1},
+      PositionModelerParams{});
+  EXPECT_EQ(n_steps.status().code(), absl::StatusCode::kInvalidArgument);
+}
+
+TEST(NumberOfStepsBetweenInputsTest, InfiniteTipVelocityIsError) {
+  absl::StatusOr<int> n_steps = NumberOfStepsBetweenInputs(
+      TipState{{5, 0}, {std::numeric_limits<float>::infinity(), 1}, Time{0}},
+      Input{.position = {5, 0}, .time = Time{0}},
+      Input{.position = {25, 20}, .time = Time{20}},
+      SamplingParams{.min_output_rate = 0.1, .max_outputs_per_call = 1},
+      PositionModelerParams{});
+  EXPECT_EQ(n_steps.status().code(), absl::StatusCode::kInvalidArgument);
+}
+
+TEST(NumberOfStepsBetweenInputsTest, InfiniteEndPositionIsError) {
+  absl::StatusOr<int> n_steps = NumberOfStepsBetweenInputs(
+      TipState{{5, 0}, {1, 1}, Time{0}},
+      Input{.position = {5, 0}, .time = Time{0}},
+      Input{.position = {25, std::numeric_limits<float>::infinity()},
+            .time = Time{20}},
+      SamplingParams{.min_output_rate = 0.1, .max_outputs_per_call = 1},
+      PositionModelerParams{});
+  EXPECT_EQ(n_steps.status().code(), absl::StatusCode::kInvalidArgument);
+}
+
+TEST(NumberOfStepsBetweenInputsTest,
+     HugeDifferenceBetweenTipAndEndCanOverflow) {
+  // If the difference between the tip and end positions is huge, dividing it by
+  // the spring mass constant can overflow to an infinite value, even over a
+  // very short time difference. This overflow should be handled gracefully with
+  // an InvalidArgument error.
+  absl::StatusOr<int> n_steps = NumberOfStepsBetweenInputs(
+      TipState{{3.4e38, 0}, {1, 1}, Time{0}},
+      Input{.position = {5, 0}, .time = Time{0}},
+      Input{.position = {5.0001, 0.0001}, .time = Time{0.0001}},
+      SamplingParams{.min_output_rate = 0.1, .max_outputs_per_call = 1},
+      PositionModelerParams{.spring_mass_constant = 0.5});
+  EXPECT_EQ(n_steps.status().code(), absl::StatusCode::kInvalidArgument);
+}
+
+TEST(NumberOfStepsBetweenInputsTest, CanHandleBigDifferenceBetweenTipAndEnd) {
+  // Even with a very big difference between tip and end positions, if the
+  // intermediate calculations don't overflow, we shouldn't get an error. This
+  // test case is the same as HugeDifferenceBetweenTipAndEndCanOverflow, but
+  // with the TipState x position a little smaller.
+  absl::StatusOr<int> n_steps = NumberOfStepsBetweenInputs(
+      TipState{{3.4e36, 0}, {1, 1}, Time{0}},
+      Input{.position = {5, 0}, .time = Time{0}},
+      Input{.position = {5.0001, 0.0001}, .time = Time{0.0001}},
+      SamplingParams{.min_output_rate = 0.1, .max_outputs_per_call = 1},
+      PositionModelerParams{.spring_mass_constant = 0.5});
+  EXPECT_EQ(n_steps.status().code(), absl::StatusCode::kOk);
+}
+
 TEST(NumberOfStepsBetweenInputsTest, UpsampleDueToSharpTurn) {
   absl::StatusOr<int> n_steps = NumberOfStepsBetweenInputs(
       TipState{{0, 0}, {0, 1}, Time{0}},
diff --git a/ink_stroke_modeler/types.cc b/ink_stroke_modeler/types.cc
index 5920f2f..ee46226 100644
--- a/ink_stroke_modeler/types.cc
+++ b/ink_stroke_modeler/types.cc
@@ -1,8 +1,10 @@
 #include "ink_stroke_modeler/types.h"
 
 #include <cmath>
+#include <sstream>
 
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "ink_stroke_modeler/internal/validation.h"
 
 // This convenience macro evaluates the given expression, and if it does not
@@ -15,7 +17,12 @@
 namespace ink {
 namespace stroke_model {
 
-float Vec2::AbsoluteAngleTo(Vec2 other) const {
+absl::StatusOr<float> Vec2::AbsoluteAngleTo(Vec2 other) const {
+  if (!IsFinite() || !other.IsFinite()) {
+    std::stringstream stream;
+    stream << "Non-finite inputs: this=" << *this << "; other=" << other;
+    return absl::InvalidArgumentError(stream.str());
+  }
   float dot = x * other.x + y * other.y;
   float det = x * other.y - y * other.x;
   return std::abs(std::atan2(det, dot));
diff --git a/ink_stroke_modeler/types.h b/ink_stroke_modeler/types.h
index 3646147..1ed42b1 100644
--- a/ink_stroke_modeler/types.h
+++ b/ink_stroke_modeler/types.h
@@ -21,6 +21,7 @@
 #include <ostream>
 
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 
 namespace ink {
 namespace stroke_model {
@@ -34,8 +35,10 @@ struct Vec2 {
   float Magnitude() const { return std::hypot(x, y); }
 
   // The difference in angle between the vector and another vector in radians
-  // [0, pi]
-  float AbsoluteAngleTo(Vec2 other) const;
+  // [0, pi]. Returns an error status if either of the inputs is non-finite.
+  absl::StatusOr<float> AbsoluteAngleTo(Vec2 other) const;
+
+  bool IsFinite() const { return std::isfinite(x) && std::isfinite(y); }
 };
 
 bool operator==(Vec2 lhs, Vec2 rhs);
diff --git a/ink_stroke_modeler/types_test.cc b/ink_stroke_modeler/types_test.cc
index acf3646..8130dde 100644
--- a/ink_stroke_modeler/types_test.cc
+++ b/ink_stroke_modeler/types_test.cc
@@ -17,10 +17,10 @@
 #include <cmath>
 #include <limits>
 #include <sstream>
-#include <string>
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "absl/status/status.h"
 #include "ink_stroke_modeler/internal/type_matchers.h"
 
 namespace ink {
@@ -141,24 +141,77 @@ TEST(TypesTest, Vec2Stream) {
 }
 
 TEST(TypesTest, Vec2AbsoluteAngleTo) {
-  EXPECT_THAT((Vec2{0, 1}.AbsoluteAngleTo(Vec2{0, 1})), FloatEq(0));
-  EXPECT_THAT((Vec2{0, 1}.AbsoluteAngleTo(Vec2{-1, 0})), FloatEq(M_PI / 2));
-  EXPECT_THAT((Vec2{0, 1}.AbsoluteAngleTo(Vec2{1, 0})), FloatEq(M_PI / 2));
-  EXPECT_THAT((Vec2{0, 1}.AbsoluteAngleTo(Vec2{0, -1})), FloatEq(M_PI));
+  auto angle = Vec2{0, 1}.AbsoluteAngleTo(Vec2{0, 1});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+
+  angle = Vec2{0, 1}.AbsoluteAngleTo(Vec2{-1, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(M_PI / 2));
+
+  angle = Vec2{0, 1}.AbsoluteAngleTo(Vec2{1, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(M_PI / 2));
+
+  angle = Vec2{0, 1}.AbsoluteAngleTo(Vec2{0, -1});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(M_PI));
 }
 
 TEST(TypesTest, Vec2AbsoluteAngleFromZeroVecIsZero) {
-  EXPECT_THAT((Vec2{0, 0}.AbsoluteAngleTo(Vec2{0, -1})), FloatEq(0));
-  EXPECT_THAT((Vec2{0, 0}.AbsoluteAngleTo(Vec2{0, -1})), FloatEq(0));
-  EXPECT_THAT((Vec2{0, 0}.AbsoluteAngleTo(Vec2{1, 0})), FloatEq(0));
-  EXPECT_THAT((Vec2{0, 0}.AbsoluteAngleTo(Vec2{-1, 0})), FloatEq(0));
+  auto angle = Vec2{0, 0}.AbsoluteAngleTo(Vec2{0, -1});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+
+  angle = Vec2{0, 0}.AbsoluteAngleTo(Vec2{0, -1});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+
+  angle = Vec2{0, 0}.AbsoluteAngleTo(Vec2{1, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+
+  angle = Vec2{0, 0}.AbsoluteAngleTo(Vec2{-1, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
 }
 
 TEST(TypesTest, Vec2AbsoluteAngleToZeroVecIsZero) {
-  EXPECT_THAT((Vec2{0, -1}.AbsoluteAngleTo(Vec2{0, 0})), FloatEq(0));
-  EXPECT_THAT((Vec2{0, 1}.AbsoluteAngleTo(Vec2{0, 0})), FloatEq(0));
-  EXPECT_THAT((Vec2{-1, 0}.AbsoluteAngleTo(Vec2{0, 0})), FloatEq(0));
-  EXPECT_THAT((Vec2{1, 0}.AbsoluteAngleTo(Vec2{0, 0})), FloatEq(0));
+  auto angle = Vec2{0, -1}.AbsoluteAngleTo(Vec2{0, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+
+  angle = Vec2{0, 1}.AbsoluteAngleTo(Vec2{0, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+
+  angle = Vec2{-1, 0}.AbsoluteAngleTo(Vec2{0, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+
+  angle = Vec2{1, 0}.AbsoluteAngleTo(Vec2{0, 0});
+  ASSERT_TRUE(angle.ok());
+  EXPECT_THAT(*angle, FloatEq(0));
+}
+
+TEST(TypesTest, Vec2AbsoluteAngleFromNonFiniteVectorIsError) {
+  auto angle = Vec2{1, std::numeric_limits<float>::infinity()}.AbsoluteAngleTo(
+      Vec2{1, 1});
+  EXPECT_EQ(angle.status().code(), absl::StatusCode::kInvalidArgument);
+
+  angle = Vec2{1, std::numeric_limits<float>::quiet_NaN()}.AbsoluteAngleTo(
+      Vec2{1, 1});
+  EXPECT_EQ(angle.status().code(), absl::StatusCode::kInvalidArgument);
+}
+
+TEST(TypesTest, Vec2AbsoluteAngleToNonFiniteVectorIsError) {
+  auto angle = Vec2{1, 1}.AbsoluteAngleTo(
+      Vec2{1, std::numeric_limits<float>::infinity()});
+  EXPECT_EQ(angle.status().code(), absl::StatusCode::kInvalidArgument);
+
+  angle = Vec2{1, 1}.AbsoluteAngleTo(
+      Vec2{1, std::numeric_limits<float>::quiet_NaN()});
+  EXPECT_EQ(angle.status().code(), absl::StatusCode::kInvalidArgument);
 }
 
 TEST(TypesTest, DurationArithmetic) {