Skip to content

Commit 4029f9d

Browse files
Ink Open Sourcecopybara-github
authored andcommitted
Remove opacity multiplier field from BrushTip
It is no longer needed or desirable now that we have brush paint color functions as a path-renderer-compatible way to modify whole-stroke opacity. PiperOrigin-RevId: 800487514
1 parent bf74170 commit 4029f9d

17 files changed

+41
-161
lines changed

ink/brush/brush_family_test.cc

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,14 @@ TEST(BrushFamilyTest, StringifyWithNoId) {
111111
absl::StatusOr<BrushFamily> family = BrushFamily::Create(
112112
BrushTip{.scale = {3, 3},
113113
.corner_rounding = 0,
114-
.opacity_multiplier = 0.7,
115114
.particle_gap_distance_scale = 0.1,
116115
.particle_gap_duration = Duration32::Seconds(2)},
117116
CreateTestPaint());
118117
ASSERT_EQ(family.status(), absl::OkStatus());
119118
EXPECT_EQ(absl::StrCat(*family),
120119
"BrushFamily(coats=[BrushCoat{tip=BrushTip{scale=<3, 3>, "
121-
"corner_rounding=0, opacity_multiplier=0.7, "
122-
"particle_gap_distance_scale=0.1, particle_gap_duration=2s}, "
120+
"corner_rounding=0, particle_gap_distance_scale=0.1, "
121+
"particle_gap_duration=2s}, "
123122
"paint_preferences={BrushPaint{texture_layers={TextureLayer{"
124123
"client_texture_id=test-paint, mapping=kStamping, "
125124
"origin=kStrokeSpaceOrigin, size_unit=kBrushSize, wrap_x=kRepeat, "
@@ -133,14 +132,13 @@ TEST(BrushFamilyTest, StringifyWithNoId) {
133132
}
134133

135134
TEST(BrushFamilyTest, StringifyWithId) {
136-
absl::StatusOr<BrushFamily> family = BrushFamily::Create(
137-
BrushTip{
138-
.scale = {3, 3}, .corner_rounding = 0, .opacity_multiplier = 0.7},
139-
CreateTestPaint(), "big-square");
135+
absl::StatusOr<BrushFamily> family =
136+
BrushFamily::Create(BrushTip{.scale = {3, 3}, .corner_rounding = 0},
137+
CreateTestPaint(), "big-square");
140138
ASSERT_EQ(family.status(), absl::OkStatus());
141139
EXPECT_EQ(absl::StrCat(*family),
142140
"BrushFamily(coats=[BrushCoat{tip=BrushTip{scale=<3, 3>, "
143-
"corner_rounding=0, opacity_multiplier=0.7}, "
141+
"corner_rounding=0}, "
144142
"paint_preferences={BrushPaint{texture_layers={TextureLayer{client_"
145143
"texture_id=test-paint, mapping=kStamping, "
146144
"origin=kStrokeSpaceOrigin, size_unit=kBrushSize, wrap_x=kRepeat, "
@@ -326,39 +324,6 @@ TEST(BrushFamilyTest, CreateWithInvalidTipRotation) {
326324
}
327325
}
328326

329-
TEST(BrushFamilyTest, CreateWithInvalidTipOpacityMultiplier) {
330-
{
331-
absl::Status status =
332-
BrushFamily::Create({.opacity_multiplier = -kInfinity}, {}).status();
333-
EXPECT_EQ(status.code(), kInvalidArgument);
334-
EXPECT_THAT(status.message(), HasSubstr("opacity_multiplier"));
335-
}
336-
{
337-
absl::Status status =
338-
BrushFamily::Create({.opacity_multiplier = kInfinity}, {}).status();
339-
EXPECT_EQ(status.code(), kInvalidArgument);
340-
EXPECT_THAT(status.message(), HasSubstr("opacity_multiplier"));
341-
}
342-
{
343-
absl::Status status =
344-
BrushFamily::Create({.opacity_multiplier = kNan}, {}).status();
345-
EXPECT_EQ(status.code(), kInvalidArgument);
346-
EXPECT_THAT(status.message(), HasSubstr("opacity_multiplier"));
347-
}
348-
{
349-
absl::Status status =
350-
BrushFamily::Create({.opacity_multiplier = -1}, {}).status();
351-
EXPECT_EQ(status.code(), kInvalidArgument);
352-
EXPECT_THAT(status.message(), HasSubstr("opacity_multiplier"));
353-
}
354-
{
355-
absl::Status status =
356-
BrushFamily::Create({.opacity_multiplier = 5}, {}).status();
357-
EXPECT_EQ(status.code(), kInvalidArgument);
358-
EXPECT_THAT(status.message(), HasSubstr("opacity_multiplier"));
359-
}
360-
}
361-
362327
TEST(BrushFamilyTest, CreateWithInvalidTipSlant) {
363328
{
364329
absl::Status status =

ink/brush/brush_test.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ BrushFamily CreateTestFamily() {
8686

8787
TEST(BrushTest, Stringify) {
8888
absl::StatusOr<BrushFamily> family = BrushFamily::Create(
89-
BrushTip{
90-
.scale = {3, 3}, .corner_rounding = 0, .opacity_multiplier = 0.7},
89+
BrushTip{.scale = {3, 3}, .corner_rounding = 0},
9190
{.texture_layers = {{.client_texture_id = std::string(kTestTextureId),
9291
.mapping = BrushPaint::TextureMapping::kStamping,
9392
.size_unit = BrushPaint::TextureSizeUnit::kBrushSize,
@@ -105,7 +104,7 @@ TEST(BrushTest, Stringify) {
105104
"Brush(color=Color({0.000000, 0.000000, 1.000000, 1.000000}, sRGB), "
106105
"size=3, epsilon=0.1, "
107106
"family=BrushFamily(coats=[BrushCoat{tip=BrushTip{scale=<3, 3>, "
108-
"corner_rounding=0, opacity_multiplier=0.7}, "
107+
"corner_rounding=0}, "
109108
"paint_preferences={BrushPaint{texture_layers={TextureLayer{client_"
110109
"texture_id=test-texture, mapping=kStamping, "
111110
"origin=kStrokeSpaceOrigin, size_unit=kBrushSize, wrap_x=kRepeat, "

ink/brush/brush_tip.cc

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,6 @@ absl::Status ValidateBrushTipTopLevel(const BrushTip& tip) {
6060
return absl::InvalidArgumentError(absl::StrFormat(
6161
"`BrushTip::rotation` must be finite. Got %v", tip.rotation));
6262
}
63-
if (!(tip.opacity_multiplier >= 0 && tip.opacity_multiplier <= 2)) {
64-
return absl::InvalidArgumentError(
65-
absl::StrFormat("`BrushTip::opacity_multiplier` must be a value in the "
66-
"interval [0, 2]. Got %f",
67-
tip.opacity_multiplier));
68-
}
6963
if (!std::isfinite(tip.particle_gap_distance_scale) ||
7064
tip.particle_gap_distance_scale < 0) {
7165
return absl::InvalidArgumentError(
@@ -107,10 +101,6 @@ std::string ToFormattedString(const BrushTip& tip) {
107101
if (tip.rotation != Angle()) {
108102
absl::StrAppend(&formatted, ", rotation=", tip.rotation);
109103
}
110-
if (tip.opacity_multiplier != 1.f) {
111-
absl::StrAppend(&formatted,
112-
", opacity_multiplier=", tip.opacity_multiplier);
113-
}
114104
if (tip.particle_gap_distance_scale != 0) {
115105
absl::StrAppend(&formatted, ", particle_gap_distance_scale=",
116106
tip.particle_gap_distance_scale);

ink/brush/brush_tip.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,6 @@ struct BrushTip {
8383
// Angle specifying the initial rotation of the tip shape after applying
8484
// `scale`, `pinch`, and `slant`.
8585
Angle rotation = Angle::Radians(0);
86-
// Scales the opacity of the base brush color for this tip, independent of
87-
// `brush_behavior`s. A possible example application is a highlighter brush.
88-
//
89-
// The multiplier must be in the range [0, 2] and the value ultimately applied
90-
// can be modified by applicable `brush_behavior`s.
91-
float opacity_multiplier = 1.f;
9286
// Parameter controlling emission of particles as a function of distance
9387
// traveled by the stroke inputs. The value must be finite and non-negative.
9488
//

ink/brush/brush_tip_test.cc

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,13 @@ TEST(BrushTipTest, Stringify) {
3232
.slant = Angle::Degrees(45),
3333
.pinch = 0.75f,
3434
.rotation = Angle::Degrees(90),
35-
.opacity_multiplier = 0.7f,
3635
}),
3736
"BrushTip{scale=<0.5, 2>, corner_rounding=0.25, slant=0.25π, "
38-
"pinch=0.75, rotation=0.5π, opacity_multiplier=0.7}");
37+
"pinch=0.75, rotation=0.5π}");
3938
EXPECT_EQ(
4039
absl::StrCat(BrushTip{
4140
.scale = {1.25f, 0.75f},
4241
.corner_rounding = 0.f,
43-
.opacity_multiplier = 0.7f,
4442
.behaviors =
4543
{
4644
BrushBehavior{{
@@ -65,7 +63,7 @@ TEST(BrushTipTest, Stringify) {
6563
}},
6664
},
6765
}),
68-
"BrushTip{scale=<1.25, 0.75>, corner_rounding=0, opacity_multiplier=0.7, "
66+
"BrushTip{scale=<1.25, 0.75>, corner_rounding=0, "
6967
"behaviors={BrushBehavior{nodes={SourceNode{source=kTimeOfInputInMillis, "
7068
"source_value_range={0, 250}}, TargetNode{target=kWidthMultiplier, "
7169
"target_modifier_range={1.5, 2}}}}, "
@@ -81,7 +79,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
8179
.slant = Angle::Degrees(45),
8280
.pinch = 0.75f,
8381
.rotation = Angle::Degrees(90),
84-
.opacity_multiplier = 0.7f,
8582
.particle_gap_distance_scale = 0.5f,
8683
.particle_gap_duration = Duration32::Seconds(0.5),
8784
.behaviors = {
@@ -105,7 +102,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
105102
.slant = Angle::Degrees(45),
106103
.pinch = 0.75f,
107104
.rotation = Angle::Degrees(90),
108-
.opacity_multiplier = 0.7f,
109105
.particle_gap_distance_scale = 0.5f,
110106
.particle_gap_duration = Duration32::Seconds(0.5),
111107
.behaviors = {
@@ -127,7 +123,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
127123
.slant = Angle::Degrees(45),
128124
.pinch = 0.75f,
129125
.rotation = Angle::Degrees(90),
130-
.opacity_multiplier = 0.7f,
131126
.particle_gap_distance_scale = 0.5f,
132127
.particle_gap_duration = Duration32::Seconds(0.5),
133128
.behaviors = {BrushBehavior{{
@@ -148,7 +143,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
148143
.slant = Angle::Degrees(45),
149144
.pinch = 0.75f,
150145
.rotation = Angle::Degrees(90),
151-
.opacity_multiplier = 0.7f,
152146
.particle_gap_distance_scale = 0.5f,
153147
.particle_gap_duration = Duration32::Seconds(0.5),
154148
.behaviors = {
@@ -171,7 +165,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
171165
.slant = Angle::Degrees(33), // Modified
172166
.pinch = 0.75f,
173167
.rotation = Angle::Degrees(90),
174-
.opacity_multiplier = 0.7f,
175168
.particle_gap_distance_scale = 0.5f,
176169
.particle_gap_duration = Duration32::Seconds(0.5),
177170
.behaviors = {
@@ -194,7 +187,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
194187
.slant = Angle::Degrees(45),
195188
.pinch = 0.88f, // Modified
196189
.rotation = Angle::Degrees(90),
197-
.opacity_multiplier = 0.7f,
198190
.particle_gap_distance_scale = 0.5f,
199191
.particle_gap_duration = Duration32::Seconds(0.5),
200192
.behaviors = {
@@ -217,7 +209,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
217209
.slant = Angle::Degrees(45),
218210
.pinch = 0.75f,
219211
.rotation = Angle::Degrees(22), // Modified
220-
.opacity_multiplier = 0.7f,
221212
.particle_gap_distance_scale = 0.5f,
222213
.particle_gap_duration = Duration32::Seconds(0.5),
223214
.behaviors = {
@@ -240,30 +231,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
240231
.slant = Angle::Degrees(45),
241232
.pinch = 0.75f,
242233
.rotation = Angle::Degrees(90),
243-
.opacity_multiplier = 0.9f, // Modified
244-
.particle_gap_distance_scale = 0.5f,
245-
.particle_gap_duration = Duration32::Seconds(0.5),
246-
.behaviors = {
247-
BrushBehavior{{
248-
BrushBehavior::SourceNode{
249-
.source = BrushBehavior::Source::kTimeOfInputInMillis,
250-
.source_value_range = {0, 250},
251-
},
252-
BrushBehavior::TargetNode{
253-
.target = BrushBehavior::Target::kWidthMultiplier,
254-
.target_modifier_range = {1.5, 2},
255-
},
256-
}},
257-
}}));
258-
EXPECT_NE(
259-
brush_tip,
260-
BrushTip(
261-
{.scale = {1.25f, 0.75f},
262-
.corner_rounding = 0.25f,
263-
.slant = Angle::Degrees(45),
264-
.pinch = 0.75f,
265-
.rotation = Angle::Degrees(90),
266-
.opacity_multiplier = 0.7f,
267234
.particle_gap_distance_scale = 0, // Modified
268235
.particle_gap_duration = Duration32::Seconds(0.5),
269236
.behaviors = {
@@ -286,7 +253,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
286253
.slant = Angle::Degrees(45),
287254
.pinch = 0.75f,
288255
.rotation = Angle::Degrees(90),
289-
.opacity_multiplier = 0.7f,
290256
.particle_gap_distance_scale = 0.5f,
291257
.particle_gap_duration = Duration32::Zero(), // Modified
292258
.behaviors = {
@@ -309,7 +275,6 @@ TEST(BrushTipTest, EqualAndNotEqual) {
309275
.slant = Angle::Degrees(45),
310276
.pinch = 0.75f,
311277
.rotation = Angle::Degrees(90),
312-
.opacity_multiplier = 0.7f,
313278
.particle_gap_distance_scale = 0.5f,
314279
.particle_gap_duration = Duration32::Seconds(0.5),
315280
// Modified:

ink/brush/fuzz_domains.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -713,9 +713,8 @@ Domain<BrushTip> ValidBrushTip(DomainVariant variant) {
713713
Filter([](Vec scale) { return scale != Vec(); },
714714
StructOf<Vec>(FiniteNonNegativeFloat(), FiniteNonNegativeFloat())),
715715
InRange<float>(0.f, 1.f), AngleInRange(-kQuarterTurn, kQuarterTurn),
716-
InRange<float>(0.f, 1.f), FiniteAngle(), InRange<float>(0.f, 2.f),
717-
FiniteNonNegativeFloat(), FiniteNonNegativeDuration32(),
718-
VectorOf(ValidBrushBehavior(variant)));
716+
InRange<float>(0.f, 1.f), FiniteAngle(), FiniteNonNegativeFloat(),
717+
FiniteNonNegativeDuration32(), VectorOf(ValidBrushBehavior(variant)));
719718
}
720719

721720
} // namespace

ink/brush/internal/jni/brush_tip_jni.cc

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ extern "C" {
5050
JNI_METHOD(brush, BrushTipNative, jlong, create)
5151
(JNIEnv* env, jobject thiz, jfloat scale_x, jfloat scale_y,
5252
jfloat corner_rounding, jfloat slant_radians, jfloat pinch,
53-
jfloat rotation_radians, jfloat opacity_multiplier,
54-
jfloat particle_gap_distance_scale, jlong particle_gap_duration_millis,
53+
jfloat rotation_radians, jfloat particle_gap_distance_scale,
54+
jlong particle_gap_duration_millis,
5555
jlongArray behavior_native_pointers_array) {
5656
std::vector<ink::BrushBehavior> behaviors;
5757
const jsize num_behaviors =
@@ -72,7 +72,6 @@ JNI_METHOD(brush, BrushTipNative, jlong, create)
7272
.slant = ink::Angle::Radians(slant_radians),
7373
.pinch = pinch,
7474
.rotation = ink::Angle::Radians(rotation_radians),
75-
.opacity_multiplier = opacity_multiplier,
7675
.particle_gap_distance_scale = particle_gap_distance_scale,
7776
.particle_gap_duration = Duration32::Millis(particle_gap_duration_millis),
7877
.behaviors = behaviors};
@@ -118,11 +117,6 @@ JNI_METHOD(brush, BrushTipNative, jfloat, getRotationRadians)
118117
return CastToBrushTip(native_pointer).rotation.ValueInRadians();
119118
}
120119

121-
JNI_METHOD(brush, BrushTipNative, jfloat, getOpacityMultiplier)
122-
(JNIEnv* env, jobject thiz, jlong native_pointer) {
123-
return CastToBrushTip(native_pointer).opacity_multiplier;
124-
}
125-
126120
JNI_METHOD(brush, BrushTipNative, jfloat, getParticleGapDistanceScale)
127121
(JNIEnv* env, jobject thiz, jlong native_pointer) {
128122
return CastToBrushTip(native_pointer).particle_gap_distance_scale;

ink/brush/type_matchers.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,6 @@ MATCHER_P(BrushTipEqMatcher, expected,
285285
Field("slant", &BrushTip::slant, AngleEq(expected.slant)),
286286
Field("pinch", &BrushTip::pinch, FloatEq(expected.pinch)),
287287
Field("rotation", &BrushTip::rotation, AngleEq(expected.rotation)),
288-
Field("opacity_multiplier", &BrushTip::opacity_multiplier,
289-
FloatEq(expected.opacity_multiplier)),
290288
Field("particle_gap_distance_scale",
291289
&BrushTip::particle_gap_distance_scale,
292290
FloatEq(expected.particle_gap_distance_scale)),

ink/rendering/skia/native/internal/path_drawable.cc

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,8 @@ void SetPaintDefaultsForPath(SkPaint& paint) {
9393
PathDrawable::PathDrawable(
9494
const MutableMesh& mesh,
9595
absl::Span<const absl::Span<const uint32_t>> index_outlines,
96-
absl::Span<const ColorFunction> color_functions, float opacity_multiplier)
97-
: color_functions_(color_functions.begin(), color_functions.end()),
98-
opacity_multiplier_(opacity_multiplier) {
96+
absl::Span<const ColorFunction> color_functions)
97+
: color_functions_(color_functions.begin(), color_functions.end()) {
9998
for (absl::Span<const uint32_t> indices : index_outlines) {
10099
if (indices.empty()) continue;
101100

@@ -106,10 +105,8 @@ PathDrawable::PathDrawable(
106105

107106
PathDrawable::PathDrawable(const PartitionedMesh& shape,
108107
uint32_t render_group_index,
109-
absl::Span<const ColorFunction> color_functions,
110-
float opacity_multiplier)
111-
: color_functions_(color_functions.begin(), color_functions.end()),
112-
opacity_multiplier_(opacity_multiplier) {
108+
absl::Span<const ColorFunction> color_functions)
109+
: color_functions_(color_functions.begin(), color_functions.end()) {
113110
absl::Span<const Mesh> mesh_group =
114111
shape.RenderGroupMeshes(render_group_index);
115112
for (uint32_t i = 0; i < shape.OutlineCount(render_group_index); ++i) {
@@ -127,9 +124,8 @@ void PathDrawable::SetBrushColor(const Color& brush_color) {
127124
.InColorSpace(ColorSpace::kSrgb)
128125
.AsFloat(Color::Format::kLinear);
129126
sk_sp<SkColorSpace> srgb_linear = SkColorSpace::MakeSRGBLinear();
130-
paint_.setColor(
131-
{.fR = c.r, .fG = c.g, .fB = c.b, .fA = c.a * opacity_multiplier_},
132-
srgb_linear.get());
127+
paint_.setColor({.fR = c.r, .fG = c.g, .fB = c.b, .fA = c.a},
128+
srgb_linear.get());
133129
}
134130

135131
void PathDrawable::SetImageFilter(sk_sp<SkImageFilter> image_filter) {

ink/rendering/skia/native/internal/path_drawable.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,11 @@ class PathDrawable {
4444
// function will need to change to accept a `span<const MutableMesh>`.
4545
PathDrawable(const MutableMesh& mesh,
4646
absl::Span<const absl::Span<const uint32_t>> index_outlines,
47-
absl::Span<const ColorFunction> color_functions,
48-
float opacity_multiplier);
47+
absl::Span<const ColorFunction> color_functions);
4948

5049
// Constructs the drawable from one render group of a `PartitionedMesh`.
5150
PathDrawable(const PartitionedMesh& shape, uint32_t render_group_index,
52-
absl::Span<const ColorFunction> color_functions,
53-
float opacity_multiplier);
51+
absl::Span<const ColorFunction> color_functions);
5452

5553
PathDrawable() = default;
5654
PathDrawable(const PathDrawable&) = default;
@@ -60,9 +58,8 @@ class PathDrawable {
6058
~PathDrawable() = default;
6159

6260
// Sets the brush color to be used for this drawable. The passed-in
63-
// `brush_color` will be transformed by the `color_functions` and
64-
// `opacity_multiplier` passed in during construction before being applied to
65-
// the underlying `SkPaint`.
61+
// `brush_color` will be transformed by the `color_functions` passed in during
62+
// construction before being applied to the underlying `SkPaint`.
6663
void SetBrushColor(const Color& brush_color);
6764

6865
void SetImageFilter(sk_sp<SkImageFilter> image_filter);
@@ -73,7 +70,6 @@ class PathDrawable {
7370
absl::InlinedVector<SkPath, 1> paths_;
7471
SkPaint paint_;
7572
absl::InlinedVector<ColorFunction, 1> color_functions_;
76-
float opacity_multiplier_;
7773
};
7874

7975
} // namespace ink::skia_native_internal

0 commit comments

Comments
 (0)