Skip to content

Commit 27e34ba

Browse files
maxmmitchellcopybara-github
authored andcommitted
Populate min_version BrushFamily proto field.
Introduces code to calculate the minimum required version of Ink for a BrushFamily (and each part of a BrushFamily). The code is used during serialization to compute the value to store in the `min_version` field. During deserialization, the `min_version` field is used to determine if a BrushFamily is compatible with the current version of Ink (<= max supported version); if it isn't, decoding it is aborted. All future features added to custom brushes must be associated with a version number in the proto, and that same number must associated with that feature in the appropriate part of `CalculateMinimumRequiredVersion`. There are two options when selecting a version for a new feature: * `Version::kMaxSupported() + 1`. This will create a new version level and cement the new feature as part of the custom brush API that the Ink team will support. If this option is chosen, `kMaxSupported` itself must be incremented, and a new version accessor static method should be added to `Version`. * `Version::kDevelopment()`. This value is a sentinel (represented under the hood with `int32 max()`) to hold features which aren't ready to be released yet. Brushes containing features associated with `kDevelopment` will fail to be deserialized under normal circumstances (an optional argument to `DecodeBrush/BrushFamily()` controls this). This is WAI, since `kDevelopment > kMaxSupported`. Features associated with `kDevelopment` are unstable, so Ink makes no guarantees they will be supported by future versions. Serializing them is risky, and deserializing them is especially risky. Brushes with the version `kDevelopment` may contain features no longer supported, or whose interfaces have changed. These features are available for the purposes of prototyping and research, and deserializing them is only provided as an option for the purpose of convenience. Deserializing a brush with a `min_version` of `kDevelopment` may lead to unexpected behavior. To ensure the code in the web of `CalculateMinimumRequiredVersion` functions lines up with expectations, protobuf options are used to build consistency check tests. `CalculateMinimumRequiredVersion` serves as the enforcement of the expectations set in the protobuf itself. These tests are designed to automagically ensure new features are thoughtfully assigned a version. It does so by verifying that new fields/messages/enum values in proto have a version assigned, and that the version descriptor for each field/message/enum value in the proto for a brush family is consistent with whatever `CalculateMinimumRequiredVersion` spits out for the same brush family. IOW, a CL which adds a new serializable feature will not be able to pass TAP until that feature is assigned a version. The tests are: * A fuzz test to generate valid brush families and serialize them. Then, it checks the value in `min_version` against all the options set for the fields, messages, and enum values, to ensure it is equal to the highest value set for the relevant options. This ensures `CalculateMinimumRequiredVersion` is consistent with the protobuf options. * A test to iterate over all the messages, fields, and enum values in `brush_family.proto`, starting at the top-level `BrushFamily` message, and verify that they all have a relevant `field_min_version`, `message_min_version`, or `enum_value_min_version` set, and that this value set is less than or equal to the maximum compatible version (`kMaxSupported`) or is equal to `kDevelopment`. * A test to verify that decoding a `min_version` out of the acceptable range (higher than `kMaxSupported`) will fail. * A test to verify that decoding a `proto::BrushFamily` with no value set for its `min_version` field will ***not*** fail, to ensure older brushes are still able to be loaded. * Tests to verify that `DecodeBrush()` and `DecodeBrushFamily()` will accept brushes with the version `kDevelopment` if passed the appropriate argument. A more direct implementation, using reflection to examine the descriptors of the protobuf directly during serialization, was considered, but ultimately rejected. It would have required both static and runtime reflection, and increased `libink.so` size by ~2%. PiperOrigin-RevId: 886568015
1 parent f6acd56 commit 27e34ba

21 files changed

+820
-25
lines changed

MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ git_repository(
6464
bazel_dep(name = "protobuf", version = "31.1")
6565
bazel_dep(name = "rules_android", version = "0.6.0")
6666
bazel_dep(name = "rules_jni", version = "0.11.1")
67-
bazel_dep(name = "rules_proto", version = "7.1.0")
67+
bazel_dep(name = "rules_proto", version = "6.0.2")
6868

6969
git_repository(
7070
name = "skia",

ink/brush/BUILD.bazel

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ cc_library(
8282
srcs = ["brush_coat.cc"],
8383
hdrs = ["brush_coat.h"],
8484
deps = [
85-
":brush_behavior",
8685
":brush_paint",
8786
":brush_tip",
87+
":version",
8888
"//ink/geometry:mesh_format",
8989
"@com_google_absl//absl/container:flat_hash_set",
9090
"@com_google_absl//absl/container:inlined_vector",
@@ -120,6 +120,7 @@ cc_library(
120120
":brush_coat",
121121
":brush_paint",
122122
":brush_tip",
123+
":version",
123124
"//ink/types:duration",
124125
"@com_google_absl//absl/status",
125126
"@com_google_absl//absl/status:statusor",
@@ -160,11 +161,11 @@ cc_library(
160161
hdrs = ["brush_tip.h"],
161162
deps = [
162163
":brush_behavior",
164+
":version",
163165
"//ink/geometry:angle",
164166
"//ink/geometry:mesh_format",
165167
"//ink/geometry:vec",
166168
"//ink/types:duration",
167-
"@com_google_absl//absl/base:core_headers",
168169
"@com_google_absl//absl/container:flat_hash_set",
169170
"@com_google_absl//absl/status",
170171
"@com_google_absl//absl/strings",
@@ -190,6 +191,7 @@ cc_library(
190191
srcs = ["color_function.cc"],
191192
hdrs = ["color_function.h"],
192193
deps = [
194+
":version",
193195
"//ink/color",
194196
"@com_google_absl//absl/status",
195197
"@com_google_absl//absl/strings",
@@ -219,6 +221,7 @@ cc_library(
219221
srcs = ["easing_function.cc"],
220222
hdrs = ["easing_function.h"],
221223
deps = [
224+
":version",
222225
"//ink/geometry:point",
223226
"@com_google_absl//absl/status",
224227
"@com_google_absl//absl/strings",
@@ -245,6 +248,7 @@ cc_library(
245248
hdrs = ["brush_behavior.h"],
246249
deps = [
247250
":easing_function",
251+
":version",
248252
"@com_google_absl//absl/status",
249253
"@com_google_absl//absl/strings",
250254
"@com_google_absl//absl/strings:str_format",
@@ -273,6 +277,7 @@ cc_library(
273277
hdrs = ["brush_paint.h"],
274278
deps = [
275279
":color_function",
280+
":version",
276281
"//ink/geometry:angle",
277282
"//ink/geometry:mesh_format",
278283
"//ink/geometry:vec",
@@ -365,6 +370,15 @@ cc_library(
365370
],
366371
)
367372

373+
cc_library(
374+
name = "version",
375+
hdrs = ["version.h"],
376+
deps = [
377+
"@com_google_absl//absl/log:absl_check",
378+
"@com_google_absl//absl/strings",
379+
],
380+
)
381+
368382
cc_test(
369383
name = "stock_brushes_test",
370384
srcs = ["stock_brushes_test.cc"],

ink/brush/brush_behavior.cc

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "ink/brush/brush_behavior.h"
1616

17+
#include <algorithm>
1718
#include <array>
1819
#include <cmath>
1920
#include <cstddef>
@@ -25,6 +26,7 @@
2526
#include "absl/strings/str_format.h"
2627
#include "absl/strings/str_join.h"
2728
#include "ink/brush/easing_function.h"
29+
#include "ink/brush/version.h"
2830

2931
namespace ink {
3032

@@ -454,6 +456,207 @@ absl::Status ValidateBrushBehavior(const BrushBehavior& behavior) {
454456
return absl::OkStatus();
455457
}
456458

459+
namespace {
460+
461+
Version CalculateMinimumRequiredVersion(BrushBehavior::Source source) {
462+
switch (source) {
463+
case BrushBehavior::Source::kNormalizedPressure:
464+
case BrushBehavior::Source::kTiltInRadians:
465+
case BrushBehavior::Source::kTiltXInRadians:
466+
case BrushBehavior::Source::kTiltYInRadians:
467+
case BrushBehavior::Source::kOrientationInRadians:
468+
case BrushBehavior::Source::kOrientationAboutZeroInRadians:
469+
case BrushBehavior::Source::kSpeedInMultiplesOfBrushSizePerSecond:
470+
case BrushBehavior::Source::kVelocityXInMultiplesOfBrushSizePerSecond:
471+
case BrushBehavior::Source::kVelocityYInMultiplesOfBrushSizePerSecond:
472+
case BrushBehavior::Source::kDirectionInRadians:
473+
case BrushBehavior::Source::kDirectionAboutZeroInRadians:
474+
case BrushBehavior::Source::kNormalizedDirectionX:
475+
case BrushBehavior::Source::kNormalizedDirectionY:
476+
case BrushBehavior::Source::kDistanceTraveledInMultiplesOfBrushSize:
477+
case BrushBehavior::Source::kTimeOfInputInSeconds:
478+
case BrushBehavior::Source::
479+
kPredictedDistanceTraveledInMultiplesOfBrushSize:
480+
case BrushBehavior::Source::kPredictedTimeElapsedInSeconds:
481+
case BrushBehavior::Source::kDistanceRemainingInMultiplesOfBrushSize:
482+
case BrushBehavior::Source::kTimeSinceInputInSeconds:
483+
case BrushBehavior::Source::
484+
kAccelerationInMultiplesOfBrushSizePerSecondSquared:
485+
case BrushBehavior::Source::
486+
kAccelerationXInMultiplesOfBrushSizePerSecondSquared:
487+
case BrushBehavior::Source::
488+
kAccelerationYInMultiplesOfBrushSizePerSecondSquared:
489+
case BrushBehavior::Source::
490+
kAccelerationForwardInMultiplesOfBrushSizePerSecondSquared:
491+
case BrushBehavior::Source::
492+
kAccelerationLateralInMultiplesOfBrushSizePerSecondSquared:
493+
case BrushBehavior::Source::kSpeedInCentimetersPerSecond:
494+
case BrushBehavior::Source::kVelocityXInCentimetersPerSecond:
495+
case BrushBehavior::Source::kVelocityYInCentimetersPerSecond:
496+
case BrushBehavior::Source::kDistanceTraveledInCentimeters:
497+
case BrushBehavior::Source::kPredictedDistanceTraveledInCentimeters:
498+
case BrushBehavior::Source::kAccelerationInCentimetersPerSecondSquared:
499+
case BrushBehavior::Source::kAccelerationXInCentimetersPerSecondSquared:
500+
case BrushBehavior::Source::kAccelerationYInCentimetersPerSecondSquared:
501+
case BrushBehavior::Source::
502+
kAccelerationForwardInCentimetersPerSecondSquared:
503+
case BrushBehavior::Source::
504+
kAccelerationLateralInCentimetersPerSecondSquared:
505+
case BrushBehavior::Source::kDistanceRemainingAsFractionOfStrokeLength:
506+
return Version::k0Jetpack1_0_0();
507+
case BrushBehavior::Source::kTimeSinceStrokeEndInSeconds:
508+
return Version::k1Jetpack1_1_0Alpha01();
509+
case BrushBehavior::Source::kTimeFromInputToStrokeEndInSeconds:
510+
return Version::kDevelopment();
511+
}
512+
}
513+
514+
Version CalculateMinimumRequiredVersion(BrushBehavior::Target target) {
515+
switch (target) {
516+
case BrushBehavior::Target::kWidthMultiplier:
517+
case BrushBehavior::Target::kHeightMultiplier:
518+
case BrushBehavior::Target::kSizeMultiplier:
519+
case BrushBehavior::Target::kSlantOffsetInRadians:
520+
case BrushBehavior::Target::kPinchOffset:
521+
case BrushBehavior::Target::kRotationOffsetInRadians:
522+
case BrushBehavior::Target::kCornerRoundingOffset:
523+
case BrushBehavior::Target::kPositionOffsetXInMultiplesOfBrushSize:
524+
case BrushBehavior::Target::kPositionOffsetYInMultiplesOfBrushSize:
525+
case BrushBehavior::Target::kPositionOffsetForwardInMultiplesOfBrushSize:
526+
case BrushBehavior::Target::kPositionOffsetLateralInMultiplesOfBrushSize:
527+
case BrushBehavior::Target::kTextureAnimationProgressOffset:
528+
case BrushBehavior::Target::kHueOffsetInRadians:
529+
case BrushBehavior::Target::kSaturationMultiplier:
530+
case BrushBehavior::Target::kLuminosityOffset:
531+
case BrushBehavior::Target::kOpacityMultiplier:
532+
return Version::k0Jetpack1_0_0();
533+
}
534+
}
535+
536+
Version CalculateMinimumRequiredVersion(BrushBehavior::PolarTarget target) {
537+
switch (target) {
538+
case BrushBehavior::PolarTarget::
539+
kPositionOffsetAbsoluteInRadiansAndMultiplesOfBrushSize:
540+
case BrushBehavior::PolarTarget::
541+
kPositionOffsetRelativeInRadiansAndMultiplesOfBrushSize:
542+
return Version::k0Jetpack1_0_0();
543+
}
544+
}
545+
546+
Version CalculateMinimumRequiredVersion(
547+
BrushBehavior::OutOfRange out_of_range) {
548+
switch (out_of_range) {
549+
case BrushBehavior::OutOfRange::kClamp:
550+
case BrushBehavior::OutOfRange::kMirror:
551+
case BrushBehavior::OutOfRange::kRepeat:
552+
return Version::k0Jetpack1_0_0();
553+
}
554+
}
555+
556+
Version CalculateMinimumRequiredVersion(
557+
BrushBehavior::EnabledToolTypes enabled) {
558+
return Version::k0Jetpack1_0_0();
559+
}
560+
561+
Version CalculateMinimumRequiredVersion(BrushBehavior::BinaryOp operation) {
562+
switch (operation) {
563+
case BrushBehavior::BinaryOp::kProduct:
564+
case BrushBehavior::BinaryOp::kSum:
565+
return Version::k0Jetpack1_0_0();
566+
case BrushBehavior::BinaryOp::kMin:
567+
case BrushBehavior::BinaryOp::kMax:
568+
case BrushBehavior::BinaryOp::kAndThen:
569+
case BrushBehavior::BinaryOp::kOrElse:
570+
case BrushBehavior::BinaryOp::kXorElse:
571+
return Version::k1Jetpack1_1_0Alpha01();
572+
}
573+
}
574+
575+
Version CalculateMinimumRequiredVersion(
576+
BrushBehavior::ProgressDomain progress_domain) {
577+
switch (progress_domain) {
578+
case BrushBehavior::ProgressDomain::kDistanceInCentimeters:
579+
case BrushBehavior::ProgressDomain::kDistanceInMultiplesOfBrushSize:
580+
case BrushBehavior::ProgressDomain::kTimeInSeconds:
581+
return Version::k0Jetpack1_0_0();
582+
}
583+
}
584+
585+
Version CalculateMinimumRequiredVersion(
586+
BrushBehavior::Interpolation interpolation) {
587+
switch (interpolation) {
588+
case BrushBehavior::Interpolation::kLerp:
589+
case BrushBehavior::Interpolation::kInverseLerp:
590+
return Version::k0Jetpack1_0_0();
591+
}
592+
}
593+
594+
Version CalculateMinimumRequiredVersion(BrushBehavior::SourceNode node) {
595+
return std::max(
596+
CalculateMinimumRequiredVersion(node.source_out_of_range_behavior),
597+
CalculateMinimumRequiredVersion(node.source));
598+
}
599+
600+
Version CalculateMinimumRequiredVersion(BrushBehavior::ConstantNode node) {
601+
return Version::k0Jetpack1_0_0();
602+
}
603+
604+
Version CalculateMinimumRequiredVersion(BrushBehavior::NoiseNode node) {
605+
return CalculateMinimumRequiredVersion(node.vary_over);
606+
}
607+
608+
Version CalculateMinimumRequiredVersion(
609+
BrushBehavior::ToolTypeFilterNode node) {
610+
return CalculateMinimumRequiredVersion(node.enabled_tool_types);
611+
}
612+
613+
Version CalculateMinimumRequiredVersion(BrushBehavior::DampingNode node) {
614+
return CalculateMinimumRequiredVersion(node.damping_source);
615+
}
616+
617+
Version CalculateMinimumRequiredVersion(BrushBehavior::ResponseNode node) {
618+
return brush_internal::CalculateMinimumRequiredVersion(node.response_curve);
619+
}
620+
621+
Version CalculateMinimumRequiredVersion(BrushBehavior::IntegralNode node) {
622+
return std::max(
623+
{Version::k1Jetpack1_1_0Alpha01(),
624+
CalculateMinimumRequiredVersion(node.integrate_over),
625+
CalculateMinimumRequiredVersion(node.integral_out_of_range_behavior)});
626+
}
627+
628+
Version CalculateMinimumRequiredVersion(BrushBehavior::BinaryOpNode node) {
629+
return CalculateMinimumRequiredVersion(node.operation);
630+
}
631+
632+
Version CalculateMinimumRequiredVersion(BrushBehavior::InterpolationNode node) {
633+
return CalculateMinimumRequiredVersion(node.interpolation);
634+
}
635+
636+
Version CalculateMinimumRequiredVersion(BrushBehavior::TargetNode node) {
637+
return CalculateMinimumRequiredVersion(node.target);
638+
}
639+
640+
Version CalculateMinimumRequiredVersion(BrushBehavior::PolarTargetNode node) {
641+
return CalculateMinimumRequiredVersion(node.target);
642+
}
643+
644+
Version CalculateMinimumRequiredVersion(const BrushBehavior::Node& node) {
645+
return std::visit(
646+
[](const auto& node) { return CalculateMinimumRequiredVersion(node); },
647+
node);
648+
}
649+
650+
} // namespace
651+
652+
Version CalculateMinimumRequiredVersion(const BrushBehavior& behavior) {
653+
Version max_version = Version::k0Jetpack1_0_0();
654+
for (const auto& node : behavior.nodes) {
655+
max_version = std::max(max_version, CalculateMinimumRequiredVersion(node));
656+
}
657+
return max_version;
658+
}
659+
457660
std::string ToFormattedString(BrushBehavior::Source source) {
458661
switch (source) {
459662
case BrushBehavior::Source::kNormalizedPressure:

ink/brush/brush_behavior.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616
#define INK_STROKES_BRUSH_BRUSH_BEHAVIOR_H_
1717

1818
#include <array>
19-
#include <cstdint>
2019
#include <string>
2120
#include <variant>
2221
#include <vector>
2322

2423
#include "absl/status/status.h"
2524
#include "ink/brush/easing_function.h"
25+
#include "ink/brush/version.h"
2626

2727
namespace ink {
2828

@@ -679,6 +679,10 @@ absl::Status ValidateBrushBehaviorTopLevel(const BrushBehavior& behavior);
679679

680680
absl::Status ValidateBrushBehaviorNode(const BrushBehavior::Node& node);
681681

682+
// Calculates the minimum version of the Ink library that is required to use
683+
// this brush behavior.
684+
Version CalculateMinimumRequiredVersion(const BrushBehavior& behavior);
685+
682686
std::string ToFormattedString(BrushBehavior::Source source);
683687
std::string ToFormattedString(BrushBehavior::Target target);
684688
std::string ToFormattedString(BrushBehavior::PolarTarget target);

ink/brush/brush_coat.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,17 @@
1414

1515
#include "ink/brush/brush_coat.h"
1616

17+
#include <algorithm>
1718
#include <string>
1819

1920
#include "absl/container/flat_hash_set.h"
2021
#include "absl/status/status.h"
2122
#include "absl/strings/str_cat.h"
2223
#include "absl/strings/str_format.h"
2324
#include "absl/strings/str_join.h"
24-
#include "ink/brush/brush_behavior.h"
2525
#include "ink/brush/brush_paint.h"
2626
#include "ink/brush/brush_tip.h"
27+
#include "ink/brush/version.h"
2728
#include "ink/geometry/mesh_format.h"
2829

2930
namespace ink {
@@ -45,6 +46,14 @@ absl::Status ValidateBrushCoat(const BrushCoat& coat) {
4546
return absl::OkStatus();
4647
}
4748

49+
Version CalculateMinimumRequiredVersion(const BrushCoat& coat) {
50+
Version max_version = CalculateMinimumRequiredVersion(coat.tip);
51+
for (const auto& paint : coat.paint_preferences) {
52+
max_version = std::max(max_version, CalculateMinimumRequiredVersion(paint));
53+
}
54+
return max_version;
55+
}
56+
4857
void AddAttributeIdsRequiredByCoat(
4958
const BrushCoat& coat,
5059
absl::flat_hash_set<MeshFormat::AttributeId>& attribute_ids) {

ink/brush/brush_coat.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "absl/status/status.h"
2323
#include "ink/brush/brush_paint.h"
2424
#include "ink/brush/brush_tip.h"
25+
#include "ink/brush/version.h"
2526
#include "ink/geometry/mesh_format.h"
2627

2728
namespace ink {
@@ -46,6 +47,10 @@ namespace brush_internal {
4647
// BrushFamily, and returns an error if not.
4748
absl::Status ValidateBrushCoat(const BrushCoat& coat);
4849

50+
// Calculates the minimum version of the Ink library that is required to use
51+
// this brush coat.
52+
Version CalculateMinimumRequiredVersion(const BrushCoat& coat);
53+
4954
// Adds the mesh attribute IDs that are required to properly render a mesh
5055
// made with this brush coat to the given `attribute_ids` set. This will always
5156
// include `kPosition` and certain other attribute IDs (`kSideDerivative`,

0 commit comments

Comments
 (0)