Skip to content

Commit a028aa6

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: 875304881
1 parent d696aa6 commit a028aa6

15 files changed

+178
-445
lines changed

‎ink/brush/brush_behavior.cc‎

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

202-
bool IsValidOptionalInputProperty(
203-
BrushBehavior::OptionalInputProperty unreported_source) {
204-
switch (unreported_source) {
205-
case BrushBehavior::OptionalInputProperty::kPressure:
206-
case BrushBehavior::OptionalInputProperty::kTilt:
207-
case BrushBehavior::OptionalInputProperty::kOrientation:
208-
case BrushBehavior::OptionalInputProperty::kTiltXAndY:
209-
return true;
210-
}
211-
return false;
212-
}
213-
214202
bool IsValidBehaviorBinaryOp(BrushBehavior::BinaryOp operation) {
215203
switch (operation) {
216204
case BrushBehavior::BinaryOp::kProduct:
@@ -249,7 +237,6 @@ bool IsValidBehaviorInterpolation(BrushBehavior::Interpolation interpolation) {
249237
int NodeInputCount(const BrushBehavior::SourceNode& node) { return 0; }
250238
int NodeInputCount(const BrushBehavior::ConstantNode& node) { return 0; }
251239
int NodeInputCount(const BrushBehavior::NoiseNode& node) { return 0; }
252-
int NodeInputCount(const BrushBehavior::FallbackFilterNode& node) { return 1; }
253240
int NodeInputCount(const BrushBehavior::ToolTypeFilterNode& node) { return 1; }
254241
int NodeInputCount(const BrushBehavior::DampingNode& node) { return 1; }
255242
int NodeInputCount(const BrushBehavior::ResponseNode& node) { return 1; }
@@ -321,15 +308,6 @@ absl::Status ValidateNode(const BrushBehavior::NoiseNode& node) {
321308
return absl::OkStatus();
322309
}
323310

324-
absl::Status ValidateNode(const BrushBehavior::FallbackFilterNode& node) {
325-
if (!IsValidOptionalInputProperty(node.is_fallback_for)) {
326-
return absl::InvalidArgumentError(absl::StrFormat(
327-
"`FallbackFilterNode::is_fallback_for` holds non-enumerator value %d",
328-
static_cast<int>(node.is_fallback_for)));
329-
}
330-
return absl::OkStatus();
331-
}
332-
333311
absl::Status ValidateNode(const BrushBehavior::ToolTypeFilterNode& node) {
334312
if (!node.enabled_tool_types.HasAnyTypes()) {
335313
return absl::InvalidArgumentError(
@@ -650,20 +628,6 @@ std::string ToFormattedString(BrushBehavior::EnabledToolTypes enabled) {
650628
return formatted;
651629
}
652630

653-
std::string ToFormattedString(BrushBehavior::OptionalInputProperty input) {
654-
switch (input) {
655-
case BrushBehavior::OptionalInputProperty::kPressure:
656-
return "kPressure";
657-
case BrushBehavior::OptionalInputProperty::kTilt:
658-
return "kTilt";
659-
case BrushBehavior::OptionalInputProperty::kOrientation:
660-
return "kOrientation";
661-
case BrushBehavior::OptionalInputProperty::kTiltXAndY:
662-
return "kTiltXAndY";
663-
}
664-
return absl::StrCat("OptionalInputProperty(", static_cast<int>(input), ")");
665-
}
666-
667631
std::string ToFormattedString(BrushBehavior::BinaryOp operation) {
668632
switch (operation) {
669633
case BrushBehavior::BinaryOp::kProduct:
@@ -731,10 +695,6 @@ std::string ToFormattedString(const BrushBehavior::NoiseNode& node) {
731695
", vary_over=", node.vary_over, ", base_period=", node.base_period, "}");
732696
}
733697

734-
std::string ToFormattedString(const BrushBehavior::FallbackFilterNode& node) {
735-
return absl::StrCat("FallbackFilterNode{", node.is_fallback_for, "}");
736-
}
737-
738698
std::string ToFormattedString(const BrushBehavior::ToolTypeFilterNode& node) {
739699
return absl::StrCat("ToolTypeFilterNode{", node.enabled_tool_types, "}");
740700
}

‎ink/brush/brush_behavior.h‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -385,18 +385,6 @@ struct BrushBehavior {
385385
static constexpr EnabledToolTypes kAllToolTypes = {
386386
.unknown = true, .mouse = true, .touch = true, .stylus = true};
387387

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

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

671645
// A post-order traversal of this behavior's node graph.
672646
std::vector<Node> nodes;
@@ -698,7 +672,6 @@ std::string ToFormattedString(BrushBehavior::Target target);
698672
std::string ToFormattedString(BrushBehavior::PolarTarget target);
699673
std::string ToFormattedString(BrushBehavior::OutOfRange out_of_range);
700674
std::string ToFormattedString(BrushBehavior::EnabledToolTypes enabled);
701-
std::string ToFormattedString(BrushBehavior::OptionalInputProperty input);
702675
std::string ToFormattedString(BrushBehavior::BinaryOp operation);
703676
std::string ToFormattedString(BrushBehavior::ProgressDomain progress_domain);
704677
std::string ToFormattedString(BrushBehavior::Interpolation interpolation);
@@ -732,11 +705,6 @@ void AbslStringify(Sink& sink, BrushBehavior::EnabledToolTypes enabled) {
732705
sink.Append(brush_internal::ToFormattedString(enabled));
733706
}
734707

735-
template <typename Sink>
736-
void AbslStringify(Sink& sink, BrushBehavior::OptionalInputProperty input) {
737-
sink.Append(brush_internal::ToFormattedString(input));
738-
}
739-
740708
template <typename Sink>
741709
void AbslStringify(Sink& sink, BrushBehavior::BinaryOp operation) {
742710
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
@@ -214,18 +214,6 @@ TEST(BrushBehaviorTest, StringifyEnabledToolTypes) {
214214
"unknown/mouse/touch");
215215
}
216216

217-
TEST(BrushBehaviorTest, StringifyOptionalInputProperty) {
218-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kPressure),
219-
"kPressure");
220-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kTilt), "kTilt");
221-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kOrientation),
222-
"kOrientation");
223-
EXPECT_EQ(absl::StrCat(BrushBehavior::OptionalInputProperty::kTiltXAndY),
224-
"kTiltXAndY");
225-
EXPECT_EQ(absl::StrCat(static_cast<BrushBehavior::OptionalInputProperty>(73)),
226-
"OptionalInputProperty(73)");
227-
}
228-
229217
TEST(BrushBehaviorTest, StringifyBinaryOp) {
230218
EXPECT_EQ(absl::StrCat(BrushBehavior::BinaryOp::kProduct), "kProduct");
231219
EXPECT_EQ(absl::StrCat(BrushBehavior::BinaryOp::kSum), "kSum");
@@ -285,12 +273,6 @@ TEST(BrushBehaviorTest, StringifyNoiseNode) {
285273
"base_period=0.25}");
286274
}
287275

288-
TEST(BrushBehaviorTest, StringifyFallbackFilterNode) {
289-
EXPECT_EQ(absl::StrCat(BrushBehavior::FallbackFilterNode{
290-
BrushBehavior::OptionalInputProperty::kPressure}),
291-
"FallbackFilterNode{kPressure}");
292-
}
293-
294276
TEST(BrushBehaviorTest, StringifyToolTypeFilterNode) {
295277
EXPECT_EQ(
296278
absl::StrCat(BrushBehavior::ToolTypeFilterNode{
@@ -497,19 +479,6 @@ TEST(BrushBehaviorTest, NoiseNodeEqualAndNotEqual) {
497479
node);
498480
}
499481

500-
TEST(BrushBehaviorTest, FallbackFilterNodeEqualAndNotEqual) {
501-
BrushBehavior::FallbackFilterNode node = {
502-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kPressure};
503-
EXPECT_EQ(
504-
(BrushBehavior::FallbackFilterNode{
505-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kPressure}),
506-
node);
507-
EXPECT_NE(
508-
(BrushBehavior::FallbackFilterNode{
509-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kTilt}),
510-
node);
511-
}
512-
513482
TEST(BrushBehaviorTest, ToolTypeFilterNodeEqualAndNotEqual) {
514483
BrushBehavior::ToolTypeFilterNode node = {
515484
.enabled_tool_types = {.stylus = true}};
@@ -809,19 +778,6 @@ TEST(BrushBehaviorTest, ValidateNoiseNode) {
809778
HasSubstr("base_period` must be finite and positive")));
810779
}
811780

812-
TEST(BrushBehaviorTest, ValidateFallbackFilterNode) {
813-
EXPECT_EQ(brush_internal::ValidateBrushBehaviorNode(
814-
BrushBehavior::FallbackFilterNode{
815-
BrushBehavior::OptionalInputProperty::kPressure}),
816-
absl::OkStatus());
817-
818-
absl::Status status = brush_internal::ValidateBrushBehaviorNode(
819-
BrushBehavior::FallbackFilterNode{
820-
static_cast<BrushBehavior::OptionalInputProperty>(123)});
821-
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
822-
EXPECT_THAT(status.message(), HasSubstr("non-enumerator value 123"));
823-
}
824-
825781
TEST(BrushBehaviorTest, ValidateToolTypeFilterNode) {
826782
EXPECT_THAT(brush_internal::ValidateBrushBehaviorNode(
827783
BrushBehavior::ToolTypeFilterNode{
@@ -1061,8 +1017,6 @@ TEST(BrushBehaviorTest, ValidateBrushBehavior) {
10611017
.source_value_range = {0.5, 0.75},
10621018
},
10631019
BrushBehavior::ToolTypeFilterNode{{.stylus = true}},
1064-
BrushBehavior::FallbackFilterNode{
1065-
BrushBehavior::OptionalInputProperty::kTilt},
10661020
BrushBehavior::DampingNode{
10671021
.damping_source = BrushBehavior::ProgressDomain::kTimeInSeconds,
10681022
.damping_gap = 0.25,
@@ -1114,8 +1068,6 @@ TEST(BrushBehaviorTest, ValidateBrushBehaviorTopLevel) {
11141068
.source_value_range = {0.5, 0.75},
11151069
},
11161070
BrushBehavior::ToolTypeFilterNode{{.stylus = true}},
1117-
BrushBehavior::FallbackFilterNode{
1118-
BrushBehavior::OptionalInputProperty::kTilt},
11191071
BrushBehavior::DampingNode{
11201072
.damping_source = BrushBehavior::ProgressDomain::kTimeInSeconds,
11211073
.damping_gap = 0.25,

‎ink/brush/brush_family_test.cc‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,6 @@ BrushTip CreatePressureTestTip() {
7373
BrushBehavior::ToolTypeFilterNode{
7474
.enabled_tool_types = {.touch = true, .stylus = true},
7575
},
76-
BrushBehavior::FallbackFilterNode{
77-
.is_fallback_for = BrushBehavior::OptionalInputProperty::kTilt,
78-
},
7976
BrushBehavior::ResponseNode{
8077
.response_curve = {EasingFunction::Predefined::kEaseInOut},
8178
},
@@ -857,28 +854,6 @@ TEST(BrushFamilyTest, CreateWithInvalidEnabledToolTypes) {
857854
EXPECT_THAT(status.message(), HasSubstr("enabled_tool_types"));
858855
}
859856

860-
TEST(BrushFamilyTest, CreateWithInvalidBehaviorFallbackSource) {
861-
BrushTip brush_tip = {
862-
.behaviors = {BrushBehavior{{
863-
BrushBehavior::SourceNode{
864-
.source = BrushBehavior::Source::kOrientationInRadians,
865-
.source_value_range = {0, 3},
866-
},
867-
BrushBehavior::FallbackFilterNode{
868-
.is_fallback_for =
869-
static_cast<BrushBehavior::OptionalInputProperty>(-1),
870-
},
871-
BrushBehavior::TargetNode{
872-
.target = BrushBehavior::Target::kPinchOffset,
873-
.target_modifier_range = {0, .2},
874-
},
875-
}}},
876-
};
877-
absl::Status status = BrushFamily::Create(brush_tip, BrushPaint{}).status();
878-
EXPECT_EQ(status.code(), kInvalidArgument);
879-
EXPECT_THAT(status.message(), HasSubstr("is_fallback_for"));
880-
}
881-
882857
TEST(BrushFamilyTest, DefaultConstruction) {
883858
BrushFamily family;
884859
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({
@@ -352,12 +340,6 @@ Domain<BrushBehavior::NoiseNode> ValidBrushBehaviorNoiseNode() {
352340
FinitePositiveFloat());
353341
}
354342

355-
Domain<BrushBehavior::FallbackFilterNode>
356-
ValidBrushBehaviorFallbackFilterNode() {
357-
return StructOf<BrushBehavior::FallbackFilterNode>(
358-
ArbitraryBrushBehaviorOptionalInputProperty());
359-
}
360-
361343
Domain<BrushBehavior::ToolTypeFilterNode>
362344
ValidBrushBehaviorToolTypeFilterNode() {
363345
return StructOf<BrushBehavior::ToolTypeFilterNode>(
@@ -445,8 +427,7 @@ ValidBrushBehaviorNodeSubtreeWithMaxDepth(int max_depth) {
445427
return result;
446428
},
447429
smaller_subtree,
448-
OneOf(BrushBehaviorNodeOf(ValidBrushBehaviorFallbackFilterNode()),
449-
BrushBehaviorNodeOf(ValidBrushBehaviorToolTypeFilterNode()),
430+
OneOf(BrushBehaviorNodeOf(ValidBrushBehaviorToolTypeFilterNode()),
450431
BrushBehaviorNodeOf(ValidBrushBehaviorDampingNode()),
451432
BrushBehaviorNodeOf(ValidBrushBehaviorResponseNode()),
452433
BrushBehaviorNodeOf(ValidBrushBehaviorIntegralNode()))),
@@ -543,10 +524,10 @@ Domain<BrushBehavior> ValidBrushBehavior(DomainVariant variant) {
543524
Domain<BrushBehavior::Node> ValidBrushBehaviorNode(DomainVariant variant) {
544525
return VariantOf(
545526
ValidBrushBehaviorSourceNode(), ValidBrushBehaviorConstantNode(),
546-
ValidBrushBehaviorNoiseNode(), ValidBrushBehaviorFallbackFilterNode(),
547-
ValidBrushBehaviorToolTypeFilterNode(), ValidBrushBehaviorDampingNode(),
548-
ValidBrushBehaviorResponseNode(), ValidBrushBehaviorIntegralNode(),
549-
ValidBrushBehaviorBinaryOpNode(), ValidBrushBehaviorInterpolationNode(),
527+
ValidBrushBehaviorNoiseNode(), ValidBrushBehaviorToolTypeFilterNode(),
528+
ValidBrushBehaviorDampingNode(), ValidBrushBehaviorResponseNode(),
529+
ValidBrushBehaviorIntegralNode(), ValidBrushBehaviorBinaryOpNode(),
530+
ValidBrushBehaviorInterpolationNode(),
550531
ValidBrushBehaviorTargetNode(variant),
551532
ValidBrushBehaviorPolarTargetNode());
552533
}

0 commit comments

Comments
 (0)