-
Notifications
You must be signed in to change notification settings - Fork 6k
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
IOS Platform view transform/clipping #9075
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass, still need to review the ObjC++ code more thoroughly.
flow/embedded_views.h
Outdated
@@ -42,8 +56,88 @@ class ExternalViewEmbedder { | |||
virtual ~ExternalViewEmbedder() = default; | |||
|
|||
FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); | |||
|
|||
#pragma mark - transforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we don't do these in our C++ code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
flow/embedded_views.h
Outdated
|
||
}; // ExternalViewEmbedder | ||
|
||
class FlutterEmbededViewTransformStack { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably omit the "Flutter" prefix from these classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -16,6 +16,134 @@ | |||
#include "flutter/fml/platform/darwin/scoped_nsobject.h" | |||
#include "flutter/shell/platform/darwin/ios/framework/Headers/FlutterChannels.h" | |||
|
|||
#pragma mark - Transforms Utils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: lets split these to a separate file, this one is getting too long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I just add these functions into the flutter namespace?
case SkRRect::kNinePatch_Type: | ||
case SkRRect::kComplex_Type: { | ||
CGMutablePathRef mutablePathRef = CGPathCreateMutable(); | ||
// Complex types, we manually add each cornor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/cornor/corner/ (here and below)
flow/layers/clip_rect_layer.cc
Outdated
@@ -50,13 +50,15 @@ void ClipRectLayer::Paint(PaintContext& context) const { | |||
SkAutoCanvasRestore save(context.internal_nodes_canvas, true); | |||
context.internal_nodes_canvas->clipRect(clip_rect_, | |||
clip_behavior_ != Clip::hardEdge); | |||
context.view_embedder->transformStack->pushClipRect(clip_rect_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
view_embedder can be null, protect against it in all call sites.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
flow/embedded_views.h
Outdated
class EmbeddedViewParams { | ||
public: | ||
SkPoint offsetPixels; | ||
SkSize sizePoints; | ||
std::shared_ptr<FlutterEmbededViewTransformStack> transformStack; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why a shared pointer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both EmbededViewParams
and ExternalViewEmbedder
holds a reference to this object. I might be wrong but I thought I had to use shared for it?
flow/embedded_views.h
Outdated
@@ -42,8 +56,88 @@ class ExternalViewEmbedder { | |||
virtual ~ExternalViewEmbedder() = default; | |||
|
|||
FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); | |||
|
|||
#pragma mark - transforms | |||
std::shared_ptr<FlutterEmbededViewTransformStack> transformStack; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the state of an ongoing paint traversal, I do not think it belongs in ExternalViewEmbedder(e.g think of an hypothetical world where we'll be painting multiple parts of the tree in parallel). I'd keep this in PaintContext.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Do you think all the stack related classes belong to the same file of paintContext?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
flow/embedded_views.h
Outdated
|
||
}; // ExternalViewEmbedder | ||
|
||
class FlutterEmbededViewTransformStack { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our language is a little ambiguous around "transforms", e.g usually we mean the transform matrix when we say transforms. I don't have a good idea though.
Update: I asked @chinmaygarde and he suggested to call it mutators, I think it's a much more reasonable name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
flow/embedded_views.h
Outdated
// and destroys it. | ||
void pop(); | ||
|
||
// Returns the iterator points to the bottom of the stack. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just from reading this comment I'm not sure what's the "bottom of the stack" the mutator that should apply to all other mutators in the stack, or the one that all other mutators applies to? can you make this more clear in the comment ?(for both iterators)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Updated to
// Returns the iterator points to the bottom of the stack.
// When we composite an embedded view, this is the first mutator we should apply to the view.
// And we should iterate through all the mutators until we reach `rend()` and apply all the mutations to the view along the way.
Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
// A stack of mutators that can be applied to an embedded platform view.
//
// The stack may include mutators like transforms and clips, each mutator applies to all the mutators that are below it in the stack
// and to the embedded view.
//
// For example consider the following stack: [T1, T2, T3], where T1 is the top of the stack and T3 is the bottom of the stack.
// Applying this mutators stack to a platform view P1 will result in T1(T2(T2(P1))).
class MutatorsStack {
...
// Returns an iterator pointing to the top of the stack.
std::vector<EmbeddedViewMutator>::reverse_iterator top();
// Returns an iterator pointing to the bottom of the stack.
std::vector<EmbeddedViewMutator>::reverse_iterator bottom();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
flow/embedded_views.h
Outdated
|
||
}; // ExternalViewEmbedder | ||
|
||
class FlutterEmbededViewTransformStack { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add unit tests for this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
flow/embedded_views.h
Outdated
// and destroys it. | ||
void pop(); | ||
|
||
// Returns the iterator points to the bottom of the stack. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
// A stack of mutators that can be applied to an embedded platform view.
//
// The stack may include mutators like transforms and clips, each mutator applies to all the mutators that are below it in the stack
// and to the embedded view.
//
// For example consider the following stack: [T1, T2, T3], where T1 is the top of the stack and T3 is the bottom of the stack.
// Applying this mutators stack to a platform view P1 will result in T1(T2(T2(P1))).
class MutatorsStack {
...
// Returns an iterator pointing to the top of the stack.
std::vector<EmbeddedViewMutator>::reverse_iterator top();
// Returns an iterator pointing to the bottom of the stack.
std::vector<EmbeddedViewMutator>::reverse_iterator bottom();
}
flow/embedded_views.cc
Outdated
@@ -6,7 +6,46 @@ | |||
|
|||
namespace flutter { | |||
|
|||
ExternalViewEmbedder::ExternalViewEmbedder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
flow/embedded_views.h
Outdated
@@ -29,7 +38,7 @@ class EmbeddedViewParams { | |||
// FlutterPlatformViewsController which is owned by FlutterViewController. | |||
class ExternalViewEmbedder { | |||
public: | |||
ExternalViewEmbedder() = default; | |||
ExternalViewEmbedder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
flow/embedded_views.h
Outdated
@@ -46,7 +55,86 @@ class ExternalViewEmbedder { | |||
virtual ~ExternalViewEmbedder() = default; | |||
|
|||
FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); | |||
}; | |||
|
|||
std::shared_ptr<EmbeddedViewMutatorStack> transformStack; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep this in PaintContext
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
EmbeddedViewMutator element = EmbeddedViewMutator(); | ||
element.setType(clip_rect); | ||
element.setRect(rect); | ||
vector_.push_back(element); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we can save some copies by making it a vector of unique pointers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried it naively and caused some issue when trying to copy the stack to the params, reverting the change and adding a TODO and will work on it in future.
@@ -88,8 +113,10 @@ class FlutterPlatformViewsController { | |||
std::map<std::string, fml::scoped_nsobject<NSObject<FlutterPlatformViewFactory>>> factories_; | |||
std::map<int64_t, fml::scoped_nsobject<NSObject<FlutterPlatformView>>> views_; | |||
std::map<int64_t, fml::scoped_nsobject<FlutterTouchInterceptingView>> touch_interceptors_; | |||
std::map<int64_t, fml::scoped_nsobject<UIView>> root_views_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment explaining this and clip_count_?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
CGRect frame = CGRectMake(0, 0, params.sizePoints.width(), params.sizePoints.height()); | ||
touch_interceptor.frame = frame; | ||
UIView* lastView = touch_interceptor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't immediately clear to me what's lastView
, the logic here is not completely trivial, maybe we can tell the story better by being a little more verbose on comment and names, maybe something like(pseudo code):
// Build a chain of UIViews that applies the mutations described by mutatorsStack.
//
// Clips are applied by adding a super view with the CALayer mask. Transforms are applied to the view that's at the
// head of the chain. For example the following mutators stack [T_1, C_2, T_3, T_4, C_5, T_6] where T denotes a
// transform and C denotes a clip, will result in the following UIView tree:
//
// C_2 -> C_5 -> PLATFORM_VIEW
// (PLATFORM_VIEW is a subview of C_5 which is a subview of C_2)
//
// T_1 is applied to C_2, T_3 and T_4 are applied to C_5, and T_6 is applied to PLATFORM_VIEW.
//
// UIView instances used for clip mutations are re-used when possible. [ADD AN EXAMPLE HERE]
//
// Returns the UIView that's at the top of the generated chain(C_2 for the example above).
UIView* FlutterPlatformViewsController::ApplyMutators(ViewMutatorsStack& mutators_stack, UIView* embedded_view) {
UIView* head = embedded_view; // [instead of the current variable named lastView]
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
// If we have less cilp operations this time, remove unnecessary views. | ||
// We skip this process if we have more clip operations this time. | ||
int64_t extraClipsFromLastTime = clip_count_[view_id] - clipCount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd keep this part in the proposed ApplyMutators
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
// If we have less cilp operations this time, remove unnecessary views. | ||
// We skip this process if we have more clip operations this time. | ||
int64_t extraClipsFromLastTime = clip_count_[view_id] - clipCount; | ||
if (extraClipsFromLastTime > 0 && lastView.superview) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like the following will look simpler:
if (!hasExtraClipViews) {
return head;
}
[head removeFromSuperView];
[root_views_[view_id] removeFromSuperView];
[root_views_[view_id] release];
[root_views_[view_id] = [head retain];
// here add head to flutter_view_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
[superview release]; | ||
superview = superSuperView; | ||
} | ||
[flutter_view_.get() addSubview:lastView]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might change the composition order in an unwanted way when we have multiple platform views, e.g [P_1, P_2] (in this order), and now the clip chain for P_1 changes, we remove the chain for P_1 and add it back to flutter_view_ which results in flipping the order([P_2, P]).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -182,6 +183,117 @@ | |||
return canvases; | |||
} | |||
|
|||
int FlutterPlatformViewsController::GetNumberOfClips(const MutatorsStack& mutators_stack) { | |||
// Apply transforms/clips. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stray comment?
UIView* FlutterPlatformViewsController::ReconstructClipViewsChain(int number_of_clips, | ||
UIView* child, | ||
UIView* parent) { | ||
NSInteger index = -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe call this indexInFlutterView
?
if (!needAddNewView && head != parent) { | ||
[head removeFromSuperview]; | ||
} | ||
if (index > -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: add a comment saying a value of -1 means that the chain was not attached to the FlutterView
UIView* parent) { | ||
NSInteger index = -1; | ||
if (parent.superview) { | ||
// TODO(cyanglaz): protentially cache the index of oldPlatformViewRoot to make this a O(1). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File an issue so we don't lose track of this.
return clipCount; | ||
} | ||
|
||
UIView* FlutterPlatformViewsController::ReconstructClipViewsChain(int number_of_clips, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what child
and parent
mean in this context... would it make sense to call it platform_view
and head_clip_view
?
case clip_rect: | ||
case clip_rrect: | ||
case clip_path: { | ||
UIView* clipView = head.superview; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe cast this to a ChildClippingView already here?
We can add a comment saying that this is called after ReconstructClipViewsChain
which assures we have a ChildClippingView parent for each clip in the stack.
@@ -122,6 +152,33 @@ class FlutterPlatformViewsController { | |||
void EnsureGLOverlayInitialized(int64_t overlay_id, | |||
std::shared_ptr<IOSGLContext> gl_context, | |||
GrContext* gr_context); | |||
// Traverse the `mutators_stack` and return the number of clip operations. | |||
int GetNumberOfClips(const MutatorsStack& mutators_stack); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: CountClips
// numbers of UIViews between the `child` and the `parent` including the `parent`. Returns the new | ||
// parent after reconsturcting. If the returning parent is not the same as child, it will be an | ||
// instance of `ChildClippingView`. | ||
UIView* ReconstructClipViewsChain(int number_of_clips, UIView* child, UIView* parent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
// Make sure that platform_view has exactly clip_count ChildClippingView ancestors.
//
// Existing ChildClippingViews are re-used. If there are currently more ChildClippingView ancestors than needed
// the extra views are detached. If there are less ChildClippingView ancestors than needed new ChildClippingViews will be added.
//
// If head_clip_view was attached as a subview to FlutterView, the head of the newly constructed ChildClippingViews chain is attached to FlutterView in the same position.
//
// Returns the new head of the clip views chain.
// instance of `ChildClippingView`. | ||
UIView* ReconstructClipViewsChain(int number_of_clips, UIView* child, UIView* parent); | ||
|
||
// Apply mutators in the mutators_stack to the UIView chain that was constructed by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: grammar - Applies the mutators
// After each clip operation, we update the head to the super view of the current head. | ||
void ApplyMutators(const MutatorsStack& mutators_stack, UIView* embedded_view); | ||
|
||
// Composite the platform view with the passed in `params`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment doesn't add much information on top of the method and argument names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This reverts commit ebb5b90. Seeing the following breakage on host build: ``` ../../flutter/flow/scene_update_context.cc:205:36: error: non-const lvalue reference to type 'flutter::MutatorsStack' cannot bind to a value of unrelated type 'const flutter::Stopwatch' frame.context().raster_time(), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../flutter/flow/scene_update_context.cc:207:36: error: no viable conversion from 'flutter::TextureRegistry' to 'const flutter::Stopwatch' frame.context().texture_registry(), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../flutter/flow/instrumentation.h:55:32: note: candidate constructor not viable: no known conversion from 'flutter::TextureRegistry' to 'const flutter::Stopwatch &' for 1st argument FML_DISALLOW_COPY_AND_ASSIGN(Stopwatch); ^ ../../flutter/fml/macros.h:28:3: note: expanded from macro 'FML_DISALLOW_COPY_AND_ASSIGN' TypeName(const TypeName&) = delete; \ ^ ../../flutter/flow/scene_update_context.cc:208:36: error: non-const lvalue reference to type 'flutter::TextureRegistry' cannot bind to a temporary of type 'flutter::RasterCache *' &frame.context().raster_cache(), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../flutter/flow/scene_update_context.cc:209:36: error: cannot initialize a member subobject of type 'const flutter::RasterCache *' with an rvalue of type 'bool' false}; ^~~~~ ```
This reverts commit ebb5b90. Seeing the following breakage on host build: ``` ../../flutter/flow/scene_update_context.cc:205:36: error: non-const lvalue reference to type 'flutter::MutatorsStack' cannot bind to a value of unrelated type 'const flutter::Stopwatch' frame.context().raster_time(), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../flutter/flow/scene_update_context.cc:207:36: error: no viable conversion from 'flutter::TextureRegistry' to 'const flutter::Stopwatch' frame.context().texture_registry(), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../flutter/flow/instrumentation.h:55:32: note: candidate constructor not viable: no known conversion from 'flutter::TextureRegistry' to 'const flutter::Stopwatch &' for 1st argument FML_DISALLOW_COPY_AND_ASSIGN(Stopwatch); ^ ../../flutter/fml/macros.h:28:3: note: expanded from macro 'FML_DISALLOW_COPY_AND_ASSIGN' TypeName(const TypeName&) = delete; \ ^ ../../flutter/flow/scene_update_context.cc:208:36: error: non-const lvalue reference to type 'flutter::TextureRegistry' cannot bind to a temporary of type 'flutter::RasterCache *' &frame.context().raster_cache(), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../flutter/flow/scene_update_context.cc:209:36: error: cannot initialize a member subobject of type 'const flutter::RasterCache *' with an rvalue of type 'bool' false}; ^~~~~ ```
…lutter#9480)" This reverts commit 00d929f.
flutter/engine@ae8e6d9...107fe82 git log ae8e6d9..107fe82 --no-merges --oneline 107fe82 Add --observatory-host switch (flutter/engine#9485) 20a76de Roll src/third_party/skia ebbc82c02471..69881fb0b5fb (11 commits) (flutter/engine#9479) 00d929f Revert "IOS Platform view transform/clipping (#9075)" (flutter/engine#9480) 3390019 fix NPE when a touch event is sent to an unknown Android platform view (flutter/engine#9476) ebb5b90 IOS Platform view transform/clipping (flutter/engine#9075) 13145e9 ios-unit-tests: Started using rsync instead of cp -R to copy frameworks. (flutter/engine#9471) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (chinmaygarde@google.com), and stop the roller if necessary.
flutter/engine@ae8e6d9...107fe82 git log ae8e6d9..107fe82 --no-merges --oneline 107fe82 Add --observatory-host switch (flutter/engine#9485) 20a76de Roll src/third_party/skia ebbc82c02471..69881fb0b5fb (11 commits) (flutter/engine#9479) 00d929f Revert &flutter#34;IOS Platform view transform/clipping (flutter#9075)&flutter#34; (flutter/engine#9480) 3390019 fix NPE when a touch event is sent to an unknown Android platform view (flutter/engine#9476) ebb5b90 IOS Platform view transform/clipping (flutter/engine#9075) 13145e9 ios-unit-tests: Started using rsync instead of cp -R to copy frameworks. (flutter/engine#9471) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (chinmaygarde@google.com), and stop the roller if necessary.
WIP:
Introduce
FlutterEmbededViewTransformStack
to store all the transforms/clips layer before a platform view.When we composite platform view, we traverse the stack and apply each transform/clip. If it is a transform we apply to the current view; if it is a clip, we create a super view and apply clip to the super view, then set the current view to this super view.
We also check whether the clip operation required for the current composition is the same as the old composition. If they are the same, we reuse the old UIView stacks; if not same, we add/remove views accordingly.
===============================
Another solution to this would be creating one UIView (say
ClipView
) for all the clips. So when we see a transform node, we apply the transform to the platform view directly. When we see a clip, we apply thetotalMatrix
until the clip to the clip and append to the current clip path. When we finish loop through all the nodes, we apply the total clip to theClipView
.I am happy to try to implement this with the alternative solution to compare the performance.
@amirh