Skip to content

Commit

Permalink
Rename kPi to kHalfTurn, and similarly kTwoPi and kHalfPi
Browse files Browse the repository at this point in the history 
The primary reason for doing this is for naming consistency with our [Jetpack API](https://android.googlesource.com/platform/frameworks/support/+/c40905d87f290be25cdc25454f5dd46ff47e4613/ink/ink-geometry/src/jvmAndroidMain/kotlin/androidx/ink/geometry/Angle.kt#66).

The other reason is principle: the `Angle` type is designed to abstract away its units--that is, an `Angle` object is an opaque representation of an angle, and you have to call a method if you want to get a representation of that angle as e.g. a number of radians or a number of degrees--so it doesn't really make sense to have a constant of type `Angle` named `kPi`.  It would make more sense to name it something like `kPiRadians` or `k180Degrees` or `kHalfTurn`.  This CL adopts the last of these names (again, for consistency with Jetpack).

An additional benefit: renaming `::ink::kPi` (which is an `Angle`) removes the ambiguity with `::ink::numbers::kPi` (which is a `double`).

PiperOrigin-RevId: 684044741
  • Loading branch information
@copybara-github
Ink Open Source authored and copybara-github committed Oct 9, 2024
1 parent e53a520 commit fcac605
Show file tree
Hide file tree
Showing 40 changed files with 825 additions and 771 deletions.
10 changes: 6 additions & 4 deletions ink/brush/brush_family_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ BrushTip CreatePressureTestTip() {
return {
.scale = {2, 1},
.corner_rounding = 0.75,
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.behaviors = {BrushBehavior{{
BrushBehavior::SourceNode{
.source = BrushBehavior::Source::kNormalizedPressure,
Expand Down Expand Up @@ -103,7 +103,7 @@ BrushPaint CreateTestPaint() {
.size_unit = BrushPaint::TextureSizeUnit::kBrushSize,
.size = {3, 5},
.size_jitter = {0.1, 2},
.keyframes = {{.progress = 0.1, .rotation = kHalfPi / 2}},
.keyframes = {{.progress = 0.1, .rotation = kFullTurn / 8}},
.blend_mode = BrushPaint::BlendMode::kDstIn}}};
}

Expand Down Expand Up @@ -393,12 +393,14 @@ TEST(BrushFamilyTest, CreateWithInvalidTipSlant) {
EXPECT_THAT(status.message(), HasSubstr("slant"));
}
{
absl::Status status = BrushFamily::Create({.slant = -kPi}, {}).status();
absl::Status status =
BrushFamily::Create({.slant = -kHalfTurn}, {}).status();
EXPECT_EQ(status.code(), kInvalidArgument);
EXPECT_THAT(status.message(), HasSubstr("slant"));
}
{
absl::Status status = BrushFamily::Create({.slant = kPi}, {}).status();
absl::Status status =
BrushFamily::Create({.slant = kHalfTurn}, {}).status();
EXPECT_EQ(status.code(), kInvalidArgument);
EXPECT_THAT(status.message(), HasSubstr("slant"));
}
Expand Down
49 changes: 25 additions & 24 deletions ink/brush/brush_paint_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ TEST(BrushPaintTest, TextureKeyframeSupportsAbslHash) {
BrushPaint::TextureKeyframe{.progress = 1},
BrushPaint::TextureKeyframe{.progress = 0, .size = Vec{1, 1}},
BrushPaint::TextureKeyframe{.progress = 0, .offset = Vec{1, 1}},
BrushPaint::TextureKeyframe{.progress = 0, .rotation = kPi},
BrushPaint::TextureKeyframe{.progress = 0, .rotation = kHalfTurn},
BrushPaint::TextureKeyframe{.progress = 0, .opacity = 0.5},
}));
}
Expand All @@ -71,13 +71,14 @@ TEST(BrushPaintTest, TextureLayerSupportsAbslHash) {
.size_unit = BrushPaint::TextureSizeUnit::kStrokeSize},
BrushPaint::TextureLayer{.color_texture_uri = *uri1, .size = {2, 2}},
BrushPaint::TextureLayer{.color_texture_uri = *uri1, .offset = {1, 1}},
BrushPaint::TextureLayer{.color_texture_uri = *uri1, .rotation = kPi},
BrushPaint::TextureLayer{.color_texture_uri = *uri1,
.rotation = kHalfTurn},
BrushPaint::TextureLayer{.color_texture_uri = *uri1,
.size_jitter = {2, 2}},
BrushPaint::TextureLayer{.color_texture_uri = *uri1,
.offset_jitter = {1, 1}},
BrushPaint::TextureLayer{.color_texture_uri = *uri1,
.rotation_jitter = kPi},
.rotation_jitter = kHalfTurn},
BrushPaint::TextureLayer{.color_texture_uri = *uri1, .opacity = 0.5},
BrushPaint::TextureLayer{.color_texture_uri = *uri1,
.keyframes = {{.progress = 1}}},
Expand All @@ -103,7 +104,7 @@ TEST(BrushPaintTest, TextureKeyframeEqualAndNotEqual) {
.progress = 1,
.size = Vec{2, 2},
.offset = Vec{1, 1},
.rotation = kPi,
.rotation = kHalfTurn,
.opacity = 0.5,
};

Expand Down Expand Up @@ -180,7 +181,7 @@ TEST(BrushPaintTest, TextureLayerEqualAndNotEqual) {
EXPECT_NE(layer, other);

other = layer;
other.rotation = kPi;
other.rotation = kHalfTurn;
EXPECT_NE(layer, other);

other = layer;
Expand All @@ -192,7 +193,7 @@ TEST(BrushPaintTest, TextureLayerEqualAndNotEqual) {
EXPECT_NE(layer, other);

other = layer;
other.rotation_jitter = kPi;
other.rotation_jitter = kHalfTurn;
EXPECT_NE(layer, other);

other = layer;
Expand Down Expand Up @@ -289,14 +290,14 @@ TEST(BrushPaintTest, StringifyTextureKeyFrame) {
.progress = 0.3,
.size = std::optional<Vec>({4, 6}),
.offset = std::optional<Vec>({2, 0.2}),
.rotation = kHalfPi}),
.rotation = kQuarterTurn}),
"TextureKeyframe{progress=0.3, size=<4, 6>, offset=<2, 0.2>, "
"rotation=0.5π}");
EXPECT_EQ(absl::StrCat(BrushPaint::TextureKeyframe{
.progress = 0.3,
.size = std::optional<Vec>({4, 6}),
.offset = std::optional<Vec>({2, 0.2}),
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.opacity = 0.6}),
"TextureKeyframe{progress=0.3, size=<4, 6>, offset=<2, 0.2>, "
"rotation=0.5π, opacity=0.6}");
Expand Down Expand Up @@ -330,14 +331,14 @@ TEST(BrushPaintTest, StringifyTextureLayer) {
.size_unit = BrushPaint::TextureSizeUnit::kBrushSize,
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.size_jitter = {0.1, 0.2},
.offset_jitter = {0.7, 0.3},
.rotation_jitter = kPi / 8,
.rotation_jitter = kFullTurn / 16,
.opacity = 0.6,
.keyframes = {{.progress = 0.2,
.size = std::optional<Vec>({2, 5}),
.rotation = kPi / 8}},
.rotation = kFullTurn / 16}},
.blend_mode = BrushPaint::BlendMode::kDstIn}),
"TextureLayer{color_texture_uri=/texture:test-texture, "
"mapping=kWinding, origin=kFirstStrokeInput, size_unit=kBrushSize, "
Expand All @@ -353,14 +354,14 @@ TEST(BrushPaintTest, StringifyTextureLayer) {
.size_unit = BrushPaint::TextureSizeUnit::kBrushSize,
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.size_jitter = {0.1, 0.2},
.offset_jitter = {0.7, 0.3},
.rotation_jitter = kPi / 8,
.rotation_jitter = kFullTurn / 16,
.opacity = 0.6,
.keyframes = {{.progress = 0.2,
.size = std::optional<Vec>({2, 5}),
.rotation = kPi / 8},
.rotation = kFullTurn / 16},
{.progress = 0.4,
.offset = std::optional<Vec>({2, 0.2}),
.opacity = 0.4}},
Expand Down Expand Up @@ -440,7 +441,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.opacity = 0.6}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
"texture:test-texture, mapping=kTiling, origin=kStrokeSpaceOrigin, "
Expand Down Expand Up @@ -469,7 +470,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
BrushPaint::TextureSizeUnit::kBrushSize,
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.opacity = 0.6}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
"texture:test-texture, mapping=kWinding, origin=kStrokeSpaceOrigin, "
Expand All @@ -484,10 +485,10 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
BrushPaint::TextureSizeUnit::kBrushSize,
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.size_jitter = {0.1, 0.2},
.offset_jitter = {0.7, 0.3},
.rotation_jitter = kPi / 8,
.rotation_jitter = kFullTurn / 16,
.opacity = 0.6,
.blend_mode = BrushPaint::BlendMode::kSrcIn}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
Expand All @@ -504,15 +505,15 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
.size_unit = BrushPaint::TextureSizeUnit::kBrushSize,
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.size_jitter = {0.1, 0.2},
.offset_jitter = {0.7, 0.3},
.rotation_jitter = kPi / 8,
.rotation_jitter = kFullTurn / 16,
.opacity = 0.6,
.keyframes = {{.progress = 0.3,
.size = std::optional<Vec>({4, 6}),
.offset = std::optional<Vec>({2, 0.2}),
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.opacity = 0.6}}}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
"texture:test-texture, mapping=kWinding, origin=kStrokeSpaceOrigin, "
Expand All @@ -529,10 +530,10 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
.size_unit = BrushPaint::TextureSizeUnit::kBrushSize,
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.rotation = kQuarterTurn,
.size_jitter = {0.1, 0.2},
.offset_jitter = {0.7, 0.3},
.rotation_jitter = kPi / 8,
.rotation_jitter = kFullTurn / 16,
.opacity = 0.6,
.blend_mode = BrushPaint::BlendMode::kSrcIn},
{.color_texture_uri = CreateTestTextureUri(),
Expand All @@ -542,7 +543,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
.opacity = 0.7,
.keyframes = {{.progress = 0.2,
.size = std::optional<Vec>({2, 5}),
.rotation = kPi / 8},
.rotation = kFullTurn / 16},
{.progress = 0.4,
.offset = std::optional<Vec>({2, 0.2}),
.opacity = 0.4}},
Expand Down
8 changes: 4 additions & 4 deletions ink/brush/brush_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ BrushFamily CreateTestFamily() {
{
.scale = {0.5, 1},
.corner_rounding = 0.3,
.rotation = kPi / 4,
.rotation = kFullTurn / 8,
.behaviors = {BrushBehavior{{
BrushBehavior::SourceNode{
.source = BrushBehavior::Source::kNormalizedPressure,
Expand Down Expand Up @@ -81,7 +81,7 @@ BrushFamily CreateTestFamily() {
.size = {3, 5},
.size_jitter = {0.1, 2},
.keyframes = {{.progress = 0.1,
.rotation = kHalfPi / 2}},
.rotation = kFullTurn / 8}},
.blend_mode = BrushPaint::BlendMode::kDstIn}}},
"/brush-family:test-family");
ABSL_CHECK_OK(family);
Expand All @@ -98,7 +98,7 @@ TEST(BrushTest, Stringify) {
.size = {3, 5},
.size_jitter = {0.1, 2},
.keyframes = {{.progress = 0.1,
.rotation = kHalfPi / 2}},
.rotation = kFullTurn / 8}},
.blend_mode = BrushPaint::BlendMode::kDstOut}}},
"/brush-family:big-square");
ASSERT_EQ(family.status(), absl::OkStatus());
Expand Down Expand Up @@ -245,7 +245,7 @@ TEST(BrushTest, SetNewFamily) {
.size = {3, 5},
.size_jitter = {0.1, 2},
.keyframes = {{.progress = 0.1,
.rotation = kHalfPi / 2}},
.rotation = kFullTurn / 8}},
.blend_mode = BrushPaint::BlendMode::kDstIn}}},
"/brush-family:new-test-family");
ASSERT_EQ(absl::OkStatus(), new_family.status());
Expand Down
6 changes: 3 additions & 3 deletions ink/brush/brush_tip.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ absl::Status ValidateBrushTip(const BrushTip& tip) {
}

if (!std::isfinite(tip.slant.ValueInRadians()) ||
!(tip.slant >= -kHalfPi && tip.slant <= kHalfPi)) {
!(tip.slant >= -kQuarterTurn && tip.slant <= kQuarterTurn)) {
return absl::InvalidArgumentError(
absl::StrFormat("`BrushTip::slant` must be finite and a value in the "
"interval [-pi/2, pi/2]. Got %v",
absl::StrFormat("`BrushTip::slant` must be a finite value in the "
"interval [-π/2, π/2] radians. Got %v",
tip.slant));
}
if (!(tip.pinch >= 0 && tip.pinch <= 1)) {
Expand Down
2 changes: 1 addition & 1 deletion ink/brush/fuzz_domains.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ Domain<BrushTip> ValidBrushTip() {
// non-negative, and cannot both be zero.
Filter([](Vec scale) { return scale != Vec(); },
StructOf<Vec>(FiniteNonNegativeFloat(), FiniteNonNegativeFloat())),
InRange<float>(0.f, 1.f), AngleInRange(-kHalfPi, kHalfPi),
InRange<float>(0.f, 1.f), AngleInRange(-kQuarterTurn, kQuarterTurn),
InRange<float>(0.f, 1.f), FiniteAngle(), InRange<float>(0.f, 2.f),
FiniteNonNegativeFloat(), FiniteNonNegativeDuration32(),
VectorOf(ValidBrushBehavior()));
Expand Down
Loading

0 comments on commit fcac605

Please sign in to comment.