From 92d9b3d364271f7b220a06c8e2f106651ae12e62 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Sat, 29 Jun 2019 12:21:48 -0700 Subject: [PATCH 1/9] disable mysterious failing tests --- flow/mutators_stack_unittests.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flow/mutators_stack_unittests.cc b/flow/mutators_stack_unittests.cc index 16d4b3d5a8ba0..8d768613314c1 100644 --- a/flow/mutators_stack_unittests.cc +++ b/flow/mutators_stack_unittests.cc @@ -10,7 +10,7 @@ TEST(MutatorsStack, Initialization) { ASSERT_TRUE(true); } -TEST(MutatorsStack, CopyConstructor) { +TEST(MutatorsStack, DISABLED_CopyConstructor) { flutter::MutatorsStack stack; SkRRect rrect; SkRect rect; @@ -20,7 +20,7 @@ TEST(MutatorsStack, CopyConstructor) { ASSERT_TRUE(copy == stack); } -TEST(MutatorsStack, PushClipRect) { +TEST(MutatorsStack, DISABLED_PushClipRect) { flutter::MutatorsStack stack; SkRect rect; stack.pushClipRect(rect); @@ -56,7 +56,7 @@ TEST(MutatorsStack, Pop) { ASSERT_TRUE(iter == stack.top()); } -TEST(MutatorsStack, Traversal) { +TEST(MutatorsStack, DISABLED_Traversal) { flutter::MutatorsStack stack; SkMatrix matrix; stack.pushTransform(matrix); From b8a9f6f6532c82e1692e53fc7d937861d2d58c31 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 3 Jul 2019 08:48:00 -0700 Subject: [PATCH 2/9] draft --- flow/embedded_views.cc | 5 +++++ flow/embedded_views.h | 30 ++++++++++++++++++------------ flow/layers/opacity_layer.cc | 1 + flow/mutators_stack_unittests.cc | 8 ++++++++ 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index 737288136ab71..97004ee2cd571 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -30,6 +30,11 @@ void MutatorsStack::pushTransform(const SkMatrix& matrix) { vector_.push_back(element); }; +void MutatorsStack::pushOpacity(const int &alpha){ + std::shared_ptr element = std::make_shared(alpha); + vector_.push_back(element); +}; + void MutatorsStack::pop() { vector_.pop_back(); }; diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 50d09ed928859..97b39a29ac864 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -17,7 +17,7 @@ namespace flutter { -enum MutatorType { clip_rect, clip_rrect, clip_path, transform }; +enum MutatorType { clip_rect, clip_rrect, clip_path, transform, opacity }; // Stores mutation information like clipping or transform. // @@ -42,6 +42,8 @@ class Mutator { case transform: matrix_ = other.matrix_; break; + case opacity: + alpha_ = other.alpha_; default: break; } @@ -53,6 +55,8 @@ class Mutator { : type_(clip_path), path_(new SkPath(path)) {} explicit Mutator(const SkMatrix& matrix) : type_(transform), matrix_(matrix) {} + explicit Mutator(const int& alpha) + : type_(opacity), alpha_(alpha) {} const MutatorType& type() const { return type_; } const SkRect& rect() const { return rect_; } @@ -64,17 +68,17 @@ class Mutator { if (type_ != other.type_) { return false; } - if (type_ == clip_rect && rect_ == other.rect_) { - return true; - } - if (type_ == clip_rrect && rrect_ == other.rrect_) { - return true; - } - if (type_ == clip_path && *path_ == *other.path_) { - return true; - } - if (type_ == transform && matrix_ == other.matrix_) { - return true; + switch (type_) { + case clip_rect: + return rect_ == other.rect_; + case clip_rrect: + return rrect_ == other.rrect_; + case clip_path: + return path_ == other.path_; + case transform: + return matrix_ == other.matrix_; + case opacity: + return alpha_ == other.alpha_; } return false; @@ -99,6 +103,7 @@ class Mutator { SkRect rect_; SkRRect rrect_; SkMatrix matrix_; + int alpha_; SkPath* path_; }; @@ -121,6 +126,7 @@ class MutatorsStack { void pushClipRRect(const SkRRect& rrect); void pushClipPath(const SkPath& path); void pushTransform(const SkMatrix& matrix); + void pushOpacity(const int& alpha); // Removes the `Mutator` on the top of the stack // and destroys it. diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index ba104dbc9e11b..1f4d6df872a6d 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -58,6 +58,7 @@ void OpacityLayer::Paint(PaintContext& context) const { SkPaint paint; paint.setAlpha(alpha_); + FML_DLOG(ERROR) << "alpha "<< paint.getAlphaf(); SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->translate(offset_.fX, offset_.fY); diff --git a/flow/mutators_stack_unittests.cc b/flow/mutators_stack_unittests.cc index c306eb63780ae..ae5023ad408c0 100644 --- a/flow/mutators_stack_unittests.cc +++ b/flow/mutators_stack_unittests.cc @@ -60,6 +60,14 @@ TEST(MutatorsStack, PushTransform) { ASSERT_TRUE(iter->get()->matrix() == matrix); } +TEST(MutatorsStack, PushOpacity) { + MutatorsStack stack; + stack.pushAlph(240); + auto iter = stack.bottom(); + ASSERT_TRUE(iter->get()->type() == MutatorType::opacity); + ASSERT_TRUE(iter->get()->matrix() == matrix); +} + TEST(MutatorsStack, Pop) { MutatorsStack stack; SkMatrix matrix; From 8583580c553752e361d0ba1f386290420be23485 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 3 Jul 2019 10:02:52 -0700 Subject: [PATCH 3/9] refactoring --- flow/embedded_views.cc | 14 +-- flow/embedded_views.h | 46 ++++--- flow/layers/clip_path_layer.cc | 4 +- flow/layers/clip_rect_layer.cc | 4 +- flow/layers/clip_rrect_layer.cc | 4 +- flow/layers/transform_layer.cc | 4 +- flow/mutators_stack_unittests.cc | 116 ++++++++---------- .../framework/Source/FlutterPlatformViews.mm | 22 ++-- 8 files changed, 97 insertions(+), 117 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index 737288136ab71..e6ce73fdeef94 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -10,37 +10,37 @@ bool ExternalViewEmbedder::SubmitFrame(GrContext* context) { return false; }; -void MutatorsStack::pushClipRect(const SkRect& rect) { +void MutatorsStack::PushClipRect(const SkRect& rect) { std::shared_ptr element = std::make_shared(rect); vector_.push_back(element); }; -void MutatorsStack::pushClipRRect(const SkRRect& rrect) { +void MutatorsStack::PushClipRRect(const SkRRect& rrect) { std::shared_ptr element = std::make_shared(rrect); vector_.push_back(element); }; -void MutatorsStack::pushClipPath(const SkPath& path) { +void MutatorsStack::PushClipPath(const SkPath& path) { std::shared_ptr element = std::make_shared(path); vector_.push_back(element); }; -void MutatorsStack::pushTransform(const SkMatrix& matrix) { +void MutatorsStack::PushTransform(const SkMatrix& matrix) { std::shared_ptr element = std::make_shared(matrix); vector_.push_back(element); }; -void MutatorsStack::pop() { +void MutatorsStack::Pop() { vector_.pop_back(); }; const std::vector>::const_reverse_iterator -MutatorsStack::top() const { +MutatorsStack::Top() const { return vector_.rend(); }; const std::vector>::const_reverse_iterator -MutatorsStack::bottom() const { +MutatorsStack::Bottom() const { return vector_.rbegin(); }; diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 1b641665e3690..16627d5b2bbb9 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -54,27 +54,25 @@ class Mutator { explicit Mutator(const SkMatrix& matrix) : type_(transform), matrix_(matrix) {} - const MutatorType& type() const { return type_; } - const SkRect& rect() const { return rect_; } - const SkRRect& rrect() const { return rrect_; } - const SkPath& path() const { return *path_; } - const SkMatrix& matrix() const { return matrix_; } + 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_; } bool operator==(const Mutator& other) const { if (type_ != other.type_) { return false; } - if (type_ == clip_rect && rect_ == other.rect_) { - return true; - } - if (type_ == clip_rrect && rrect_ == other.rrect_) { - return true; - } - if (type_ == clip_path && *path_ == *other.path_) { - return true; - } - if (type_ == transform && matrix_ == other.matrix_) { - return true; + switch (type_) { + case clip_rect: + return rect_ == other.rect_; + case clip_rrect: + return rrect_ == other.rrect_; + case clip_path: + return *path_ == *other.path_; + case transform: + return matrix_ == other.matrix_; } return false; @@ -82,7 +80,7 @@ class Mutator { bool operator!=(const Mutator& other) const { return !operator==(other); } - bool isClipType() { + bool IsClipType() { return type_ == clip_rect || type_ == clip_rrect || type_ == clip_path; } @@ -117,20 +115,20 @@ class MutatorsStack { public: MutatorsStack() = default; - void pushClipRect(const SkRect& rect); - void pushClipRRect(const SkRRect& rrect); - void pushClipPath(const SkPath& path); - void pushTransform(const SkMatrix& matrix); + void PushClipRect(const SkRect& rect); + void PushClipRRect(const SkRRect& rrect); + void PushClipPath(const SkPath& path); + void PushTransform(const SkMatrix& matrix); // Removes the `Mutator` on the top of the stack // and destroys it. - void pop(); + void Pop(); // Returns an iterator pointing to the top of the stack. - const std::vector>::const_reverse_iterator top() + const std::vector>::const_reverse_iterator Top() const; // Returns an iterator pointing to the bottom of the stack. - const std::vector>::const_reverse_iterator bottom() + const std::vector>::const_reverse_iterator Bottom() const; bool operator==(const MutatorsStack& other) const { diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc index cb7c8454e4743..c81b3a2190182 100644 --- a/flow/layers/clip_path_layer.cc +++ b/flow/layers/clip_path_layer.cc @@ -23,14 +23,14 @@ void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect previous_cull_rect = context->cull_rect; SkRect clip_path_bounds = clip_path_.getBounds(); if (context->cull_rect.intersect(clip_path_bounds)) { - context->mutators_stack.pushClipPath(clip_path_); + context->mutators_stack.PushClipPath(clip_path_); SkRect child_paint_bounds = SkRect::MakeEmpty(); PrerollChildren(context, matrix, &child_paint_bounds); if (child_paint_bounds.intersect(clip_path_bounds)) { set_paint_bounds(child_paint_bounds); } - context->mutators_stack.pop(); + context->mutators_stack.Pop(); } context->cull_rect = previous_cull_rect; } diff --git a/flow/layers/clip_rect_layer.cc b/flow/layers/clip_rect_layer.cc index 3f6cda5954be0..084b0957c1ac9 100644 --- a/flow/layers/clip_rect_layer.cc +++ b/flow/layers/clip_rect_layer.cc @@ -16,14 +16,14 @@ ClipRectLayer::~ClipRectLayer() = default; void ClipRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect previous_cull_rect = context->cull_rect; if (context->cull_rect.intersect(clip_rect_)) { - context->mutators_stack.pushClipRect(clip_rect_); + context->mutators_stack.PushClipRect(clip_rect_); SkRect child_paint_bounds = SkRect::MakeEmpty(); PrerollChildren(context, matrix, &child_paint_bounds); if (child_paint_bounds.intersect(clip_rect_)) { set_paint_bounds(child_paint_bounds); } - context->mutators_stack.pop(); + context->mutators_stack.Pop(); } context->cull_rect = previous_cull_rect; } diff --git a/flow/layers/clip_rrect_layer.cc b/flow/layers/clip_rrect_layer.cc index 7cd7dd24d1ee7..d810bb70f7448 100644 --- a/flow/layers/clip_rrect_layer.cc +++ b/flow/layers/clip_rrect_layer.cc @@ -17,14 +17,14 @@ void ClipRRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect previous_cull_rect = context->cull_rect; SkRect clip_rrect_bounds = clip_rrect_.getBounds(); if (context->cull_rect.intersect(clip_rrect_bounds)) { - context->mutators_stack.pushClipRRect(clip_rrect_); + context->mutators_stack.PushClipRRect(clip_rrect_); SkRect child_paint_bounds = SkRect::MakeEmpty(); PrerollChildren(context, matrix, &child_paint_bounds); if (child_paint_bounds.intersect(clip_rrect_bounds)) { set_paint_bounds(child_paint_bounds); } - context->mutators_stack.pop(); + context->mutators_stack.Pop(); } context->cull_rect = previous_cull_rect; } diff --git a/flow/layers/transform_layer.cc b/flow/layers/transform_layer.cc index f1c2b013dfba5..5a7af132c68f2 100644 --- a/flow/layers/transform_layer.cc +++ b/flow/layers/transform_layer.cc @@ -29,7 +29,7 @@ TransformLayer::~TransformLayer() = default; void TransformLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkMatrix child_matrix; child_matrix.setConcat(matrix, transform_); - context->mutators_stack.pushTransform(transform_); + context->mutators_stack.PushTransform(transform_); SkRect previous_cull_rect = context->cull_rect; SkMatrix inverse_transform_; // Perspective projections don't produce rectangles that are useful for @@ -47,7 +47,7 @@ void TransformLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { set_paint_bounds(child_paint_bounds); context->cull_rect = previous_cull_rect; - context->mutators_stack.pop(); + context->mutators_stack.Pop(); } #if defined(OS_FUCHSIA) diff --git a/flow/mutators_stack_unittests.cc b/flow/mutators_stack_unittests.cc index e69f1f83d7257..47f1b20e6067d 100644 --- a/flow/mutators_stack_unittests.cc +++ b/flow/mutators_stack_unittests.cc @@ -13,105 +13,87 @@ TEST(MutatorsStack, Initialization) { ASSERT_TRUE(true); } -<<<<<<< HEAD -TEST(MutatorsStack, DISABLED_CopyConstructor) { - flutter::MutatorsStack stack; - SkRRect rrect; - SkRect rect; -======= TEST(MutatorsStack, CopyConstructor) { MutatorsStack stack; auto rrect = SkRRect::MakeEmpty(); auto rect = SkRect::MakeEmpty(); ->>>>>>> 791143f17281dda91bd0a5428de78d0871937324 - stack.pushClipRect(rect); - stack.pushClipRRect(rrect); + stack.PushClipRect(rect); + stack.PushClipRRect(rrect); MutatorsStack copy = MutatorsStack(stack); ASSERT_TRUE(copy == stack); } -<<<<<<< HEAD -TEST(MutatorsStack, DISABLED_PushClipRect) { - flutter::MutatorsStack stack; - SkRect rect; -======= TEST(MutatorsStack, PushClipRect) { MutatorsStack stack; auto rect = SkRect::MakeEmpty(); ->>>>>>> 791143f17281dda91bd0a5428de78d0871937324 - stack.pushClipRect(rect); - auto iter = stack.bottom(); - ASSERT_TRUE(iter->get()->type() == MutatorType::clip_rect); - ASSERT_TRUE(iter->get()->rect() == rect); + stack.PushClipRect(rect); + auto iter = stack.Bottom(); + ASSERT_TRUE(iter->get()->GetType() == MutatorType::clip_rect); + ASSERT_TRUE(iter->get()->GetRect() == rect); } TEST(MutatorsStack, PushClipRRect) { MutatorsStack stack; auto rrect = SkRRect::MakeEmpty(); - stack.pushClipRRect(rrect); - auto iter = stack.bottom(); - ASSERT_TRUE(iter->get()->type() == MutatorType::clip_rrect); - ASSERT_TRUE(iter->get()->rrect() == rrect); + stack.PushClipRRect(rrect); + auto iter = stack.Bottom(); + ASSERT_TRUE(iter->get()->GetType() == MutatorType::clip_rrect); + ASSERT_TRUE(iter->get()->GetRRect() == rrect); } TEST(MutatorsStack, PushClipPath) { flutter::MutatorsStack stack; SkPath path; - stack.pushClipPath(path); - auto iter = stack.bottom(); - ASSERT_TRUE(iter->get()->type() == flutter::MutatorType::clip_path); - ASSERT_TRUE(iter->get()->path() == path); + stack.PushClipPath(path); + auto iter = stack.Bottom(); + ASSERT_TRUE(iter->get()->GetType() == flutter::MutatorType::clip_path); + ASSERT_TRUE(iter->get()->GetPath() == path); } TEST(MutatorsStack, PushTransform) { MutatorsStack stack; SkMatrix matrix; matrix.setIdentity(); - stack.pushTransform(matrix); - auto iter = stack.bottom(); - ASSERT_TRUE(iter->get()->type() == MutatorType::transform); - ASSERT_TRUE(iter->get()->matrix() == matrix); + stack.PushTransform(matrix); + auto iter = stack.Bottom(); + ASSERT_TRUE(iter->get()->GetType() == MutatorType::transform); + ASSERT_TRUE(iter->get()->GetMatrix() == matrix); } TEST(MutatorsStack, Pop) { MutatorsStack stack; SkMatrix matrix; matrix.setIdentity(); - stack.pushTransform(matrix); - stack.pop(); - auto iter = stack.bottom(); - ASSERT_TRUE(iter == stack.top()); + stack.PushTransform(matrix); + stack.Pop(); + auto iter = stack.Bottom(); + ASSERT_TRUE(iter == stack.Top()); } -<<<<<<< HEAD -TEST(MutatorsStack, DISABLED_Traversal) { - flutter::MutatorsStack stack; -======= TEST(MutatorsStack, Traversal) { MutatorsStack stack; ->>>>>>> 791143f17281dda91bd0a5428de78d0871937324 SkMatrix matrix; matrix.setIdentity(); - stack.pushTransform(matrix); + stack.PushTransform(matrix); auto rect = SkRect::MakeEmpty(); - stack.pushClipRect(rect); + stack.PushClipRect(rect); auto rrect = SkRRect::MakeEmpty(); - stack.pushClipRRect(rrect); - auto iter = stack.bottom(); + stack.PushClipRRect(rrect); + auto iter = stack.Bottom(); int index = 0; - while (iter != stack.top()) { + while (iter != stack.Top()) { switch (index) { case 0: - ASSERT_TRUE(iter->get()->type() == MutatorType::clip_rrect); - ASSERT_TRUE(iter->get()->rrect() == rrect); + ASSERT_TRUE(iter->get()->GetType() == MutatorType::clip_rrect); + ASSERT_TRUE(iter->get()->GetRRect() == rrect); break; case 1: - ASSERT_TRUE(iter->get()->type() == MutatorType::clip_rect); - ASSERT_TRUE(iter->get()->rect() == rect); + ASSERT_TRUE(iter->get()->GetType() == MutatorType::clip_rect); + ASSERT_TRUE(iter->get()->GetRect() == rect); break; case 2: - ASSERT_TRUE(iter->get()->type() == MutatorType::transform); - ASSERT_TRUE(iter->get()->matrix() == matrix); + ASSERT_TRUE(iter->get()->GetType() == MutatorType::transform); + ASSERT_TRUE(iter->get()->GetMatrix() == matrix); break; default: break; @@ -124,23 +106,23 @@ TEST(MutatorsStack, Traversal) { TEST(MutatorsStack, Equality) { MutatorsStack stack; SkMatrix matrix = SkMatrix::MakeScale(1, 1); - stack.pushTransform(matrix); + stack.PushTransform(matrix); SkRect rect = SkRect::MakeEmpty(); - stack.pushClipRect(rect); + stack.PushClipRect(rect); SkRRect rrect = SkRRect::MakeEmpty(); - stack.pushClipRRect(rrect); + stack.PushClipRRect(rrect); SkPath path; - stack.pushClipPath(path); + stack.PushClipPath(path); MutatorsStack stackOther; SkMatrix matrixOther = SkMatrix::MakeScale(1, 1); - stackOther.pushTransform(matrixOther); + stackOther.PushTransform(matrixOther); SkRect rectOther = SkRect::MakeEmpty(); - stackOther.pushClipRect(rectOther); + stackOther.PushClipRect(rectOther); SkRRect rrectOther = SkRRect::MakeEmpty(); - stackOther.pushClipRRect(rrectOther); + stackOther.PushClipRRect(rrectOther); SkPath otherPath; - stackOther.pushClipPath(otherPath); + stackOther.PushClipPath(otherPath); ASSERT_TRUE(stack == stackOther); } @@ -148,24 +130,24 @@ TEST(MutatorsStack, Equality) { TEST(Mutator, Initialization) { SkRect rect = SkRect::MakeEmpty(); Mutator mutator = Mutator(rect); - ASSERT_TRUE(mutator.type() == MutatorType::clip_rect); - ASSERT_TRUE(mutator.rect() == rect); + ASSERT_TRUE(mutator.GetType() == MutatorType::clip_rect); + ASSERT_TRUE(mutator.GetRect() == rect); SkRRect rrect = SkRRect::MakeEmpty(); Mutator mutator2 = Mutator(rrect); - ASSERT_TRUE(mutator2.type() == MutatorType::clip_rrect); - ASSERT_TRUE(mutator2.rrect() == rrect); + ASSERT_TRUE(mutator2.GetType() == MutatorType::clip_rrect); + ASSERT_TRUE(mutator2.GetRRect() == rrect); SkPath path; Mutator mutator3 = Mutator(path); - ASSERT_TRUE(mutator3.type() == MutatorType::clip_path); - ASSERT_TRUE(mutator3.path() == path); + ASSERT_TRUE(mutator3.GetType() == MutatorType::clip_path); + ASSERT_TRUE(mutator3.GetPath() == path); SkMatrix matrix; matrix.setIdentity(); Mutator mutator4 = Mutator(matrix); - ASSERT_TRUE(mutator4.type() == MutatorType::transform); - ASSERT_TRUE(mutator4.matrix() == matrix); + ASSERT_TRUE(mutator4.GetType() == MutatorType::transform); + ASSERT_TRUE(mutator4.GetMatrix() == matrix); } TEST(Mutator, CopyConstructor) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 33c5332baa96d..f0cb6939e6c04 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -205,10 +205,10 @@ } int FlutterPlatformViewsController::CountClips(const MutatorsStack& mutators_stack) { - std::vector>::const_reverse_iterator iter = mutators_stack.bottom(); + std::vector>::const_reverse_iterator iter = mutators_stack.Bottom(); int clipCount = 0; - while (iter != mutators_stack.top()) { - if ((*iter)->isClipType()) { + while (iter != mutators_stack.Top()) { + if ((*iter)->IsClipType()) { clipCount++; } ++iter; @@ -256,11 +256,11 @@ head.clipsToBounds = YES; ResetAnchor(head.layer); - std::vector>::const_reverse_iterator iter = mutators_stack.bottom(); - while (iter != mutators_stack.top()) { - switch ((*iter)->type()) { + std::vector>::const_reverse_iterator iter = mutators_stack.Bottom(); + while (iter != mutators_stack.Top()) { + switch ((*iter)->GetType()) { case transform: { - CATransform3D transform = GetCATransform3DFromSkMatrix((*iter)->matrix()); + CATransform3D transform = GetCATransform3DFromSkMatrix((*iter)->GetMatrix()); head.layer.transform = CATransform3DConcat(head.layer.transform, transform); break; } @@ -269,10 +269,10 @@ case clip_path: { ChildClippingView* clipView = (ChildClippingView*)head.superview; clipView.layer.transform = CATransform3DIdentity; - [clipView setClip:(*iter)->type() - rect:(*iter)->rect() - rrect:(*iter)->rrect() - path:(*iter)->path()]; + [clipView setClip:(*iter)->GetType() + rect:(*iter)->GetRect() + rrect:(*iter)->GetRRect() + path:(*iter)->GetPath()]; head.clipsToBounds = YES; ResetAnchor(clipView.layer); head = clipView; From 740e8575289b6c3ee51498c69fad68ced902d170 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 3 Jul 2019 10:19:54 -0700 Subject: [PATCH 4/9] formatting --- flow/embedded_views.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 16627d5b2bbb9..8f35f64238939 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -70,7 +70,7 @@ class Mutator { case clip_rrect: return rrect_ == other.rrect_; case clip_path: - return *path_ == *other.path_; + return *path_ == *other.path_; case transform: return matrix_ == other.matrix_; } From ee78ff9a93d370249be57638c15ea6ab7e304bd4 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 3 Jul 2019 14:07:20 -0700 Subject: [PATCH 5/9] add OpacityParams --- flow/embedded_views.cc | 4 +- flow/embedded_views.h | 25 +++++++++--- flow/layers/opacity_layer.cc | 4 +- flow/mutators_stack_unittests.cc | 39 +++++++++++-------- .../framework/Source/FlutterPlatformViews.mm | 5 ++- 5 files changed, 49 insertions(+), 28 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index c660f4691318b..1be768012b4fa 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -30,8 +30,8 @@ void MutatorsStack::PushTransform(const SkMatrix& matrix) { vector_.push_back(element); }; -void MutatorsStack::PushOpacity(const int& alpha) { - std::shared_ptr element = std::make_shared(alpha); +void MutatorsStack::PushOpacity(const OpacityParams& opacityParams) { + std::shared_ptr element = std::make_shared(opacityParams); vector_.push_back(element); }; diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 2f1cae0df6f5a..acaea33280962 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -19,6 +19,19 @@ namespace flutter { enum MutatorType { clip_rect, clip_rrect, clip_path, transform, opacity }; +struct OpacityParams { + int alpha; + s=SkPoint offset; + + 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); } +}; + // Stores mutation information like clipping or transform. // // The `type` indicates the type of the mutation: clip_rect, transform and etc. @@ -43,7 +56,7 @@ class Mutator { matrix_ = other.matrix_; break; case opacity: - alpha_ = other.alpha_; + opacityParams_ = OpacityParams{other.opacityParams_.alpha, other.opacityParams_.offset}; default: break; } @@ -55,15 +68,14 @@ class Mutator { : type_(clip_path), path_(new SkPath(path)) {} explicit Mutator(const SkMatrix& matrix) : type_(transform), matrix_(matrix) {} - explicit Mutator(const int& alpha) : type_(opacity), alpha_(alpha) {} + 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 int& GetAlpha() const { return alpha_; } - float GetAlphaF() const { return (alpha_ / 255.0); } + const OpacityParams& GetOpacityParams() const { return opacityParams_; } bool operator==(const Mutator& other) const { if (type_ != other.type_) { @@ -79,7 +91,7 @@ class Mutator { case transform: return matrix_ == other.matrix_; case opacity: - return alpha_ == other.alpha_; + return opacityParams_ == other.opacityParams_; } return false; @@ -104,6 +116,7 @@ class Mutator { SkRect rect_; SkRRect rrect_; SkMatrix matrix_; + OpacityParams opacityParams_; int alpha_; SkPath* path_; }; @@ -127,7 +140,7 @@ class MutatorsStack { void PushClipRRect(const SkRRect& rrect); void PushClipPath(const SkPath& path); void PushTransform(const SkMatrix& matrix); - void PushOpacity(const int& alpha); + void PushOpacity(const OpacityParams& opacityParams); // Removes the `Mutator` on the top of the stack // and destroys it. diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 607dca14eb026..02bd79bfcdba7 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -37,7 +37,8 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { EnsureSingleChild(); SkMatrix child_matrix = matrix; child_matrix.postTranslate(offset_.fX, offset_.fY); - context->mutators_stack.PushOpacity(alpha_); + // 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)); @@ -60,7 +61,6 @@ void OpacityLayer::Paint(PaintContext& context) const { SkPaint paint; paint.setAlpha(alpha_); - FML_DLOG(ERROR) << "alpha " << paint.getAlphaf(); SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->translate(offset_.fX, offset_.fY); diff --git a/flow/mutators_stack_unittests.cc b/flow/mutators_stack_unittests.cc index 948c7df9b4681..53e007738ab56 100644 --- a/flow/mutators_stack_unittests.cc +++ b/flow/mutators_stack_unittests.cc @@ -42,7 +42,7 @@ TEST(MutatorsStack, PushClipRRect) { } TEST(MutatorsStack, PushClipPath) { - flutter::MutatorsStack stack; + MutatorsStack stack; SkPath path; stack.PushClipPath(path); auto iter = stack.Bottom(); @@ -62,10 +62,11 @@ TEST(MutatorsStack, PushTransform) { TEST(MutatorsStack, PushOpacity) { MutatorsStack stack; - stack.PushOpacity(240); + SkPoint point = SkPoint::Make(0, 0); + stack.PushOpacity(OpacityParams{240, point}); auto iter = stack.Bottom(); ASSERT_TRUE(iter->get()->GetType() == MutatorType::opacity); - ASSERT_TRUE(iter->get()->GetAlpha() == 240); + ASSERT_TRUE(iter->get()->GetOpacityParams().alpha == 240); } TEST(MutatorsStack, Pop) { @@ -121,8 +122,8 @@ TEST(MutatorsStack, Equality) { stack.PushClipRRect(rrect); SkPath path; stack.PushClipPath(path); - int alpha = 240; - stack.PushOpacity(alpha); + SkPoint point = SkPoint::Make(0, 0); + stack.PushOpacity(OpacityParams{240, point}); MutatorsStack stackOther; SkMatrix matrixOther = SkMatrix::MakeScale(1, 1); @@ -133,8 +134,8 @@ TEST(MutatorsStack, Equality) { stackOther.PushClipRRect(rrectOther); SkPath otherPath; stackOther.PushClipPath(otherPath); - int otherAlpha = 240; - stackOther.PushOpacity(otherAlpha); + SkPoint ohterPoint = SkPoint::Make(0, 0); + stackOther.PushOpacity(OpacityParams{240, ohterPoint}); ASSERT_TRUE(stack == stackOther); } @@ -161,8 +162,8 @@ TEST(Mutator, Initialization) { ASSERT_TRUE(mutator4.GetType() == MutatorType::transform); ASSERT_TRUE(mutator4.GetMatrix() == matrix); - int alpha = 240; - Mutator mutator5 = Mutator(alpha); + SkPoint point = SkPoint::Make(0, 0); + Mutator mutator5 = Mutator(OpacityParams{240, point}); ASSERT_TRUE(mutator5.GetType() == MutatorType::opacity); // ASSERT_TRUE(fabs(mutator5.GetAlphaF()-alpha/255) < EPSILON); } @@ -189,8 +190,8 @@ TEST(Mutator, CopyConstructor) { Mutator copy4 = Mutator(mutator4); ASSERT_TRUE(mutator4 == copy4); - int alpha = 240; - Mutator mutator5 = Mutator(alpha); + SkPoint point = SkPoint::Make(0, 0); + Mutator mutator5 = Mutator(OpacityParams{240, point}); Mutator copy5 = Mutator(mutator5); ASSERT_TRUE(mutator5 == copy5); } @@ -218,12 +219,10 @@ TEST(Mutator, Equality) { ASSERT_TRUE(mutator4 == otherMutator4); ASSERT_FALSE(mutator2 == mutator); - int alpha = 240; - Mutator mutator5 = Mutator(alpha); - Mutator otherMutator5 = Mutator(alpha); - ASSERT_TRUE(mutator5 == otherMutator5 - - ); + SkPoint point = SkPoint::Make(0, 0); + Mutator mutator5 = Mutator(OpacityParams{240, point}); + Mutator otherMutator5 = Mutator(OpacityParams{240, point}); + ASSERT_TRUE(mutator5 == otherMutator5); } TEST(Mutator, UnEquality) { @@ -233,6 +232,12 @@ TEST(Mutator, UnEquality) { matrix.setIdentity(); Mutator notEqualMutator = Mutator(matrix); ASSERT_TRUE(notEqualMutator != mutator); + + SkPoint point = SkPoint::Make(0, 0); + SkPoint point2 = SkPoint::Make(1, 0); + Mutator mutator2 = Mutator(OpacityParams{240, point}); + Mutator otherMutator2 = Mutator(OpacityParams{240, point2}); + ASSERT_TRUE(mutator2 != otherMutator2); } } // namespace testing diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index f5e5008dd0de0..d1efe00933eb8 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -258,6 +258,7 @@ std::vector>::const_reverse_iterator iter = mutators_stack.Bottom(); while (iter != mutators_stack.Top()) { + FML_DLOG(ERROR) << "type " << (*iter)->GetType(); switch ((*iter)->GetType()) { case transform: { CATransform3D transform = GetCATransform3DFromSkMatrix((*iter)->GetMatrix()); @@ -279,7 +280,9 @@ break; } case opacity: - embedded_view.alpha = (*iter)->GetAlphaF() * embedded_view.alpha; + embedded_view.alpha = (*iter)->GetOpacityParams().GetAlphaF() * embedded_view.alpha; + CATransform3D transform = CATransform3DMakeTranslation((*iter)->GetOpacityParams().offset.fX, (*iter)->GetOpacityParams().offset.fY, 0); + head.layer.transform = CATransform3DConcat(head.layer.transform, transform); break; } ++iter; From 23c6807661af51762aa40c442077710c004341f4 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 3 Jul 2019 14:29:26 -0700 Subject: [PATCH 6/9] make OpacityParams members to be references --- flow/embedded_views.h | 16 ++++++---- flow/layers/opacity_layer.cc | 3 +- flow/mutators_stack_unittests.cc | 29 ++++++++++++------- .../framework/Source/FlutterPlatformViews.mm | 7 +++-- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index acaea33280962..8fe6b5d6084aa 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -19,15 +19,19 @@ namespace flutter { 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 { - int alpha; - s=SkPoint offset; + std::reference_wrapper alpha; + std::reference_wrapper offset; bool operator==(const OpacityParams& other) const { return alpha == other.alpha && offset == other.offset; } - bool operator!=(const OpacityParams& other) const { return !operator==(other); } + bool operator!=(const OpacityParams& other) const { + return !operator==(other); + } float GetAlphaF() const { return (alpha / 255.0); } }; @@ -56,7 +60,8 @@ class Mutator { matrix_ = other.matrix_; break; case opacity: - opacityParams_ = OpacityParams{other.opacityParams_.alpha, other.opacityParams_.offset}; + opacityParams_ = OpacityParams{other.opacityParams_.alpha, + other.opacityParams_.offset}; default: break; } @@ -68,7 +73,8 @@ 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) {} + explicit Mutator(const OpacityParams& opacityParams) + : type_(opacity), opacityParams_(opacityParams) {} const MutatorType& GetType() const { return type_; } const SkRect& GetRect() const { return rect_; } diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 02bd79bfcdba7..8621ad56e4dab 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -37,7 +37,8 @@ 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. + // 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(); diff --git a/flow/mutators_stack_unittests.cc b/flow/mutators_stack_unittests.cc index 53e007738ab56..cc3a44cb6b856 100644 --- a/flow/mutators_stack_unittests.cc +++ b/flow/mutators_stack_unittests.cc @@ -63,7 +63,8 @@ TEST(MutatorsStack, PushTransform) { TEST(MutatorsStack, PushOpacity) { MutatorsStack stack; SkPoint point = SkPoint::Make(0, 0); - stack.PushOpacity(OpacityParams{240, point}); + 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); @@ -122,8 +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{240, point}); + stack.PushOpacity(OpacityParams{alpha, point}); MutatorsStack stackOther; SkMatrix matrixOther = SkMatrix::MakeScale(1, 1); @@ -134,8 +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{240, ohterPoint}); + stackOther.PushOpacity(OpacityParams{otherAlpha, ohterPoint}); ASSERT_TRUE(stack == stackOther); } @@ -163,7 +166,8 @@ TEST(Mutator, Initialization) { ASSERT_TRUE(mutator4.GetMatrix() == matrix); SkPoint point = SkPoint::Make(0, 0); - Mutator mutator5 = Mutator(OpacityParams{240, point}); + int alpha = 240; + Mutator mutator5 = Mutator(OpacityParams{alpha, point}); ASSERT_TRUE(mutator5.GetType() == MutatorType::opacity); // ASSERT_TRUE(fabs(mutator5.GetAlphaF()-alpha/255) < EPSILON); } @@ -191,7 +195,8 @@ TEST(Mutator, CopyConstructor) { ASSERT_TRUE(mutator4 == copy4); SkPoint point = SkPoint::Make(0, 0); - Mutator mutator5 = Mutator(OpacityParams{240, point}); + int alpha = 240; + Mutator mutator5 = Mutator(OpacityParams{alpha, point}); Mutator copy5 = Mutator(mutator5); ASSERT_TRUE(mutator5 == copy5); } @@ -218,10 +223,10 @@ 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{240, point}); - Mutator otherMutator5 = Mutator(OpacityParams{240, point}); + Mutator mutator5 = Mutator(OpacityParams{alpha, point}); + Mutator otherMutator5 = Mutator(OpacityParams{alpha, point}); ASSERT_TRUE(mutator5 == otherMutator5); } @@ -233,11 +238,15 @@ TEST(Mutator, UnEquality) { 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{240, point}); - Mutator otherMutator2 = Mutator(OpacityParams{240, point2}); + 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 diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index d1efe00933eb8..5fa6ec143a7c1 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -258,7 +258,6 @@ std::vector>::const_reverse_iterator iter = mutators_stack.Bottom(); while (iter != mutators_stack.Top()) { - FML_DLOG(ERROR) << "type " << (*iter)->GetType(); switch ((*iter)->GetType()) { case transform: { CATransform3D transform = GetCATransform3DFromSkMatrix((*iter)->GetMatrix()); @@ -280,8 +279,10 @@ break; } case opacity: - embedded_view.alpha = (*iter)->GetOpacityParams().GetAlphaF() * embedded_view.alpha; - CATransform3D transform = CATransform3DMakeTranslation((*iter)->GetOpacityParams().offset.fX, (*iter)->GetOpacityParams().offset.fY, 0); + embedded_view.alpha = (*iter)->GetOpacityParams().GetAlphaF() * embedded_view.alpha; + CATransform3D transform = + CATransform3DMakeTranslation((*iter)->GetOpacityParams().offset.get().fX, + (*iter)->GetOpacityParams().offset.get().fY, 0); head.layer.transform = CATransform3DConcat(head.layer.transform, transform); break; } From a7e342729a4671edd8b31b7434850a6998b4a898 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 3 Jul 2019 14:44:05 -0700 Subject: [PATCH 7/9] cleanup --- flow/embedded_views.h | 1 - flow/layers/opacity_layer.cc | 2 -- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 3 +++ 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 8fe6b5d6084aa..4f1c421050b2c 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -123,7 +123,6 @@ class Mutator { SkRRect rrect_; SkMatrix matrix_; OpacityParams opacityParams_; - int alpha_; SkPath* path_; }; diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 8621ad56e4dab..7baa22e414946 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -37,8 +37,6 @@ 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(); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 5fa6ec143a7c1..ced74a00264b6 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -280,6 +280,9 @@ } case opacity: embedded_view.alpha = (*iter)->GetOpacityParams().GetAlphaF() * embedded_view.alpha; + if ((*iter)->GetOpacityParams().offset.get().isZero()) { + break; + } CATransform3D transform = CATransform3DMakeTranslation((*iter)->GetOpacityParams().offset.get().fX, (*iter)->GetOpacityParams().offset.get().fY, 0); From ce8bad561b898d8bba22f536e178973685764985 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 8 Jul 2019 14:09:07 -0700 Subject: [PATCH 8/9] review fixes --- flow/embedded_views.cc | 4 +-- flow/embedded_views.h | 33 +++++-------------- flow/layers/opacity_layer.cc | 5 ++- flow/mutators_stack_unittests.cc | 30 ++++++----------- .../framework/Source/FlutterPlatformViews.mm | 9 +---- 5 files changed, 25 insertions(+), 56 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index 1be768012b4fa..c660f4691318b 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -30,8 +30,8 @@ void MutatorsStack::PushTransform(const SkMatrix& matrix) { vector_.push_back(element); }; -void MutatorsStack::PushOpacity(const OpacityParams& opacityParams) { - std::shared_ptr element = std::make_shared(opacityParams); +void MutatorsStack::PushOpacity(const int& alpha) { + std::shared_ptr element = std::make_shared(alpha); vector_.push_back(element); }; diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 4f1c421050b2c..4c94896cddeef 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -19,23 +19,6 @@ namespace flutter { 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 alpha; - std::reference_wrapper offset; - - 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); } -}; - // Stores mutation information like clipping or transform. // // The `type` indicates the type of the mutation: clip_rect, transform and etc. @@ -60,8 +43,8 @@ class Mutator { matrix_ = other.matrix_; break; case opacity: - opacityParams_ = OpacityParams{other.opacityParams_.alpha, - other.opacityParams_.offset}; + alpha_ = other.alpha_; + break; default: break; } @@ -73,15 +56,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) {} + explicit Mutator(const int& alpha) : type_(opacity), alpha_(alpha) {} 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_; } + const int& GetAlpha() const { return alpha_; } + float GetAlphaF() const { return (alpha_ / 255.0); } bool operator==(const Mutator& other) const { if (type_ != other.type_) { @@ -97,7 +80,7 @@ class Mutator { case transform: return matrix_ == other.matrix_; case opacity: - return opacityParams_ == other.opacityParams_; + return alpha_ == other.alpha_; } return false; @@ -122,8 +105,8 @@ class Mutator { SkRect rect_; SkRRect rrect_; SkMatrix matrix_; - OpacityParams opacityParams_; SkPath* path_; + int alpha_; }; }; // Mutator @@ -145,7 +128,7 @@ class MutatorsStack { void PushClipRRect(const SkRRect& rrect); void PushClipPath(const SkPath& path); void PushTransform(const SkMatrix& matrix); - void PushOpacity(const OpacityParams& opacityParams); + void PushOpacity(const int& alpha); // Removes the `Mutator` on the top of the stack // and destroys it. diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 7baa22e414946..b3f8b0cac368c 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -37,9 +37,12 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { EnsureSingleChild(); SkMatrix child_matrix = matrix; child_matrix.postTranslate(offset_.fX, offset_.fY); - context->mutators_stack.PushOpacity(OpacityParams{alpha_, offset_}); + context->mutators_stack.PushTransform( + SkMatrix::MakeTrans(offset_.fX, offset_.fY)); + context->mutators_stack.PushOpacity(alpha_); ContainerLayer::Preroll(context, child_matrix); context->mutators_stack.Pop(); + context->mutators_stack.Pop(); set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY)); // See |EnsureSingleChild|. FML_DCHECK(layers().size() == 1); diff --git a/flow/mutators_stack_unittests.cc b/flow/mutators_stack_unittests.cc index 5a6d1268dbdee..97cfe9a54a7c7 100644 --- a/flow/mutators_stack_unittests.cc +++ b/flow/mutators_stack_unittests.cc @@ -62,12 +62,11 @@ TEST(MutatorsStack, PushTransform) { TEST(MutatorsStack, PushOpacity) { MutatorsStack stack; - SkPoint point = SkPoint::Make(0, 0); int alpha = 240; - stack.PushOpacity(OpacityParams{alpha, point}); + stack.PushOpacity(alpha); auto iter = stack.Bottom(); ASSERT_TRUE(iter->get()->GetType() == MutatorType::opacity); - ASSERT_TRUE(iter->get()->GetOpacityParams().alpha == 240); + ASSERT_TRUE(iter->get()->GetAlpha() == 240); } TEST(MutatorsStack, Pop) { @@ -124,8 +123,7 @@ TEST(MutatorsStack, Equality) { SkPath path; stack.PushClipPath(path); int alpha = 240; - SkPoint point = SkPoint::Make(0, 0); - stack.PushOpacity(OpacityParams{alpha, point}); + stack.PushOpacity(alpha); MutatorsStack stackOther; SkMatrix matrixOther = SkMatrix::MakeScale(1, 1); @@ -137,8 +135,7 @@ TEST(MutatorsStack, Equality) { SkPath otherPath; stackOther.PushClipPath(otherPath); int otherAlpha = 240; - SkPoint ohterPoint = SkPoint::Make(0, 0); - stackOther.PushOpacity(OpacityParams{otherAlpha, ohterPoint}); + stackOther.PushOpacity(otherAlpha); ASSERT_TRUE(stack == stackOther); } @@ -165,9 +162,8 @@ TEST(Mutator, Initialization) { 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}); + Mutator mutator5 = Mutator(alpha); ASSERT_TRUE(mutator5.GetType() == MutatorType::opacity); } @@ -193,9 +189,8 @@ TEST(Mutator, CopyConstructor) { Mutator copy4 = Mutator(mutator4); ASSERT_TRUE(mutator4 == copy4); - SkPoint point = SkPoint::Make(0, 0); int alpha = 240; - Mutator mutator5 = Mutator(OpacityParams{alpha, point}); + Mutator mutator5 = Mutator(alpha); Mutator copy5 = Mutator(mutator5); ASSERT_TRUE(mutator5 == copy5); } @@ -223,9 +218,8 @@ TEST(Mutator, Equality) { 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}); + Mutator mutator5 = Mutator(alpha); + Mutator otherMutator5 = Mutator(alpha); ASSERT_TRUE(mutator5 == otherMutator5); } @@ -239,13 +233,9 @@ TEST(Mutator, UnEquality) { 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}); + Mutator mutator2 = Mutator(alpha); + Mutator otherMutator2 = Mutator(alpha2); ASSERT_TRUE(mutator2 != otherMutator2); - ASSERT_TRUE(mutator2 != otherMutator3); } } // namespace testing diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index ced74a00264b6..f23ed4ce993f1 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -279,14 +279,7 @@ break; } case opacity: - embedded_view.alpha = (*iter)->GetOpacityParams().GetAlphaF() * embedded_view.alpha; - if ((*iter)->GetOpacityParams().offset.get().isZero()) { - break; - } - CATransform3D transform = - CATransform3DMakeTranslation((*iter)->GetOpacityParams().offset.get().fX, - (*iter)->GetOpacityParams().offset.get().fY, 0); - head.layer.transform = CATransform3DConcat(head.layer.transform, transform); + embedded_view.alpha = (*iter)->GetAlphaF() * embedded_view.alpha; break; } ++iter; From fa898f51c30f042214d9d04980fb168f12fa2ce4 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 8 Jul 2019 15:39:20 -0700 Subject: [PATCH 9/9] rename GetAlphaF to GetAlphaFloat --- flow/embedded_views.h | 2 +- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 4c94896cddeef..adab60d67339d 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -64,7 +64,7 @@ class Mutator { const SkPath& GetPath() const { return *path_; } const SkMatrix& GetMatrix() const { return matrix_; } const int& GetAlpha() const { return alpha_; } - float GetAlphaF() const { return (alpha_ / 255.0); } + float GetAlphaFloat() const { return (alpha_ / 255.0); } bool operator==(const Mutator& other) const { if (type_ != other.type_) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index f23ed4ce993f1..c40d645600d9e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -279,7 +279,7 @@ break; } case opacity: - embedded_view.alpha = (*iter)->GetAlphaF() * embedded_view.alpha; + embedded_view.alpha = (*iter)->GetAlphaFloat() * embedded_view.alpha; break; } ++iter;