Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 10 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
5 changes: 5 additions & 0 deletions flow/embedded_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ void MutatorsStack::PushTransform(const SkMatrix& matrix) {
vector_.push_back(element);
};

void MutatorsStack::PushOpacity(const OpacityParams& opacityParams) {
std::shared_ptr<Mutator> element = std::make_shared<Mutator>(opacityParams);
vector_.push_back(element);
};

void MutatorsStack::Pop() {
vector_.pop_back();
};
Expand Down
30 changes: 29 additions & 1 deletion flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,24 @@

namespace flutter {

enum MutatorType { clip_rect, clip_rrect, clip_path, transform };
enum MutatorType { clip_rect, clip_rrect, clip_path, transform, opacity };

// Stores information required for an opacity Mutator.
// Matches the members in a `OpacityLayer`.
struct OpacityParams {
std::reference_wrapper<int> alpha;
std::reference_wrapper<SkPoint> offset;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't read the rest of the PR yet, but looking at this I'm not sure what does an offset means for opacity, should probably add a comment.


bool operator==(const OpacityParams& other) const {
return alpha == other.alpha && offset == other.offset;
}

bool operator!=(const OpacityParams& other) const {
return !operator==(other);
}

float GetAlphaF() const { return (alpha / 255.0); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a class if we include such a method, see: https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes

};

// Stores mutation information like clipping or transform.
//
Expand All @@ -42,6 +59,9 @@ class Mutator {
case transform:
matrix_ = other.matrix_;
break;
case opacity:
opacityParams_ = OpacityParams{other.opacityParams_.alpha,
other.opacityParams_.offset};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing break;

default:
break;
}
Expand All @@ -53,12 +73,15 @@ class Mutator {
: type_(clip_path), path_(new SkPath(path)) {}
explicit Mutator(const SkMatrix& matrix)
: type_(transform), matrix_(matrix) {}
explicit Mutator(const OpacityParams& opacityParams)
: type_(opacity), opacityParams_(opacityParams) {}

const MutatorType& GetType() const { return type_; }
const SkRect& GetRect() const { return rect_; }
const SkRRect& GetRRect() const { return rrect_; }
const SkPath& GetPath() const { return *path_; }
const SkMatrix& GetMatrix() const { return matrix_; }
const OpacityParams& GetOpacityParams() const { return opacityParams_; }

bool operator==(const Mutator& other) const {
if (type_ != other.type_) {
Expand All @@ -73,6 +96,8 @@ class Mutator {
return *path_ == *other.path_;
case transform:
return matrix_ == other.matrix_;
case opacity:
return opacityParams_ == other.opacityParams_;
}

return false;
Expand All @@ -97,6 +122,8 @@ class Mutator {
SkRect rect_;
SkRRect rrect_;
SkMatrix matrix_;
OpacityParams opacityParams_;
int alpha_;
SkPath* path_;
};

Expand All @@ -119,6 +146,7 @@ class MutatorsStack {
void PushClipRRect(const SkRRect& rrect);
void PushClipPath(const SkPath& path);
void PushTransform(const SkMatrix& matrix);
void PushOpacity(const OpacityParams& opacityParams);

// Removes the `Mutator` on the top of the stack
// and destroys it.
Expand Down
4 changes: 4 additions & 0 deletions flow/layers/opacity_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
EnsureSingleChild();
SkMatrix child_matrix = matrix;
child_matrix.postTranslate(offset_.fX, offset_.fY);
// If any non-zero offset is applied to the matrix. Reverse the translate to
// the embedded view.
context->mutators_stack.PushOpacity(OpacityParams{alpha_, offset_});
ContainerLayer::Preroll(context, child_matrix);
context->mutators_stack.Pop();
set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY));
// See |EnsureSingleChild|.
FML_DCHECK(layers().size() == 1);
Expand Down
44 changes: 43 additions & 1 deletion flow/mutators_stack_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ TEST(MutatorsStack, PushClipRRect) {
}

TEST(MutatorsStack, PushClipPath) {
flutter::MutatorsStack stack;
MutatorsStack stack;
SkPath path;
stack.PushClipPath(path);
auto iter = stack.Bottom();
Expand All @@ -60,6 +60,16 @@ TEST(MutatorsStack, PushTransform) {
ASSERT_TRUE(iter->get()->GetMatrix() == matrix);
}

TEST(MutatorsStack, PushOpacity) {
MutatorsStack stack;
SkPoint point = SkPoint::Make(0, 0);
int alpha = 240;
stack.PushOpacity(OpacityParams{alpha, point});
auto iter = stack.Bottom();
ASSERT_TRUE(iter->get()->GetType() == MutatorType::opacity);
ASSERT_TRUE(iter->get()->GetOpacityParams().alpha == 240);
}

TEST(MutatorsStack, Pop) {
MutatorsStack stack;
SkMatrix matrix;
Expand Down Expand Up @@ -113,6 +123,9 @@ TEST(MutatorsStack, Equality) {
stack.PushClipRRect(rrect);
SkPath path;
stack.PushClipPath(path);
int alpha = 240;
SkPoint point = SkPoint::Make(0, 0);
stack.PushOpacity(OpacityParams{alpha, point});

MutatorsStack stackOther;
SkMatrix matrixOther = SkMatrix::MakeScale(1, 1);
Expand All @@ -123,6 +136,9 @@ TEST(MutatorsStack, Equality) {
stackOther.PushClipRRect(rrectOther);
SkPath otherPath;
stackOther.PushClipPath(otherPath);
int otherAlpha = 240;
SkPoint ohterPoint = SkPoint::Make(0, 0);
stackOther.PushOpacity(OpacityParams{otherAlpha, ohterPoint});

ASSERT_TRUE(stack == stackOther);
}
Expand All @@ -148,6 +164,11 @@ TEST(Mutator, Initialization) {
Mutator mutator4 = Mutator(matrix);
ASSERT_TRUE(mutator4.GetType() == MutatorType::transform);
ASSERT_TRUE(mutator4.GetMatrix() == matrix);

SkPoint point = SkPoint::Make(0, 0);
int alpha = 240;
Mutator mutator5 = Mutator(OpacityParams{alpha, point});
ASSERT_TRUE(mutator5.GetType() == MutatorType::opacity);
}

TEST(Mutator, CopyConstructor) {
Expand All @@ -171,6 +192,12 @@ TEST(Mutator, CopyConstructor) {
Mutator mutator4 = Mutator(matrix);
Mutator copy4 = Mutator(mutator4);
ASSERT_TRUE(mutator4 == copy4);

SkPoint point = SkPoint::Make(0, 0);
int alpha = 240;
Mutator mutator5 = Mutator(OpacityParams{alpha, point});
Mutator copy5 = Mutator(mutator5);
ASSERT_TRUE(mutator5 == copy5);
}

TEST(Mutator, Equality) {
Expand All @@ -195,6 +222,11 @@ TEST(Mutator, Equality) {
flutter::Mutator otherMutator4 = flutter::Mutator(path);
ASSERT_TRUE(mutator4 == otherMutator4);
ASSERT_FALSE(mutator2 == mutator);
int alpha = 240;
SkPoint point = SkPoint::Make(0, 0);
Mutator mutator5 = Mutator(OpacityParams{alpha, point});
Mutator otherMutator5 = Mutator(OpacityParams{alpha, point});
ASSERT_TRUE(mutator5 == otherMutator5);
}

TEST(Mutator, UnEquality) {
Expand All @@ -204,6 +236,16 @@ TEST(Mutator, UnEquality) {
matrix.setIdentity();
Mutator notEqualMutator = Mutator(matrix);
ASSERT_TRUE(notEqualMutator != mutator);

int alpha = 240;
int alpha2 = 241;
SkPoint point = SkPoint::Make(0, 0);
SkPoint point2 = SkPoint::Make(1, 0);
Mutator mutator2 = Mutator(OpacityParams{alpha, point});
Mutator otherMutator2 = Mutator(OpacityParams{alpha, point2});
Mutator otherMutator3 = Mutator(OpacityParams{alpha2, point});
ASSERT_TRUE(mutator2 != otherMutator2);
ASSERT_TRUE(mutator2 != otherMutator3);
}

} // namespace testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,13 @@
head = clipView;
break;
}
case opacity:
embedded_view.alpha = (*iter)->GetOpacityParams().GetAlphaF() * embedded_view.alpha;
CATransform3D transform =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we bundling a transform with the opacity vs. pushing a separate transform mutator?

CATransform3DMakeTranslation((*iter)->GetOpacityParams().offset.get().fX,
(*iter)->GetOpacityParams().offset.get().fY, 0);
head.layer.transform = CATransform3DConcat(head.layer.transform, transform);
break;
}
++iter;
}
Expand All @@ -299,6 +306,7 @@
UIView* touchInterceptor = touch_interceptors_[view_id].get();
touchInterceptor.layer.transform = CATransform3DIdentity;
touchInterceptor.frame = frame;
touchInterceptor.alpha = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since we are adding more stuff here and might add even more later, we can have a possible refactoring to make a separate method to do all of these: ResetEmbeddedViewMutation(int view_id, EmbeddedViewParams params)


int currentClippingCount = CountClips(params.mutatorsStack);
int previousClippingCount = clip_count_[view_id];
Expand Down