Skip to content

Commit 0cfa68b

Browse files
Ink Open Sourcecopybara-github
authored andcommitted
Remove brush behavior sources measured in milliseconds
These are redundant with the equivalent sources that are measured in seconds, since you can always just scale the source value range bounds. We don't need both, for the same reason that we don't have e.g. `kTiltInDegrees` in addition to `kTiltInRadians`. For now, the `*_IN_MILLIS` proto enum values have been retained (and marked deprecated), and the proto decoder will scale the source range to the "in seconds" version as necessary. PiperOrigin-RevId: 869853147
1 parent 71f6f6c commit 0cfa68b

15 files changed

+67
-186
lines changed

ink/brush/brush_behavior.cc

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,11 @@ bool IsValidBehaviorSource(BrushBehavior::Source source) {
5656
case BrushBehavior::Source::kNormalizedDirectionY:
5757
case BrushBehavior::Source::kDistanceTraveledInMultiplesOfBrushSize:
5858
case BrushBehavior::Source::kTimeOfInputInSeconds:
59-
case BrushBehavior::Source::kTimeOfInputInMillis:
6059
case BrushBehavior::Source::
6160
kPredictedDistanceTraveledInMultiplesOfBrushSize:
6261
case BrushBehavior::Source::kPredictedTimeElapsedInSeconds:
63-
case BrushBehavior::Source::kPredictedTimeElapsedInMillis:
6462
case BrushBehavior::Source::kDistanceRemainingInMultiplesOfBrushSize:
6563
case BrushBehavior::Source::kTimeSinceInputInSeconds:
66-
case BrushBehavior::Source::kTimeSinceInputInMillis:
6764
case BrushBehavior::Source::
6865
kAccelerationInMultiplesOfBrushSizePerSecondSquared:
6966
case BrushBehavior::Source::
@@ -98,10 +95,9 @@ absl::Status ValidateSourceAndOutOfRangeCombination(
9895
BrushBehavior::Source source, BrushBehavior::OutOfRange out_of_range) {
9996
switch (source) {
10097
case BrushBehavior::Source::kTimeSinceInputInSeconds:
101-
case BrushBehavior::Source::kTimeSinceInputInMillis:
10298
if (out_of_range != BrushBehavior::OutOfRange::kClamp) {
10399
return absl::InvalidArgumentError(
104-
"`Source::kTimeSinceInput*` must only be used with "
100+
"`Source::kTimeSinceInputInSeconds` must only be used with "
105101
"`source_out_of_range_behavior` of `kClamp`.");
106102
}
107103
break;
@@ -120,11 +116,9 @@ absl::Status ValidateSourceAndOutOfRangeCombination(
120116
case BrushBehavior::Source::kNormalizedDirectionY:
121117
case BrushBehavior::Source::kDistanceTraveledInMultiplesOfBrushSize:
122118
case BrushBehavior::Source::kTimeOfInputInSeconds:
123-
case BrushBehavior::Source::kTimeOfInputInMillis:
124119
case BrushBehavior::Source::
125120
kPredictedDistanceTraveledInMultiplesOfBrushSize:
126121
case BrushBehavior::Source::kPredictedTimeElapsedInSeconds:
127-
case BrushBehavior::Source::kPredictedTimeElapsedInMillis:
128122
case BrushBehavior::Source::kDistanceRemainingInMultiplesOfBrushSize:
129123
case BrushBehavior::Source::
130124
kAccelerationInMultiplesOfBrushSizePerSecondSquared:
@@ -511,21 +505,15 @@ std::string ToFormattedString(BrushBehavior::Source source) {
511505
return "kDistanceTraveledInMultiplesOfBrushSize";
512506
case BrushBehavior::Source::kTimeOfInputInSeconds:
513507
return "kTimeOfInputInSeconds";
514-
case BrushBehavior::Source::kTimeOfInputInMillis:
515-
return "kTimeOfInputInMillis";
516508
case BrushBehavior::Source::
517509
kPredictedDistanceTraveledInMultiplesOfBrushSize:
518510
return "kPredictedDistanceTraveledInMultiplesOfBrushSize";
519511
case BrushBehavior::Source::kPredictedTimeElapsedInSeconds:
520512
return "kPredictedTimeElapsedInSeconds";
521-
case BrushBehavior::Source::kPredictedTimeElapsedInMillis:
522-
return "kPredictedTimeElapsedInMillis";
523513
case BrushBehavior::Source::kDistanceRemainingInMultiplesOfBrushSize:
524514
return "kDistanceRemainingInMultiplesOfBrushSize";
525515
case BrushBehavior::Source::kTimeSinceInputInSeconds:
526516
return "kTimeSinceInputInSeconds";
527-
case BrushBehavior::Source::kTimeSinceInputInMillis:
528-
return "kTimeSinceInputInMillis";
529517
case BrushBehavior::Source::
530518
kAccelerationInMultiplesOfBrushSizePerSecondSquared:
531519
return "kAccelerationInMultiplesOfBrushSizePerSecondSquared";

ink/brush/brush_behavior.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,13 @@ struct BrushBehavior {
167167
// input. The value remains fixed for any given part of the stroke once
168168
// drawn.
169169
kTimeOfInputInSeconds,
170-
kTimeOfInputInMillis,
171170
// Distance traveled by the inputs of the current prediction, starting at 0
172171
// at the last non-predicted input, in multiples of the brush size. Zero for
173172
// inputs before the predicted portion of the stroke.
174173
kPredictedDistanceTraveledInMultiplesOfBrushSize,
175174
// Elapsed time of the prediction, starting at 0 at the last non-predicted
176175
// input. Zero for inputs before the predicted portion of the stroke.
177176
kPredictedTimeElapsedInSeconds,
178-
kPredictedTimeElapsedInMillis,
179177
// The distance left to be traveled from a given modeled input to the
180178
// current last modeled input of the stroke in multiples of the brush size.
181179
// This value changes for each input as the stroke is drawn.
@@ -186,7 +184,6 @@ struct BrushBehavior {
186184
// `source_out_of_range_behavior` of `kClamp`, to ensure that the animation
187185
// will eventually end.
188186
kTimeSinceInputInSeconds,
189-
kTimeSinceInputInMillis,
190187
// Absolute acceleration of the modeled stroke input in multiples of the
191188
// brush size per second squared. Note that this value doesn't take into
192189
// account brush behaviors that offset the position of that visible point in

ink/brush/brush_behavior_test.cc

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,17 @@ TEST(BrushBehaviorTest, StringifySource) {
6969
"kDistanceTraveledInMultiplesOfBrushSize");
7070
EXPECT_EQ(absl::StrCat(BrushBehavior::Source::kTimeOfInputInSeconds),
7171
"kTimeOfInputInSeconds");
72-
EXPECT_EQ(absl::StrCat(BrushBehavior::Source::kTimeOfInputInMillis),
73-
"kTimeOfInputInMillis");
7472
EXPECT_EQ(absl::StrCat(BrushBehavior::Source::
7573
kPredictedDistanceTraveledInMultiplesOfBrushSize),
7674
"kPredictedDistanceTraveledInMultiplesOfBrushSize");
7775
EXPECT_EQ(absl::StrCat(BrushBehavior::Source::kPredictedTimeElapsedInSeconds),
7876
"kPredictedTimeElapsedInSeconds");
79-
EXPECT_EQ(absl::StrCat(BrushBehavior::Source::kPredictedTimeElapsedInMillis),
80-
"kPredictedTimeElapsedInMillis");
8177
EXPECT_EQ(
8278
absl::StrCat(
8379
BrushBehavior::Source::kDistanceRemainingInMultiplesOfBrushSize),
8480
"kDistanceRemainingInMultiplesOfBrushSize");
8581
EXPECT_EQ(absl::StrCat(BrushBehavior::Source::kTimeSinceInputInSeconds),
8682
"kTimeSinceInputInSeconds");
87-
EXPECT_EQ(absl::StrCat(BrushBehavior::Source::kTimeSinceInputInMillis),
88-
"kTimeSinceInputInMillis");
8983
EXPECT_EQ(
9084
absl::StrCat(BrushBehavior::Source::
9185
kAccelerationInMultiplesOfBrushSizePerSecondSquared),
@@ -735,7 +729,7 @@ TEST(BrushBehaviorTest, ValidateSourceNode) {
735729
});
736730
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
737731
EXPECT_THAT(status.message(),
738-
HasSubstr("kTimeSinceInput*` must only be used with "
732+
HasSubstr("kTimeSinceInputInSeconds` must only be used with "
739733
"`source_out_of_range_behavior` of `kClamp"));
740734

741735
status = brush_internal::ValidateBrushBehaviorNode(BrushBehavior::SourceNode{

ink/brush/brush_family_test.cc

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -547,16 +547,7 @@ TEST(BrushFamilyTest, CreateWithInvalidBehaviorSourceAndOutOfRangeBehavior) {
547547
absl::Status status = BrushFamily::Create(brush_tip, BrushPaint{}).status();
548548
EXPECT_EQ(status.code(), kInvalidArgument);
549549
EXPECT_THAT(status.message(),
550-
HasSubstr("kTimeSinceInput*` must only be used with "
551-
"`source_out_of_range_behavior` of `kClamp"));
552-
553-
source_node->source = BrushBehavior::Source::kTimeSinceInputInMillis;
554-
source_node->source_out_of_range_behavior =
555-
BrushBehavior::OutOfRange::kMirror;
556-
status = BrushFamily::Create(brush_tip, BrushPaint{}).status();
557-
EXPECT_EQ(status.code(), kInvalidArgument);
558-
EXPECT_THAT(status.message(),
559-
HasSubstr("kTimeSinceInput*` must only be used with "
550+
HasSubstr("kTimeSinceInputInSeconds` must only be used with "
560551
"`source_out_of_range_behavior` of `kClamp"));
561552
}
562553

ink/brush/brush_tip_test.cc

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ TEST(BrushTipTest, Stringify) {
4343
{
4444
BrushBehavior{{
4545
BrushBehavior::SourceNode{
46-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
47-
.source_value_range = {0, 250},
46+
.source =
47+
BrushBehavior::Source::kTimeOfInputInSeconds,
48+
.source_value_range = {0, 0.25},
4849
},
4950
BrushBehavior::TargetNode{
5051
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -64,8 +65,9 @@ TEST(BrushTipTest, Stringify) {
6465
},
6566
}),
6667
"BrushTip{scale=<1.25, 0.75>, corner_rounding=0, "
67-
"behaviors={BrushBehavior{nodes={SourceNode{source=kTimeOfInputInMillis, "
68-
"source_value_range={0, 250}}, TargetNode{target=kWidthMultiplier, "
68+
"behaviors={BrushBehavior{nodes={SourceNode{source=kTimeOfInputInSeconds,"
69+
" "
70+
"source_value_range={0, 0.25}}, TargetNode{target=kWidthMultiplier, "
6971
"target_modifier_range={1.5, 2}}}}, "
7072
"BrushBehavior{nodes={SourceNode{source=kNormalizedPressure, "
7173
"source_value_range={0, 1}}, TargetNode{target=kPinchOffset, "
@@ -84,8 +86,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
8486
.behaviors = {
8587
BrushBehavior{{
8688
BrushBehavior::SourceNode{
87-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
88-
.source_value_range = {0, 250},
89+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
90+
.source_value_range = {0, 0.25},
8991
},
9092
BrushBehavior::TargetNode{
9193
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -107,8 +109,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
107109
.behaviors = {
108110
BrushBehavior{{
109111
BrushBehavior::SourceNode{
110-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
111-
.source_value_range = {0, 250},
112+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
113+
.source_value_range = {0, 0.25},
112114
},
113115
BrushBehavior::TargetNode{
114116
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -127,8 +129,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
127129
.particle_gap_duration = Duration32::Seconds(0.5),
128130
.behaviors = {BrushBehavior{{
129131
BrushBehavior::SourceNode{
130-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
131-
.source_value_range = {0, 250},
132+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
133+
.source_value_range = {0, 0.25},
132134
},
133135
BrushBehavior::TargetNode{
134136
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -148,8 +150,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
148150
.behaviors = {
149151
BrushBehavior{{
150152
BrushBehavior::SourceNode{
151-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
152-
.source_value_range = {0, 250},
153+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
154+
.source_value_range = {0, 0.25},
153155
},
154156
BrushBehavior::TargetNode{
155157
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -170,8 +172,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
170172
.behaviors = {
171173
BrushBehavior{{
172174
BrushBehavior::SourceNode{
173-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
174-
.source_value_range = {0, 250},
175+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
176+
.source_value_range = {0, 0.25},
175177
},
176178
BrushBehavior::TargetNode{
177179
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -192,8 +194,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
192194
.behaviors = {
193195
BrushBehavior{{
194196
BrushBehavior::SourceNode{
195-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
196-
.source_value_range = {0, 250},
197+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
198+
.source_value_range = {0, 0.25},
197199
},
198200
BrushBehavior::TargetNode{
199201
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -214,8 +216,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
214216
.behaviors = {
215217
BrushBehavior{{
216218
BrushBehavior::SourceNode{
217-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
218-
.source_value_range = {0, 250},
219+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
220+
.source_value_range = {0, 0.25},
219221
},
220222
BrushBehavior::TargetNode{
221223
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -236,8 +238,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
236238
.behaviors = {
237239
BrushBehavior{{
238240
BrushBehavior::SourceNode{
239-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
240-
.source_value_range = {0, 250},
241+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
242+
.source_value_range = {0, 0.25},
241243
},
242244
BrushBehavior::TargetNode{
243245
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -258,8 +260,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
258260
.behaviors = {
259261
BrushBehavior{{
260262
BrushBehavior::SourceNode{
261-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
262-
.source_value_range = {0, 250},
263+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
264+
.source_value_range = {0, 0.25},
263265
},
264266
BrushBehavior::TargetNode{
265267
.target = BrushBehavior::Target::kWidthMultiplier,
@@ -281,8 +283,8 @@ TEST(BrushTipTest, EqualAndNotEqual) {
281283
.behaviors = {
282284
BrushBehavior{{
283285
BrushBehavior::SourceNode{
284-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
285-
.source_value_range = {0, 250},
286+
.source = BrushBehavior::Source::kTimeOfInputInSeconds,
287+
.source_value_range = {0, 0.25},
286288
},
287289
BrushBehavior::TargetNode{
288290
.target = BrushBehavior::Target::kWidthMultiplier,

ink/brush/fuzz_domains.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ Domain<BrushBehavior::OutOfRange> ValidBrushBehaviorOutOfRangeForSource(
177177
BrushBehavior::Source source) {
178178
switch (source) {
179179
case BrushBehavior::Source::kTimeSinceInputInSeconds:
180-
case BrushBehavior::Source::kTimeSinceInputInMillis:
181180
return Just(BrushBehavior::OutOfRange::kClamp);
182181
default:
183182
return ArbitraryBrushBehaviorOutOfRange();
@@ -203,13 +202,10 @@ Domain<BrushBehavior::Source> ArbitraryBrushBehaviorSource() {
203202
BrushBehavior::Source::kNormalizedDirectionY,
204203
BrushBehavior::Source::kDistanceTraveledInMultiplesOfBrushSize,
205204
BrushBehavior::Source::kTimeOfInputInSeconds,
206-
BrushBehavior::Source::kTimeOfInputInMillis,
207205
BrushBehavior::Source::kPredictedDistanceTraveledInMultiplesOfBrushSize,
208206
BrushBehavior::Source::kPredictedTimeElapsedInSeconds,
209-
BrushBehavior::Source::kPredictedTimeElapsedInMillis,
210207
BrushBehavior::Source::kDistanceRemainingInMultiplesOfBrushSize,
211208
BrushBehavior::Source::kTimeSinceInputInSeconds,
212-
BrushBehavior::Source::kTimeSinceInputInMillis,
213209
BrushBehavior::Source::
214210
kAccelerationInMultiplesOfBrushSizePerSecondSquared,
215211
BrushBehavior::Source::

ink/brush/stock_brushes.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ using ToolTypeFilterNode = ::ink::BrushBehavior::ToolTypeFilterNode;
4747
BrushBehavior PredictionFadeOutBehavior() {
4848
return BrushBehavior{
4949
.nodes =
50-
{SourceNode{.source = Source::kPredictedTimeElapsedInMillis,
51-
.source_value_range = {0.0f, 24.0f}},
50+
{SourceNode{.source = Source::kPredictedTimeElapsedInSeconds,
51+
.source_value_range = {0.0f, 0.024f}},
5252
// The second branch of the binary op node keeps the opacity
5353
// fade-out from starting until the predicted inputs have traveled
5454
// at least 1.5x brush-size.

0 commit comments

Comments
 (0)