Skip to content

Commit 8e1d422

Browse files
Ink Open Sourcecopybara-github
Ink Open Source
authored and
copybara-github
committed
Refactor StrokeInput batch validation in terms of helper functions
Resolving the validation allows for better handling of invalid inputs. PiperOrigin-RevId: 826099337
1 parent 9e97e6d commit 8e1d422

File tree

3 files changed

+161
-91
lines changed

3 files changed

+161
-91
lines changed

‎ink/strokes/input/internal/stroke_input_validation_helpers.cc‎

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

2222
namespace ink::stroke_input_internal {
2323

24-
absl::Status ValidateConsecutiveInputs(const StrokeInput& first,
25-
const StrokeInput& second) {
26-
if (first.tool_type != second.tool_type) {
27-
return absl::InvalidArgumentError(absl::Substitute(
28-
"All inputs must report the same value of `tool_type`. Got $0 and $1",
29-
static_cast<int>(first.tool_type), static_cast<int>(second.tool_type)));
30-
}
31-
24+
absl::Status ValidateAdvancingXYT(const StrokeInput& first,
25+
const StrokeInput& second) {
3226
if (first.position == second.position &&
3327
first.elapsed_time == second.elapsed_time) {
3428
return absl::InvalidArgumentError(
@@ -43,6 +37,16 @@ absl::Status ValidateConsecutiveInputs(const StrokeInput& first,
4337
"$0, to be followed by: $1",
4438
first.elapsed_time.ToSeconds(), second.elapsed_time.ToSeconds()));
4539
}
40+
return absl::OkStatus();
41+
}
42+
43+
absl::Status ValidateConsistentAttributes(const StrokeInput& first,
44+
const StrokeInput& second) {
45+
if (first.tool_type != second.tool_type) {
46+
return absl::InvalidArgumentError(absl::Substitute(
47+
"All inputs must report the same value of `tool_type`. Got $0 and $1",
48+
static_cast<int>(first.tool_type), static_cast<int>(second.tool_type)));
49+
}
4650

4751
if (first.stroke_unit_length != second.stroke_unit_length) {
4852
return absl::InvalidArgumentError(
@@ -70,4 +74,13 @@ absl::Status ValidateConsecutiveInputs(const StrokeInput& first,
7074
return absl::OkStatus();
7175
}
7276

77+
absl::Status ValidateConsecutiveInputs(const StrokeInput& first,
78+
const StrokeInput& second) {
79+
if (absl::Status status = ValidateAdvancingXYT(first, second); !status.ok()) {
80+
return status;
81+
}
82+
83+
return ValidateConsistentAttributes(first, second);
84+
}
85+
7386
} // namespace ink::stroke_input_internal

‎ink/strokes/input/internal/stroke_input_validation_helpers.h‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,22 @@
2020

2121
namespace ink::stroke_input_internal {
2222

23+
// Validates that an ordered pair of inputs have non-decreasing
24+
// `elapsed_time` and do not have duplicate `x`, `y`, `elapsed_time` values.
25+
absl::Status ValidateAdvancingXYT(const StrokeInput& first,
26+
const StrokeInput& second);
27+
28+
// Validates that a pair of inputs have the same tool type and the same
29+
// format of reported `stroke_unit_length`, `pressure`, `tilt`, and
30+
// `orientation`.
31+
absl::Status ValidateConsistentAttributes(const StrokeInput& first,
32+
const StrokeInput& second);
33+
2334
// Validates that a pair of inputs can be consecutive in a `StrokeInputBatch`.
2435
//
2536
// This includes checking that `first` and `second`:
26-
// * Have the same `tool_type`
27-
// * Do not have duplicate `x`, `y`, `elapsed_time`
28-
// * Have non-decreasing `elapsed_time`
29-
// * Have the same format of reported optional `pressure`, `tilt`, and
30-
// `orientation`
37+
// * Have advancing `x`, `y`, and `elapsed_time`.
38+
// * Have consistent tool type and attribute format.
3139
absl::Status ValidateConsecutiveInputs(const StrokeInput& first,
3240
const StrokeInput& second);
3341

‎ink/strokes/input/internal/stroke_input_validation_helpers_test.cc‎

Lines changed: 127 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -27,101 +27,101 @@ namespace {
2727

2828
using ::testing::HasSubstr;
2929

30-
TEST(ValidateConsecutiveInputsTest, ValidInputsWithNoOptionalProperties) {
30+
TEST(ValidateConsistentAttributesTest, ValidInputsWithNoOptionalProperties) {
3131
EXPECT_EQ(absl::OkStatus(),
32-
ValidateConsecutiveInputs(
32+
ValidateConsistentAttributes(
3333
{.position = {1, 2}, .elapsed_time = Duration32::Millis(5)},
3434
{.position = {2, 3}, .elapsed_time = Duration32::Millis(10)}));
3535
}
3636

37-
TEST(ValidateConsecutiveInputsTest, ValidInputsWithAllOptionalProperties) {
38-
EXPECT_EQ(absl::OkStatus(),
39-
ValidateConsecutiveInputs({.position = {1, 2},
40-
.elapsed_time = Duration32::Millis(5),
41-
.pressure = 0.5,
42-
.tilt = kFullTurn / 8,
43-
.orientation = kHalfTurn},
44-
{.position = {2, 3},
45-
.elapsed_time = Duration32::Millis(10),
46-
.pressure = 0.6,
47-
.tilt = kQuarterTurn,
48-
.orientation = kFullTurn}));
49-
}
50-
51-
TEST(ValidateConsecutiveInputsTest, ValidInputsWithOnlyPressure) {
52-
EXPECT_EQ(absl::OkStatus(),
53-
ValidateConsecutiveInputs({.position = {1, 2},
54-
.elapsed_time = Duration32::Millis(5),
55-
.pressure = 0.5},
56-
{.position = {2, 3},
57-
.elapsed_time = Duration32::Millis(10),
58-
.pressure = 0.6}));
37+
TEST(ValidateConsistentAttributesTest, ValidInputsWithAllOptionalProperties) {
38+
EXPECT_EQ(absl::OkStatus(), ValidateConsistentAttributes(
39+
{.position = {1, 2},
40+
.elapsed_time = Duration32::Millis(5),
41+
.pressure = 0.5,
42+
.tilt = kFullTurn / 8,
43+
.orientation = kHalfTurn},
44+
{.position = {2, 3},
45+
.elapsed_time = Duration32::Millis(10),
46+
.pressure = 0.6,
47+
.tilt = kQuarterTurn,
48+
.orientation = kFullTurn}));
5949
}
6050

61-
TEST(ValidateConsecutiveInputsTest, ValidInputsWithOnlyTilt) {
62-
EXPECT_EQ(absl::OkStatus(),
63-
ValidateConsecutiveInputs({.position = {1, 2},
64-
.elapsed_time = Duration32::Millis(5),
65-
.tilt = kFullTurn / 8},
66-
{.position = {2, 3},
67-
.elapsed_time = Duration32::Millis(10),
68-
.tilt = kQuarterTurn}));
51+
TEST(ValidateConsistentAttributesTest, ValidInputsWithOnlyPressure) {
52+
EXPECT_EQ(absl::OkStatus(), ValidateConsistentAttributes(
53+
{.position = {1, 2},
54+
.elapsed_time = Duration32::Millis(5),
55+
.pressure = 0.5},
56+
{.position = {2, 3},
57+
.elapsed_time = Duration32::Millis(10),
58+
.pressure = 0.6}));
6959
}
7060

71-
TEST(ValidateConsecutiveInputsTest, ValidInputsWithOnlyOrientation) {
72-
EXPECT_EQ(absl::OkStatus(),
73-
ValidateConsecutiveInputs({.position = {1, 2},
74-
.elapsed_time = Duration32::Millis(5),
75-
.orientation = kHalfTurn},
76-
{.position = {2, 3},
77-
.elapsed_time = Duration32::Millis(10),
78-
.orientation = kFullTurn}));
61+
TEST(ValidateConsistentAttributesTest, ValidInputsWithOnlyTilt) {
62+
EXPECT_EQ(absl::OkStatus(), ValidateConsistentAttributes(
63+
{.position = {1, 2},
64+
.elapsed_time = Duration32::Millis(5),
65+
.tilt = kFullTurn / 8},
66+
{.position = {2, 3},
67+
.elapsed_time = Duration32::Millis(10),
68+
.tilt = kQuarterTurn}));
69+
}
70+
71+
TEST(ValidateConsistentAttributesTest, ValidInputsWithOnlyOrientation) {
72+
EXPECT_EQ(absl::OkStatus(), ValidateConsistentAttributes(
73+
{.position = {1, 2},
74+
.elapsed_time = Duration32::Millis(5),
75+
.orientation = kHalfTurn},
76+
{.position = {2, 3},
77+
.elapsed_time = Duration32::Millis(10),
78+
.orientation = kFullTurn}));
7979
}
8080

81-
TEST(ValidateConsecutiveInputsTest, ValidInputsDuplicatePosition) {
81+
TEST(ValidateAdvancingXytTest, ValidInputsDuplicatePosition) {
8282
EXPECT_EQ(absl::OkStatus(),
83-
ValidateConsecutiveInputs(
83+
ValidateAdvancingXYT(
8484
{.position = {1, 2}, .elapsed_time = Duration32::Millis(5)},
8585
{.position = {1, 2}, .elapsed_time = Duration32::Millis(10)}));
8686
}
8787

88-
TEST(ValidateConsecutiveInputsTest, ValidInputsDuplicateElapsedTime) {
88+
TEST(ValidateAdvancingXytTest, ValidInputsDuplicateElapsedTime) {
8989
EXPECT_EQ(absl::OkStatus(),
90-
ValidateConsecutiveInputs(
90+
ValidateAdvancingXYT(
9191
{.position = {1, 2}, .elapsed_time = Duration32::Millis(5)},
9292
{.position = {2, 3}, .elapsed_time = Duration32::Millis(5)}));
9393
}
9494

95-
TEST(ValidateConsecutiveInputsTest, MismatchedToolTypes) {
95+
TEST(ValidateConsistentAttributesTest, MismatchedToolTypes) {
9696
absl::Status mismatched_tool_type =
97-
ValidateConsecutiveInputs({.tool_type = StrokeInput::ToolType::kMouse,
98-
.position = {1, 2},
99-
.elapsed_time = Duration32::Millis(5)},
100-
{.tool_type = StrokeInput::ToolType::kStylus,
101-
.position = {2, 3},
102-
.elapsed_time = Duration32::Millis(10)});
97+
ValidateConsistentAttributes({.tool_type = StrokeInput::ToolType::kMouse,
98+
.position = {1, 2},
99+
.elapsed_time = Duration32::Millis(5)},
100+
{.tool_type = StrokeInput::ToolType::kStylus,
101+
.position = {2, 3},
102+
.elapsed_time = Duration32::Millis(10)});
103103
EXPECT_EQ(mismatched_tool_type.code(), absl::StatusCode::kInvalidArgument);
104104
EXPECT_THAT(mismatched_tool_type.message(), HasSubstr("tool_type"));
105105
}
106106

107-
TEST(ValidateConsecutiveInputsTest, DuplicatePositionAndElapsedTime) {
108-
absl::Status duplicate_input = ValidateConsecutiveInputs(
107+
TEST(ValidateAdvancingXytTest, DuplicatePositionAndElapsedTime) {
108+
absl::Status duplicate_input = ValidateAdvancingXYT(
109109
{.position = {1, 2}, .elapsed_time = Duration32::Millis(5)},
110110
{.position = {1, 2}, .elapsed_time = Duration32::Millis(5)});
111111
EXPECT_EQ(duplicate_input.code(), absl::StatusCode::kInvalidArgument);
112112
EXPECT_THAT(duplicate_input.message(), HasSubstr("duplicate"));
113113
}
114114

115-
TEST(ValidateConsecutiveInputsTest, DecreasingElapsedTime) {
116-
absl::Status decreasing_time = ValidateConsecutiveInputs(
115+
TEST(ValidateAdvancingXytTest, DecreasingElapsedTime) {
116+
absl::Status decreasing_time = ValidateAdvancingXYT(
117117
{.position = {1, 2}, .elapsed_time = Duration32::Seconds(1)},
118118
{.position = {2, 3}, .elapsed_time = Duration32::Seconds(0.99)});
119119
EXPECT_EQ(decreasing_time.code(), absl::StatusCode::kInvalidArgument);
120120
EXPECT_THAT(decreasing_time.message(), HasSubstr("non-decreasing"));
121121
}
122122

123-
TEST(ValidateConsecutiveInputsTest, MismatchedStrokeUnitLength) {
124-
absl::Status mismatched_tool_type = ValidateConsecutiveInputs(
123+
TEST(ValidateConsistentAttributesTest, MismatchedStrokeUnitLength) {
124+
absl::Status mismatched_tool_type = ValidateConsistentAttributes(
125125
{.position = {1, 2},
126126
.elapsed_time = Duration32::Millis(5),
127127
.stroke_unit_length = PhysicalDistance::Centimeters(1)},
@@ -132,42 +132,91 @@ TEST(ValidateConsecutiveInputsTest, MismatchedStrokeUnitLength) {
132132
EXPECT_THAT(mismatched_tool_type.message(), HasSubstr("stroke_unit_length"));
133133
}
134134

135-
TEST(ValidateConsecutiveInputsTest, MismatchedOptionalPressure) {
135+
TEST(ValidateConsistentAttributesTest, MismatchedOptionalPressure) {
136136
absl::Status mismatched_has_pressure =
137-
ValidateConsecutiveInputs({.position = {1, 2},
138-
.elapsed_time = Duration32::Millis(5),
139-
.pressure = StrokeInput::kNoPressure},
140-
{.position = {2, 3},
141-
.elapsed_time = Duration32::Millis(10),
142-
.pressure = 0.5});
137+
ValidateConsistentAttributes({.position = {1, 2},
138+
.elapsed_time = Duration32::Millis(5),
139+
.pressure = StrokeInput::kNoPressure},
140+
{.position = {2, 3},
141+
.elapsed_time = Duration32::Millis(10),
142+
.pressure = 0.5});
143143
EXPECT_EQ(mismatched_has_pressure.code(), absl::StatusCode::kInvalidArgument);
144144
EXPECT_THAT(mismatched_has_pressure.message(), HasSubstr("pressure"));
145145
}
146146

147-
TEST(ValidateConsecutiveInputsTest, MismatchedOptionalTilt) {
147+
TEST(ValidateConsistentAttributesTest, MismatchedOptionalTilt) {
148148
absl::Status mismatched_has_tilt =
149-
ValidateConsecutiveInputs({.position = {1, 2},
150-
.elapsed_time = Duration32::Millis(5),
151-
.tilt = StrokeInput::kNoTilt},
152-
{.position = {2, 3},
153-
.elapsed_time = Duration32::Millis(10),
154-
.tilt = kQuarterTurn});
149+
ValidateConsistentAttributes({.position = {1, 2},
150+
.elapsed_time = Duration32::Millis(5),
151+
.tilt = StrokeInput::kNoTilt},
152+
{.position = {2, 3},
153+
.elapsed_time = Duration32::Millis(10),
154+
.tilt = kQuarterTurn});
155155
EXPECT_EQ(mismatched_has_tilt.code(), absl::StatusCode::kInvalidArgument);
156156
EXPECT_THAT(mismatched_has_tilt.message(), HasSubstr("tilt"));
157157
}
158158

159-
TEST(ValidateConsecutiveInputsTest, MismatchedOptionalOrientation) {
159+
TEST(ValidateConsistentAttributesTest, MismatchedOptionalOrientation) {
160160
absl::Status mismatched_has_orientation =
161-
ValidateConsecutiveInputs({.position = {1, 2},
162-
.elapsed_time = Duration32::Millis(5),
163-
.orientation = StrokeInput::kNoOrientation},
164-
{.position = {2, 3},
165-
.elapsed_time = Duration32::Millis(10),
166-
.orientation = kHalfTurn});
161+
ValidateConsistentAttributes({.position = {1, 2},
162+
.elapsed_time = Duration32::Millis(5),
163+
.orientation = StrokeInput::kNoOrientation},
164+
{.position = {2, 3},
165+
.elapsed_time = Duration32::Millis(10),
166+
.orientation = kHalfTurn});
167167
EXPECT_EQ(mismatched_has_orientation.code(),
168168
absl::StatusCode::kInvalidArgument);
169169
EXPECT_THAT(mismatched_has_orientation.message(), HasSubstr("orientation"));
170170
}
171171

172+
TEST(ValidateConsecutiveInputs, MismatchedAttributes) {
173+
// Mismatched attributes
174+
{
175+
absl::Status mismatched_has_orientation =
176+
ValidateConsecutiveInputs({.position = {1, 2},
177+
.elapsed_time = Duration32::Millis(5),
178+
.orientation = StrokeInput::kNoOrientation},
179+
{.position = {2, 3},
180+
.elapsed_time = Duration32::Millis(10),
181+
.orientation = kHalfTurn});
182+
EXPECT_EQ(mismatched_has_orientation.code(),
183+
absl::StatusCode::kInvalidArgument);
184+
EXPECT_THAT(mismatched_has_orientation.message(), HasSubstr("orientation"));
185+
}
186+
187+
// Mismatched tool-types
188+
{
189+
absl::Status mismatched_tool_type =
190+
ValidateConsecutiveInputs({.tool_type = StrokeInput::ToolType::kMouse,
191+
.position = {1, 2},
192+
.elapsed_time = Duration32::Millis(5)},
193+
{.tool_type = StrokeInput::ToolType::kStylus,
194+
.position = {2, 3},
195+
.elapsed_time = Duration32::Millis(10)});
196+
EXPECT_EQ(mismatched_tool_type.code(), absl::StatusCode::kInvalidArgument);
197+
EXPECT_THAT(mismatched_tool_type.message(), HasSubstr("tool_type"));
198+
}
199+
}
200+
201+
TEST(ValidateConsecutiveInputs, InvalidPositionsAndTimes) {
202+
// Decreasing time
203+
{
204+
absl::Status decreasing_time = ValidateConsecutiveInputs(
205+
{.position = {1, 2}, .elapsed_time = Duration32::Seconds(1)},
206+
{.position = {2, 3}, .elapsed_time = Duration32::Seconds(0.99)});
207+
EXPECT_EQ(decreasing_time.code(), absl::StatusCode::kInvalidArgument);
208+
EXPECT_THAT(decreasing_time.message(), HasSubstr("non-decreasing"));
209+
}
210+
211+
// Duplicate position and time
212+
{
213+
absl::Status duplicate_input = ValidateConsecutiveInputs(
214+
{.position = {1, 2}, .elapsed_time = Duration32::Millis(5)},
215+
{.position = {1, 2}, .elapsed_time = Duration32::Millis(5)});
216+
EXPECT_EQ(duplicate_input.code(), absl::StatusCode::kInvalidArgument);
217+
EXPECT_THAT(duplicate_input.message(), HasSubstr("duplicate"));
218+
}
219+
}
220+
172221
} // namespace
173222
} // namespace ink::stroke_input_internal

0 commit comments

Comments
 (0)