Skip to content

Commit 1ffe85f

Browse files
Ink Open Sourcecopybara-github
Ink Open Source
authored and
copybara-github
committed
Remove FallbackFilterNode and OptionalInputProperty
The new `kOrElse`, `kAndThen`, and `kXorElse` binary operators are generally a better alternative, in addition to covering other use cases. For now, `FallbackFilterNode` is still supported (though deprecated) in the protobuf, and the decoder will use the above binary operators to emulate it. PiperOrigin-RevId: 876263702
1 parent f3c6544 commit 1ffe85f

16 files changed

+206
-448
lines changed

‎ink/brush/brush_behavior.cc‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -201,18 +201,6 @@ bool IsRangeValid(std::array<float, 2> range) {
201201
std::isfinite(range[1]) && range[0] != range[1];
202202
}
203203

204-
bool IsValidOptionalInputProperty(
205-
BrushBehavior::OptionalInputProperty unreported_source) {
206-
switch (unreported_source) {
207-
case BrushBehavior::OptionalInputProperty::kPressure:
208-
case BrushBehavior::OptionalInputProperty::kTilt:
209-
case BrushBehavior::OptionalInputProperty::kOrientation:
210-
case BrushBehavior::OptionalInputProperty::kTiltXAndY:
211-
return true;
212-
}
213-
return false;
214-
}
215-
216204
bool IsValidBehaviorBinaryOp(BrushBehavior::BinaryOp operation) {
217205
switch (operation) {
218206
case BrushBehavior::BinaryOp::kProduct:
@@ -251,7 +239,6 @@ bool IsValidBehaviorInterpolation(BrushBehavior::Interpolation interpolation) {
251239
int NodeInputCount(const BrushBehavior::SourceNode& node) { return 0; }
252240
int NodeInputCount(const BrushBehavior::ConstantNode& node) { return 0; }
253241
int NodeInputCount(const BrushBehavior::NoiseNode& node) { return 0; }
254-
int NodeInputCount(const BrushBehavior::FallbackFilterNode& node) { return 1; }
255242
int NodeInputCount(const BrushBehavior::ToolTypeFilterNode& node) { return 1; }
256243
int NodeInputCount(const BrushBehavior::DampingNode& node) { return 1; }
257244
int NodeInputCount(const BrushBehavior::ResponseNode& node) { return 1; }
@@ -323,15 +310,6 @@ absl::Status ValidateNode(const BrushBehavior::NoiseNode& node) {
323310
return absl::OkStatus();
324311
}
325312

326-
absl::Status ValidateNode(const BrushBehavior::FallbackFilterNode& node) {
327-
if (!IsValidOptionalInputProperty(node.is_fallback_for)) {
328-
return absl::InvalidArgumentError(absl::StrFormat(
329-
"`FallbackFilterNode::is_fallback_for` holds non-enumerator value %d",
330-
static_cast<int>(node.is_fallback_for)));
331-
}
332-
return absl::OkStatus();
333-
}
334-
335313
absl::Status ValidateNode(const BrushBehavior::ToolTypeFilterNode& node) {
336314
if (!node.enabled_tool_types.HasAnyTypes()) {
337315
return absl::InvalidArgumentError(
@@ -654,20 +632,6 @@ std::string ToFormattedString(BrushBehavior::EnabledToolTypes enabled) {
654632
return formatted;
655633
}
656634

657-
std::string ToFormattedString(BrushBehavior::OptionalInputProperty input) {
658-
switch (input) {
659-
case BrushBehavior::OptionalInputProperty::kPressure:
660-
return "kPressure";
661-
case BrushBehavior::OptionalInputProperty::kTilt:
662-
return "kTilt";
663-
case BrushBehavior::OptionalInputProperty::kOrientation:
664-
return "kOrientation";
665-
case BrushBehavior::OptionalInputProperty::kTiltXAndY:
666-
return "kTiltXAndY";
667-
}
668-
return absl::StrCat("OptionalInputProperty(", static_cast<int>(input), ")");
669-
}
670-
671635
std::string ToFormattedString(BrushBehavior::BinaryOp operation) {
672636
switch (operation) {
673637
case BrushBehavior::BinaryOp::kProduct:
@@ -735,10 +699,6 @@ std::string ToFormattedString(const BrushBehavior::NoiseNode& node) {
735699
", vary_over=", node.vary_over, ", base_period=", node.base_period, "}");
736700
}
737701

738-
std::string ToFormattedString(const BrushBehavior::FallbackFilterNode& node) {
739-
return absl::StrCat("FallbackFilterNode{", node.is_fallback_for, "}");
740-
}
741-
742702
std::string ToFormattedString(const BrushBehavior::ToolTypeFilterNode& node) {
743703
return absl::StrCat("ToolTypeFilterNode{", node.enabled_tool_types, "}");
744704
}

‎ink/brush/brush_behavior.h‎

Lines changed: 4 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -387,17 +387,6 @@ struct BrushBehavior {
387387
static constexpr EnabledToolTypes kAllToolTypes = {
388388
.unknown = true, .mouse = true, .touch = true, .stylus = true};
389389

390-
// List of input properties that might not be reported by `StrokeInput`.
391-
//
392-
// This should match the enum in BrushBehavior.kt.
393-
enum OptionalInputProperty : int8_t {
394-
kPressure,
395-
kTilt,
396-
kOrientation,
397-
// Tilt-x and tilt-y require both tilt and orientation to be reported.
398-
kTiltXAndY,
399-
};
400-
401390
// A binary operation for combining two values in a `BinaryOpNode`. Unless
402391
// otherwise specified for a particular operator, the result will be null
403392
// (i.e. undefined) if either input value is null.
@@ -506,20 +495,6 @@ struct BrushBehavior {
506495
/// FILTER VALUE NODES ///
507496
//////////////////////////
508497

509-
// Value node for filtering out a branch of a behavior graph unless a
510-
// particular stroke input property is missing.
511-
// Inputs: 1
512-
// Output: Null if the specified property is present in the stroke input
513-
// batch, otherwise the input value.
514-
// To be valid:
515-
// - `is_fallback_for` must be a valid `OptionalInputProperty` enumerator.
516-
struct FallbackFilterNode {
517-
OptionalInputProperty is_fallback_for;
518-
519-
friend bool operator==(const FallbackFilterNode&,
520-
const FallbackFilterNode&) = default;
521-
};
522-
523498
// Value node for filtering out a branch of a behavior graph unless this
524499
// stroke's tool type is in the specified set.
525500
// Inputs: 1
@@ -664,10 +639,10 @@ struct BrushBehavior {
664639
// value, or a "terminal node" which consumes one or more input values and
665640
// applies some effect to the brush tip (but does not produce any output
666641
// value).
667-
using Node = std::variant<SourceNode, ConstantNode, NoiseNode,
668-
FallbackFilterNode, ToolTypeFilterNode, DampingNode,
669-
ResponseNode, IntegralNode, BinaryOpNode,
670-
InterpolationNode, TargetNode, PolarTargetNode>;
642+
using Node =
643+
std::variant<SourceNode, ConstantNode, NoiseNode, ToolTypeFilterNode,
644+
DampingNode, ResponseNode, IntegralNode, BinaryOpNode,
645+
InterpolationNode, TargetNode, PolarTargetNode>;
671646

672647
// A post-order traversal of this behavior's node graph.
673648
std::vector<Node> nodes;
@@ -699,7 +674,6 @@ std::string ToFormattedString(BrushBehavior::Target target);
699674
std::string ToFormattedString(BrushBehavior::PolarTarget target);
700675
std::string ToFormattedString(BrushBehavior::OutOfRange out_of_range);
701676
std::string ToFormattedString(BrushBehavior::EnabledToolTypes enabled);
702-
std::string ToFormattedString(BrushBehavior::OptionalInputProperty input);
703677
std::string ToFormattedString(BrushBehavior::BinaryOp operation);
704678
std::string ToFormattedString(BrushBehavior::ProgressDomain progress_domain);
705679
std::string ToFormattedString(BrushBehavior::Interpolation interpolation);
@@ -733,11 +707,6 @@ void AbslStringify(Sink& sink, BrushBehavior::EnabledToolTypes enabled) {
733707
sink.Append(brush_internal::ToFormattedString(enabled));
734708
}
735709

736-
template <typename Sink>
737-
void AbslStringify(Sink& sink, BrushBehavior::OptionalInputProperty input) {
738-
sink.Append(brush_internal::ToFormattedString(input));
739-
}
740-
741710
template <typename Sink>
742711
void AbslStringify(Sink& sink, BrushBehavior::BinaryOp operation) {
743712
sink.Append(brush_internal::ToFormattedString(operation));

‎ink/brush/brush_behavior_test.cc‎

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -216,18 +216,6 @@ TEST(BrushBehaviorTest, StringifyEnabledToolTypes) {
216216
"unknown/mouse/touch");
217217
}
218218

219-
TEST(BrushBehaviorTest, StringifyOptionalInputProperty) {
220-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kPressure),
221-
"kPressure");
222-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kTilt), "kTilt");
223-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kOrientation),
224-
"kOrientation");
225-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kTiltXAndY),
226-
"kTiltXAndY");
227-
EXPECT_EQ(absl::StrCat(static_cast<BrushBehavior::OptionalInputProperty>(73)),
228-
"OptionalInputProperty(73)");
229-
}
230-
231219
TEST(BrushBehaviorTest, StringifyBinaryOp) {
232220
EXPECT_EQ(absl::StrCat(BrushBehavior::BinaryOp::kProduct), "kProduct");
233221
EXPECT_EQ(absl::StrCat(BrushBehavior::BinaryOp::kSum), "kSum");
@@ -287,12 +275,6 @@ TEST(BrushBehaviorTest, StringifyNoiseNode) {
287275
"base_period=0.25}");
288276
}
289277

290-
TEST(BrushBehaviorTest, StringifyFallbackFilterNode) {
291-
EXPECT_EQ(absl::StrCat(BrushBehavior::FallbackFilterNode{
292-
BrushBehavior::OptionalInputProperty::kPressure}),
293-
"FallbackFilterNode{kPressure}");
294-
}
295-
296278
TEST(BrushBehaviorTest, StringifyToolTypeFilterNode) {
297279
EXPECT_EQ(
298280
absl::StrCat(BrushBehavior::ToolTypeFilterNode{
@@ -499,19 +481,6 @@ TEST(BrushBehaviorTest, NoiseNodeEqualAndNotEqual) {
499481
node);
500482
}
501483

502-
TEST(BrushBehaviorTest, FallbackFilterNodeEqualAndNotEqual) {
503-
BrushBehavior::FallbackFilterNode node = {
504-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kPressure};
505-
EXPECT_EQ(
506-
(BrushBehavior::FallbackFilterNode{
507-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kPressure}),
508-
node);
509-
EXPECT_NE(
510-
(BrushBehavior::FallbackFilterNode{
511-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kTilt}),
512-
node);
513-
}
514-
515484
TEST(BrushBehaviorTest, ToolTypeFilterNodeEqualAndNotEqual) {
516485
BrushBehavior::ToolTypeFilterNode node = {
517486
.enabled_tool_types = {.stylus = true}};
@@ -821,19 +790,6 @@ TEST(BrushBehaviorTest, ValidateNoiseNode) {
821790
HasSubstr("base_period` must be finite and positive")));
822791
}
823792

824-
TEST(BrushBehaviorTest, ValidateFallbackFilterNode) {
825-
EXPECT_EQ(brush_internal::ValidateBrushBehaviorNode(
826-
BrushBehavior::FallbackFilterNode{
827-
BrushBehavior::OptionalInputProperty::kPressure}),
828-
absl::OkStatus());
829-
830-
absl::Status status = brush_internal::ValidateBrushBehaviorNode(
831-
BrushBehavior::FallbackFilterNode{
832-
static_cast<BrushBehavior::OptionalInputProperty>(123)});
833-
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
834-
EXPECT_THAT(status.message(), HasSubstr("non-enumerator value 123"));
835-
}
836-
837793
TEST(BrushBehaviorTest, ValidateToolTypeFilterNode) {
838794
EXPECT_THAT(brush_internal::ValidateBrushBehaviorNode(
839795
BrushBehavior::ToolTypeFilterNode{
@@ -1073,8 +1029,6 @@ TEST(BrushBehaviorTest, ValidateBrushBehavior) {
10731029
.source_value_range = {0.5, 0.75},
10741030
},
10751031
BrushBehavior::ToolTypeFilterNode{{.stylus = true}},
1076-
BrushBehavior::FallbackFilterNode{
1077-
BrushBehavior::OptionalInputProperty::kTilt},
10781032
BrushBehavior::DampingNode{
10791033
.damping_source = BrushBehavior::ProgressDomain::kTimeInSeconds,
10801034
.damping_gap = 0.25,
@@ -1126,8 +1080,6 @@ TEST(BrushBehaviorTest, ValidateBrushBehaviorTopLevel) {
11261080
.source_value_range = {0.5, 0.75},
11271081
},
11281082
BrushBehavior::ToolTypeFilterNode{{.stylus = true}},
1129-
BrushBehavior::FallbackFilterNode{
1130-
BrushBehavior::OptionalInputProperty::kTilt},
11311083
BrushBehavior::DampingNode{
11321084
.damping_source = BrushBehavior::ProgressDomain::kTimeInSeconds,
11331085
.damping_gap = 0.25,

‎ink/brush/brush_family_test.cc‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,6 @@ BrushTip CreatePressureTestTip() {
7272
BrushBehavior::ToolTypeFilterNode{
7373
.enabled_tool_types = {.touch = true, .stylus = true},
7474
},
75-
BrushBehavior::FallbackFilterNode{
76-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kTilt,
77-
},
7875
BrushBehavior::ResponseNode{
7976
.response_curve = {EasingFunction::Predefined::kEaseInOut},
8077
},
@@ -853,28 +850,6 @@ TEST(BrushFamilyTest, CreateWithInvalidEnabledToolTypes) {
853850
EXPECT_THAT(status.message(), HasSubstr("enabled_tool_types"));
854851
}
855852

856-
TEST(BrushFamilyTest, CreateWithInvalidBehaviorFallbackSource) {
857-
BrushTip brush_tip = {
858-
.behaviors = {BrushBehavior{{
859-
BrushBehavior::SourceNode{
860-
.source = BrushBehavior::Source::kOrientationInRadians,
861-
.source_value_range = {0, 3},
862-
},
863-
BrushBehavior::FallbackFilterNode{
864-
.is_fallback_for =
865-
static_cast<BrushBehavior::OptionalInputProperty>(-1),
866-
},
867-
BrushBehavior::TargetNode{
868-
.target = BrushBehavior::Target::kPinchOffset,
869-
.target_modifier_range = {0, .2},
870-
},
871-
}}},
872-
};
873-
absl::Status status = BrushFamily::Create(brush_tip, BrushPaint{}).status();
874-
EXPECT_EQ(status.code(), kInvalidArgument);
875-
EXPECT_THAT(status.message(), HasSubstr("is_fallback_for"));
876-
}
877-
878853
TEST(BrushFamilyTest, DefaultConstruction) {
879854
BrushFamily family;
880855
EXPECT_THAT(family.GetCoats(),

‎ink/brush/fuzz_domains.cc‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -156,18 +156,6 @@ Domain<BrushBehavior::Interpolation> ArbitraryBrushBehaviorInterpolation() {
156156
}
157157
// LINT.ThenChange(brush_behavior.h:interpolation)
158158

159-
// LINT.IfChange(optional_input_property)
160-
Domain<BrushBehavior::OptionalInputProperty>
161-
ArbitraryBrushBehaviorOptionalInputProperty() {
162-
return ElementOf({
163-
BrushBehavior::OptionalInputProperty::kPressure,
164-
BrushBehavior::OptionalInputProperty::kTilt,
165-
BrushBehavior::OptionalInputProperty::kOrientation,
166-
BrushBehavior::OptionalInputProperty::kTiltXAndY,
167-
});
168-
}
169-
// LINT.ThenChange(brush_behavior.h:optional_input_property)
170-
171159
// LINT.IfChange(out_of_range)
172160
Domain<BrushBehavior::OutOfRange> ArbitraryBrushBehaviorOutOfRange() {
173161
return ElementOf({
@@ -354,12 +342,6 @@ Domain<BrushBehavior::NoiseNode> ValidBrushBehaviorNoiseNode() {
354342
FinitePositiveFloat());
355343
}
356344

357-
Domain<BrushBehavior::FallbackFilterNode>
358-
ValidBrushBehaviorFallbackFilterNode() {
359-
return StructOf<BrushBehavior::FallbackFilterNode>(
360-
ArbitraryBrushBehaviorOptionalInputProperty());
361-
}
362-
363345
Domain<BrushBehavior::ToolTypeFilterNode>
364346
ValidBrushBehaviorToolTypeFilterNode() {
365347
return StructOf<BrushBehavior::ToolTypeFilterNode>(
@@ -447,8 +429,7 @@ ValidBrushBehaviorNodeSubtreeWithMaxDepth(int max_depth) {
447429
return result;
448430
},
449431
smaller_subtree,
450-
OneOf(BrushBehaviorNodeOf(ValidBrushBehaviorFallbackFilterNode()),
451-
BrushBehaviorNodeOf(ValidBrushBehaviorToolTypeFilterNode()),
432+
OneOf(BrushBehaviorNodeOf(ValidBrushBehaviorToolTypeFilterNode()),
452433
BrushBehaviorNodeOf(ValidBrushBehaviorDampingNode()),
453434
BrushBehaviorNodeOf(ValidBrushBehaviorResponseNode()),
454435
BrushBehaviorNodeOf(ValidBrushBehaviorIntegralNode()))),
@@ -545,10 +526,10 @@ Domain<BrushBehavior> ValidBrushBehavior(DomainVariant variant) {
545526
Domain<BrushBehavior::Node> ValidBrushBehaviorNode(DomainVariant variant) {
546527
return VariantOf(
547528
ValidBrushBehaviorSourceNode(), ValidBrushBehaviorConstantNode(),
548-
ValidBrushBehaviorNoiseNode(), ValidBrushBehaviorFallbackFilterNode(),
549-
ValidBrushBehaviorToolTypeFilterNode(), ValidBrushBehaviorDampingNode(),
550-
ValidBrushBehaviorResponseNode(), ValidBrushBehaviorIntegralNode(),
551-
ValidBrushBehaviorBinaryOpNode(), ValidBrushBehaviorInterpolationNode(),
529+
ValidBrushBehaviorNoiseNode(), ValidBrushBehaviorToolTypeFilterNode(),
530+
ValidBrushBehaviorDampingNode(), ValidBrushBehaviorResponseNode(),
531+
ValidBrushBehaviorIntegralNode(), ValidBrushBehaviorBinaryOpNode(),
532+
ValidBrushBehaviorInterpolationNode(),
552533
ValidBrushBehaviorTargetNode(variant),
553534
ValidBrushBehaviorPolarTargetNode());
554535
}

0 commit comments

Comments
 (0)