Skip to content

Commit d4794e9

Browse files
Ink Open Sourcecopybara-github
authored andcommitted
Fix a float-to-int cast crash in AppendArcToPolyline
This crash was found by temporarily re-enabling the stroke generation fuzz test in `strokes/stroke_test.cc`, as the crash can be triggered by an adversarial (but valid) brush definition. Fixing this crash is one step toward re-enabling that fuzz test. PiperOrigin-RevId: 826478319
1 parent 1ad07d4 commit d4794e9

File tree

2 files changed

+34
-3
lines changed

2 files changed

+34
-3
lines changed

ink/geometry/internal/circle.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,11 @@ void Circle::AppendArcToPolyline(Angle start, Angle arc_angle,
4141

4242
Angle max_step_angle = GetArcAngleForChordHeight(max_chord_height);
4343

44-
int32_t steps =
45-
std::clamp<float>(std::ceil(std::abs(arc_angle / max_step_angle)), 1,
46-
std::numeric_limits<int16_t>::max());
44+
float unclamped_steps = std::ceil(std::abs(arc_angle / max_step_angle));
45+
int32_t steps = std::isnan(unclamped_steps)
46+
? 1
47+
: std::clamp<float>(unclamped_steps, 1,
48+
std::numeric_limits<int16_t>::max());
4749
Angle step_angle = arc_angle / steps;
4850

4951
// We do not call `vector::reserve` because we expect cases with multiple

ink/geometry/internal/circle_test.cc

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "ink/geometry/internal/circle.h"
1616

1717
#include <cmath>
18+
#include <limits>
1819
#include <optional>
1920
#include <vector>
2021

@@ -34,6 +35,10 @@ using ::testing::ElementsAre;
3435
using ::testing::Eq;
3536
using ::testing::ExplainMatchResult;
3637
using ::testing::FloatNear;
38+
using ::testing::Ge;
39+
using ::testing::SizeIs;
40+
41+
constexpr float kNan = std::numeric_limits<float>::quiet_NaN();
3742

3843
// Matcher for checking that every `Point` in the `arg` array lies on the
3944
// `circle`.
@@ -333,6 +338,30 @@ TEST(CircleTest, AppendArcToPolylineKeepsPriorContents) {
333338
PointEq({5, 6}), PointEq({5, 6})));
334339
}
335340

341+
TEST(CircleTest, AppendArcToPolylineWithNanStartingAngle) {
342+
Circle circle({0, 0}, 1);
343+
std::vector<Point> polyline;
344+
circle.AppendArcToPolyline(Angle::Radians(kNan), kQuarterTurn, 0.01,
345+
polyline);
346+
// The starting angle isn't well-defined, so the start and end points are also
347+
// not well-defined.
348+
ASSERT_THAT(polyline, SizeIs(Ge(2)));
349+
EXPECT_THAT(polyline.front(), NanSensitivePointEq({kNan, kNan}));
350+
EXPECT_THAT(polyline.back(), NanSensitivePointEq({kNan, kNan}));
351+
}
352+
353+
TEST(CircleTest, AppendArcToPolylineWithNanArcAngle) {
354+
Circle circle({0, 0}, 1);
355+
std::vector<Point> polyline;
356+
circle.AppendArcToPolyline(kQuarterTurn, Angle::Radians(kNan), 0.01,
357+
polyline);
358+
// The arc angle isn't well-defined, so we shouldn't subdivide the arc into
359+
// steps. Instead we should just include the start and end point (the latter
360+
// of which is isn't well-defined).
361+
EXPECT_THAT(polyline, ElementsAre(PointNear({0, 1}, 0.001),
362+
NanSensitivePointEq({kNan, kNan})));
363+
}
364+
336365
TEST(CircleTest, GetArcAngleForChordHeightZeroRadius) {
337366
Circle circle({1, 2}, 0);
338367

0 commit comments

Comments
 (0)