Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BrushPaint::opacity field #35

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions ink/brush/brush_paint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ absl::Status ValidateBrushPaint(const BrushPaint& paint) {
return status;
}
}
if (!(paint.opacity >= 0.0f && paint.opacity <= 1.0f)) {
return absl::InvalidArgumentError(absl::StrFormat(
"`BrushPaint::opacity` must be in the interval [0, 1]. Got %v",
paint.opacity));
}
return absl::OkStatus();
}

Expand Down Expand Up @@ -318,9 +323,9 @@ std::string ToFormattedString(const BrushPaint::TextureLayer& texture_layer) {
}

std::string ToFormattedString(const BrushPaint& paint) {
std::string formatted =
absl::StrCat("BrushPaint{texture_layers={",
absl::StrJoin(paint.texture_layers, ", "), "}}");
std::string formatted = absl::StrCat(
"BrushPaint{texture_layers={", absl::StrJoin(paint.texture_layers, ", "),
"}, opacity=", paint.opacity, "}");

return formatted;
}
Expand Down
9 changes: 7 additions & 2 deletions ink/brush/brush_paint.h
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,11 @@ struct BrushPaint {
};

std::vector<TextureLayer> texture_layers;
// Overall paint opacity, which is applied after texture layer blending with
// the brush color. This allows for a static opacity multiplier that (unlike a
// `BrushTip` opacity behavior) can be applied even by a path-based renderer
// that doesn't support per-vertex opacity.
float opacity = 1;
};

bool operator==(const BrushPaint::TextureKeyframe& lhs,
Expand Down Expand Up @@ -326,7 +331,7 @@ H AbslHashValue(H h, const BrushPaint::TextureLayer& layer) {

template <typename H>
H AbslHashValue(H h, const BrushPaint& paint) {
return H::combine(std::move(h), paint.texture_layers);
return H::combine(std::move(h), paint.texture_layers, paint.opacity);
}

inline bool operator!=(const BrushPaint::TextureKeyframe& lhs,
Expand All @@ -340,7 +345,7 @@ inline bool operator!=(const BrushPaint::TextureLayer& lhs,
}

inline bool operator==(const BrushPaint& lhs, const BrushPaint& rhs) {
return lhs.texture_layers == rhs.texture_layers;
return lhs.texture_layers == rhs.texture_layers && lhs.opacity == rhs.opacity;
}

inline bool operator!=(const BrushPaint& lhs, const BrushPaint& rhs) {
Expand Down
86 changes: 51 additions & 35 deletions ink/brush/brush_paint_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -375,14 +375,15 @@ TEST(BrushPaintTest, StringifyTextureLayer) {
}

TEST(BrushPaintTest, StringifyBrushPaint) {
EXPECT_EQ(absl::StrCat(BrushPaint{}), "BrushPaint{texture_layers={}}");
EXPECT_EQ(absl::StrCat(BrushPaint{}),
"BrushPaint{texture_layers={}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{.texture_layers = {{}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=, "
"mapping=kTiling, origin=kStrokeSpaceOrigin, "
"size_unit=kStrokeCoordinates, size=<1, 1>, offset=<0, 0>, "
"rotation=0π, size_jitter=<0, 0>, offset_jitter=<0, 0>, "
"rotation_jitter=0π, opacity=1, keyframes={}, blend_mode=kModulate}}}");
"size_unit=kStrokeCoordinates, size=<1, 1>, offset=<0, 0>, rotation=0π, "
"size_jitter=<0, 0>, offset_jitter=<0, 0>, rotation_jitter=0π, "
"opacity=1, keyframes={}, blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri()}}}),
Expand All @@ -392,7 +393,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"size=<1, 1>, offset=<0, 0>, rotation=0π, "
"size_jitter=<0, 0>, "
"offset_jitter=<0, 0>, rotation_jitter=0π, opacity=1, keyframes={}, "
"blend_mode=kModulate}}}");
"blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
Expand All @@ -403,7 +404,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"texture:test-texture, mapping=kWinding, origin=kStrokeSpaceOrigin, "
"size_unit=kBrushSize, size=<1, 1>, offset=<0, 0>, rotation=0π, "
"size_jitter=<0, 0>, offset_jitter=<0, 0>, rotation_jitter=0π, "
"opacity=1, keyframes={}, blend_mode=kModulate}}}");
"opacity=1, keyframes={}, blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
Expand All @@ -415,7 +416,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"texture:test-texture, mapping=kWinding, origin=kStrokeSpaceOrigin, "
"size_unit=kBrushSize, size=<3, 5>, offset=<0, 0>, rotation=0π, "
"size_jitter=<0, 0>, offset_jitter=<0, 0>, rotation_jitter=0π, "
"opacity=1, keyframes={}, blend_mode=kModulate}}}");
"opacity=1, keyframes={}, blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
Expand All @@ -424,29 +425,29 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"texture:test-texture, mapping=kTiling, origin=kStrokeSpaceOrigin, "
"size_unit=kStrokeCoordinates, size=<3, 5>, offset=<0, 0>, rotation=0π, "
"size_jitter=<0, 0>, offset_jitter=<0, 0>, rotation_jitter=0π, "
"opacity=1, keyframes={}, blend_mode=kModulate}}}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
.size = {3, 5},
.offset = {2, 0.2}}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
"texture:test-texture, mapping=kTiling, origin=kStrokeSpaceOrigin, "
"size_unit=kStrokeCoordinates, size=<3, 5>, offset=<2, 0.2>, "
"rotation=0π, size_jitter=<0, 0>, offset_jitter=<0, 0>, "
"rotation_jitter=0π, opacity=1, keyframes={}, blend_mode=kModulate}}}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.opacity = 0.6}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
"texture:test-texture, mapping=kTiling, origin=kStrokeSpaceOrigin, "
"size_unit=kStrokeCoordinates, size=<3, 5>, offset=<2, 0.2>, "
"rotation=0.5π, size_jitter=<0, 0>, offset_jitter=<0, 0>, "
"rotation_jitter=0π, opacity=0.6, keyframes={}, blend_mode=kModulate}}}");
"opacity=1, keyframes={}, blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
.size = {3, 5},
.offset = {2, 0.2}}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
"texture:test-texture, mapping=kTiling, origin=kStrokeSpaceOrigin, "
"size_unit=kStrokeCoordinates, size=<3, 5>, offset=<2, 0.2>, "
"rotation=0π, size_jitter=<0, 0>, offset_jitter=<0, 0>, "
"rotation_jitter=0π, opacity=1, keyframes={}, "
"blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
.size = {3, 5},
.offset = {2, 0.2},
.rotation = kHalfPi,
.opacity = 0.6}}}),
"BrushPaint{texture_layers={TextureLayer{color_texture_uri=/"
"texture:test-texture, mapping=kTiling, origin=kStrokeSpaceOrigin, "
"size_unit=kStrokeCoordinates, size=<3, 5>, offset=<2, 0.2>, "
"rotation=0.5π, size_jitter=<0, 0>, offset_jitter=<0, 0>, "
"rotation_jitter=0π, opacity=0.6, keyframes={}, "
"blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
Expand All @@ -460,7 +461,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"texture:test-texture, mapping=kWinding, origin=kStrokeSpaceOrigin, "
"size_unit=kBrushSize, size=<3, 5>, offset=<2, 0.2>, rotation=0π, "
"size_jitter=<0, 0>, offset_jitter=<0, 0>, rotation_jitter=0π, "
"opacity=1, keyframes={}, blend_mode=kSrcIn}}}");
"opacity=1, keyframes={}, blend_mode=kSrcIn}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
Expand All @@ -475,7 +476,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"texture:test-texture, mapping=kWinding, origin=kStrokeSpaceOrigin, "
"size_unit=kBrushSize, size=<3, 5>, offset=<2, 0.2>, rotation=0.5π, "
"size_jitter=<0, 0>, offset_jitter=<0, 0>, rotation_jitter=0π, "
"opacity=0.6, keyframes={}, blend_mode=kModulate}}}");
"opacity=0.6, keyframes={}, blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers = {{.color_texture_uri = CreateTestTextureUri(),
Expand All @@ -495,7 +496,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"size_unit=kBrushSize, size=<3, 5>, offset=<2, 0.2>, rotation=0.5π, "
"size_jitter=<0.1, 0.2>, offset_jitter=<0.7, 0.3>, "
"rotation_jitter=0.125π, opacity=0.6, keyframes={}, "
"blend_mode=kSrcIn}}}");
"blend_mode=kSrcIn}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers =
Expand All @@ -520,7 +521,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"size_jitter=<0.1, 0.2>, offset_jitter=<0.7, 0.3>, "
"rotation_jitter=0.125π, opacity=0.6, "
"keyframes={TextureKeyframe{progress=0.3, size=<4, 6>, offset=<2, 0.2>, "
"rotation=0.5π, opacity=0.6}}, blend_mode=kModulate}}}");
"rotation=0.5π, opacity=0.6}}, blend_mode=kModulate}}, opacity=1}");
EXPECT_EQ(
absl::StrCat(BrushPaint{
.texture_layers =
Expand Down Expand Up @@ -558,7 +559,7 @@ TEST(BrushPaintTest, StringifyBrushPaint) {
"rotation_jitter=0π, opacity=0.7, "
"keyframes={TextureKeyframe{progress=0.2, size=<2, 5>, rotation=0.125π}, "
"TextureKeyframe{progress=0.4, offset=<2, 0.2>, opacity=0.4}}, "
"blend_mode=kDstIn}}}");
"blend_mode=kDstIn}}, opacity=1}");
}

TEST(BrushPaintTest, InvalidTextureLayerRotation) {
Expand Down Expand Up @@ -589,5 +590,20 @@ TEST(BrushPaintTest, InvalidTextureLayerRotationJitter) {
EXPECT_THAT(status.message(), HasSubstr("rotation_jitter` must be finite"));
}

TEST(BrushPaintTest, InvalidPaintOpacity) {
absl::Status status =
brush_internal::ValidateBrushPaint(BrushPaint{.opacity = -0.5f});
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
EXPECT_THAT(status.message(), HasSubstr("opacity"));

status = brush_internal::ValidateBrushPaint(BrushPaint{.opacity = 1.25f});
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
EXPECT_THAT(status.message(), HasSubstr("opacity"));

status = brush_internal::ValidateBrushPaint(BrushPaint{.opacity = kNan});
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
EXPECT_THAT(status.message(), HasSubstr("opacity"));
}

} // namespace
} // namespace ink
4 changes: 2 additions & 2 deletions ink/brush/fuzz_domains.cc
Original file line number Diff line number Diff line change
Expand Up @@ -595,8 +595,8 @@ fuzztest::Domain<BrushPaint::TextureLayer> ValidBrushPaintTextureLayer() {
}

fuzztest::Domain<BrushPaint> ValidBrushPaint() {
return fuzztest::StructOf<BrushPaint>(
VectorOf(ValidBrushPaintTextureLayer()));
return fuzztest::StructOf<BrushPaint>(VectorOf(ValidBrushPaintTextureLayer()),
InRange<float>(0.f, 1.f));
}

Domain<BrushTip> ValidBrushTip() {
Expand Down
14 changes: 11 additions & 3 deletions ink/brush/internal/jni/brush_paint_jni.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#include <jni.h>

#include <memory>
#include <utility>
#include <vector>

#include "absl/status/status.h"
Expand Down Expand Up @@ -46,11 +48,17 @@ extern "C" {

// Construct a native BrushPaint and return a pointer to it as a long.
JNI_METHOD(brush, BrushPaint, jlong, nativeCreateBrushPaint)
(JNIEnv* env, jobject thiz, jint texture_layers_count) {
(JNIEnv* env, jobject thiz, jint texture_layers_count, jfloat opacity) {
std::vector<ink::BrushPaint::TextureLayer> texture_layers;
texture_layers.reserve(texture_layers_count);
return reinterpret_cast<jlong>(
new ink::BrushPaint{.texture_layers = texture_layers});
auto brush_paint = std::make_unique<ink::BrushPaint>(ink::BrushPaint{
.texture_layers = std::move(texture_layers), .opacity = opacity});
absl::Status status = ink::brush_internal::ValidateBrushPaint(*brush_paint);
if (!status.ok()) {
ink::jni::ThrowExceptionFromStatus(env, status);
return 0; // This return value is ignored.
}
return reinterpret_cast<jlong>(brush_paint.release());
}

// Appends a texture layer to a *mutable* C++ BrushPaint object as referenced by
Expand Down
7 changes: 4 additions & 3 deletions ink/brush/type_matchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,10 @@ MATCHER_P(BrushPaintEqMatcher, expected,
" BrushPaint (expected: ",
::testing::PrintToString(expected), ")")) {
return ExplainMatchResult(
AllOf(Field(
"texture_layers", &BrushPaint::texture_layers,
Pointwise(BrushPaintTextureLayerEq(), expected.texture_layers))),
AllOf(
Field("texture_layers", &BrushPaint::texture_layers,
Pointwise(BrushPaintTextureLayerEq(), expected.texture_layers)),
Field("opacity", &BrushPaint::opacity, FloatEq(expected.opacity))),
arg, result_listener);
}

Expand Down
1 change: 1 addition & 0 deletions ink/rendering/skia/native/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ cc_library(
],
deps = [
"//ink/brush",
"//ink/brush:brush_coat",
"//ink/brush:brush_paint",
"//ink/color",
"//ink/geometry:affine_transform",
Expand Down
14 changes: 11 additions & 3 deletions ink/rendering/skia/native/skia_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "absl/strings/str_cat.h"
#include "absl/types/span.h"
#include "ink/brush/brush.h"
#include "ink/brush/brush_coat.h"
#include "ink/brush/brush_paint.h"
#include "ink/color/color.h"
#include "ink/geometry/affine_transform.h"
Expand Down Expand Up @@ -93,7 +94,12 @@ bool UsePathRendering(GrDirectContext* context, const BrushPaint&) {
// TODO: b/285594469 - Add a brush tip index parameter once a valid `BrushCoat`
// has something other than exactly one `BrushTip`.
float OpacityMultiplierForPath(const Brush& brush, uint32_t coat_index) {
return brush.GetCoats()[coat_index].tips.front().opacity_multiplier;
const BrushCoat& coat = brush.GetCoats()[coat_index];
return coat.tips.front().opacity_multiplier * coat.paint.opacity;
}

Color ApplyPaintOpacity(const BrushPaint& paint, const Color& color) {
return color.WithAlphaFloat(color.GetAlphaFloat() * paint.opacity);
}

} // namespace
Expand Down Expand Up @@ -157,7 +163,8 @@ absl::StatusOr<SkiaRenderer::Drawable> SkiaRenderer::CreateDrawable(
}});
if (!mesh_drawable.ok()) return mesh_drawable.status();

mesh_drawable->SetBrushColor(brush->GetColor());
mesh_drawable->SetBrushColor(
ApplyPaintOpacity(brush_paint, brush->GetColor()));
drawables.push_back(*std::move(mesh_drawable));
}

Expand Down Expand Up @@ -232,7 +239,8 @@ absl::StatusOr<SkiaRenderer::Drawable> SkiaRenderer::CreateDrawable(
std::move(partitions), std::move(uniform_data));
if (!mesh_drawable.ok()) return mesh_drawable.status();

mesh_drawable->SetBrushColor(brush.GetColor());
mesh_drawable->SetBrushColor(
ApplyPaintOpacity(brush_paint, brush.GetColor()));
drawables.push_back(*std::move(mesh_drawable));
}

Expand Down
Loading