Skip to content

Commit 74d55df

Browse files
Ink Open Sourcecopybara-github
authored andcommitted
Add fuzz tests for stroke input modeling
`StrokeInputModeler` should be able to model any inputs, with any valid input model, and not crash. Right now, the spring-based modeler (which we are moving away from) rejects some large-value inputs to avoid float overflow, and previously, we were CHECK-failing if that happened. This change removes that CHECK, since we shouldn't be crashing on inputs that pass `StrokeInputBatch` validation. PiperOrigin-RevId: 827562618
1 parent d9c10aa commit 74d55df

File tree

5 files changed

+49
-19
lines changed

5 files changed

+49
-19
lines changed

ink/brush/fuzz_domains.cc

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ Domain<BrushPaint::TextureKeyframe> ValidBrushPaintTextureKeyframe() {
626626
OptionalOf(FiniteAngle()), OptionalOf(InRange(0.f, 1.f)));
627627
}
628628

629-
fuzztest::Domain<BrushPaint::TextureLayer>
629+
Domain<BrushPaint::TextureLayer>
630630
ValidBrushPaintTextureLayerWithMappingAndAnimationFrames(
631631
BrushPaint::TextureMapping mapping, int animation_frames,
632632
int animation_rows, int animation_columns,
@@ -680,7 +680,7 @@ Domain<BrushPaint::SelfOverlap> ArbitraryBrushPaintSelfOverlap() {
680680
}
681681
// LINT.ThenChange(brush_paint.h:self_overlap)
682682

683-
fuzztest::Domain<BrushPaint> ValidBrushPaint(DomainVariant variant) {
683+
Domain<BrushPaint> ValidBrushPaint(DomainVariant variant) {
684684
return FlatMap(
685685
[=](BrushPaint::TextureMapping mapping,
686686
std::tuple<int, int, int> animation_frames_rows_columns,
@@ -721,11 +721,9 @@ Domain<BrushTip> ValidBrushTip(DomainVariant variant) {
721721

722722
} // namespace
723723

724-
fuzztest::Domain<Brush> ValidBrush() {
725-
return ValidBrush(DomainVariant::kValid);
726-
}
724+
Domain<Brush> ValidBrush() { return ValidBrush(DomainVariant::kValid); }
727725

728-
fuzztest::Domain<Brush> SerializableBrush() {
726+
Domain<Brush> SerializableBrush() {
729727
return ValidBrush(DomainVariant::kValidAndSerializable);
730728
}
731729

@@ -753,19 +751,21 @@ Domain<BrushCoat> SerializableBrushCoat() {
753751
return ValidBrushCoat(DomainVariant::kValidAndSerializable);
754752
}
755753

756-
fuzztest::Domain<BrushFamily> ValidBrushFamily() {
754+
Domain<float> ValidBrushEpsilon() { return FinitePositiveFloat(); }
755+
756+
Domain<BrushFamily> ValidBrushFamily() {
757757
return ValidBrushFamily(DomainVariant::kValid);
758758
}
759759

760-
fuzztest::Domain<BrushFamily> SerializableBrushFamily() {
760+
Domain<BrushFamily> SerializableBrushFamily() {
761761
return ValidBrushFamily(DomainVariant::kValidAndSerializable);
762762
}
763763

764-
fuzztest::Domain<BrushPaint> ValidBrushPaint() {
764+
Domain<BrushPaint> ValidBrushPaint() {
765765
return ValidBrushPaint(DomainVariant::kValid);
766766
}
767767

768-
fuzztest::Domain<BrushPaint> SerializableBrushPaint() {
768+
Domain<BrushPaint> SerializableBrushPaint() {
769769
return ValidBrushPaint(DomainVariant::kValidAndSerializable);
770770
}
771771

ink/brush/fuzz_domains.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ fuzztest::Domain<BrushCoat> ValidBrushCoat();
4747
// The domain of all valid brush coats that can be serialized to proto.
4848
fuzztest::Domain<BrushCoat> SerializableBrushCoat();
4949

50+
// The domain of all valid brush epsilon values.
51+
fuzztest::Domain<float> ValidBrushEpsilon();
52+
5053
// The domain of all valid brush families.
5154
fuzztest::Domain<BrushFamily> ValidBrushFamily();
5255
// The domain of all valid brush families that can be serialized to proto.

ink/strokes/internal/BUILD.bazel

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,19 @@ cc_test(
112112
deps = [
113113
":stroke_input_modeler",
114114
"//ink/brush:brush_family",
115+
"//ink/brush:fuzz_domains",
115116
"//ink/geometry:angle",
116117
"//ink/geometry:type_matchers",
118+
"//ink/strokes/input:fuzz_domains",
117119
"//ink/strokes/input:stroke_input",
118120
"//ink/strokes/input:stroke_input_batch",
119121
"//ink/types:duration",
122+
"//ink/types:fuzz_domains",
120123
"//ink/types:physical_distance",
121124
"//ink/types:type_matchers",
122125
"@com_google_absl//absl/log:absl_check",
123126
"@com_google_absl//absl/status:status_matchers",
127+
"@com_google_fuzztest//fuzztest",
124128
"@com_google_googletest//:gtest_main",
125129
],
126130
)

ink/strokes/internal/stroke_input_modeler/spring_based_input_modeler.cc

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -232,15 +232,18 @@ void SpringBasedInputModeler::ModelInput(
232232
result_buffer_.clear();
233233

234234
// `StrokeInputBatch` and `InProgressStroke` are designed to perform all the
235-
// necessary validation so that this operation should not fail.
236-
ABSL_CHECK_OK(stroke_modeler_.Update(
237-
{.event_type = event_type,
238-
.position = {input.position.x, input.position.y},
239-
.time = stroke_model::Time(input.elapsed_time.ToSeconds()),
240-
.pressure = input.pressure,
241-
.tilt = input.tilt.ValueInRadians(),
242-
.orientation = input.orientation.ValueInRadians()},
243-
result_buffer_));
235+
// necessary validation so that this operation should mostly not
236+
// fail. However, the spring-based modeler will still fail on very large input
237+
// values. If that happens, just ignore the error rather than crashing.
238+
stroke_modeler_
239+
.Update({.event_type = event_type,
240+
.position = {input.position.x, input.position.y},
241+
.time = stroke_model::Time(input.elapsed_time.ToSeconds()),
242+
.pressure = input.pressure,
243+
.tilt = input.tilt.ValueInRadians(),
244+
.orientation = input.orientation.ValueInRadians()},
245+
result_buffer_)
246+
.IgnoreError();
244247

245248
std::optional<Point> previous_position;
246249
float traveled_distance = 0;

ink/strokes/internal/stroke_input_modeler_test.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,18 @@
2121

2222
#include "gmock/gmock.h"
2323
#include "gtest/gtest.h"
24+
#include "fuzztest/fuzztest.h"
2425
#include "absl/log/absl_check.h"
2526
#include "absl/status/status_matchers.h"
2627
#include "ink/brush/brush_family.h"
28+
#include "ink/brush/fuzz_domains.h"
2729
#include "ink/geometry/angle.h"
2830
#include "ink/geometry/type_matchers.h"
31+
#include "ink/strokes/input/fuzz_domains.h"
2932
#include "ink/strokes/input/stroke_input.h"
3033
#include "ink/strokes/input/stroke_input_batch.h"
3134
#include "ink/types/duration.h"
35+
#include "ink/types/fuzz_domains.h"
3236
#include "ink/types/physical_distance.h"
3337
#include "ink/types/type_matchers.h"
3438

@@ -437,5 +441,21 @@ INSTANTIATE_TEST_SUITE_P(
437441
return info.param.test_name;
438442
});
439443

444+
void CanModelAnyStrokeInputBatch(const BrushFamily::InputModel& input_model,
445+
float brush_epsilon,
446+
const StrokeInputBatch& input_batch,
447+
Duration32 elapsed_time) {
448+
StrokeInputModeler modeler;
449+
modeler.StartStroke(input_model, brush_epsilon);
450+
modeler.ExtendStroke(input_batch, StrokeInputBatch(), elapsed_time);
451+
EXPECT_LE(modeler.GetState().stable_input_count,
452+
modeler.GetState().real_input_count);
453+
EXPECT_LE(modeler.GetState().real_input_count,
454+
modeler.GetModeledInputs().size());
455+
}
456+
FUZZ_TEST(StrokeInputModelerFuzzTest, CanModelAnyStrokeInputBatch)
457+
.WithDomains(ValidBrushFamilyInputModel(), ValidBrushEpsilon(),
458+
ArbitraryStrokeInputBatch(), FiniteNonNegativeDuration32());
459+
440460
} // namespace
441461
} // namespace ink::strokes_internal

0 commit comments

Comments
 (0)