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 16 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
853cbd6
get the stack ready
May 16, 2019
72042cf
draft
May 17, 2019
8489c08
Do nothing if the params didn't change when compositing platform views.
May 17, 2019
647e66b
merge
May 17, 2019
f7d7aaa
draft
May 17, 2019
c0741b2
draft
May 18, 2019
6e95d26
draft2
May 24, 2019
a3e1d61
clip rrect
May 28, 2019
4cf9f8a
typo fix
May 28, 2019
1daacb0
Merge branch 'master' into platform_view_transform
May 28, 2019
04d1f40
counting previous clips and only add and remove uiview when number of…
May 29, 2019
5b05eb2
draft
May 30, 2019
9b290cb
draft
May 30, 2019
52a8252
comments and formatting
May 31, 2019
3a899fa
english
May 31, 2019
cabc67a
remove friend from operator==
May 31, 2019
4496b21
merge master
Jun 3, 2019
4bdae98
fix issue caused by merging
Jun 3, 2019
33513b9
review fixes
Jun 3, 2019
e8926a5
remove tranform\cilpping util method into a different file and add th…
Jun 4, 2019
3f4c586
rename after review
Jun 4, 2019
d4f88f2
move stack to paint context
Jun 4, 2019
4eeb64f
draft vector with unique_ptr
Jun 4, 2019
6d78901
more review fixes
Jun 4, 2019
e494853
unittests for mutator stack
Jun 5, 2019
81e5c19
move stack to paint context complete
Jun 5, 2019
ea11fad
formatting
Jun 5, 2019
2c79050
Merge branch 'master' into platform_view_transform
Jun 5, 2019
0f70a44
mutator stack compare fixes
Jun 6, 2019
32d2681
mutator stack compare fixes
Jun 6, 2019
b874420
revert the change that made the vector containing unique_ptr
Jun 6, 2019
a1794de
formatting
Jun 6, 2019
2f0125c
add TODO
Jun 6, 2019
d22acbd
fix a bug where the reused clip view doesn't reset transform
Jun 10, 2019
2b08642
addition to last fix
Jun 10, 2019
1599067
adding touch transparent view
Jun 10, 2019
a027fa0
some review fixes
Jun 11, 2019
12ff43a
union done
Jun 11, 2019
a9a660f
draft
Jun 11, 2019
f4b9503
review fixes
Jun 12, 2019
7c0fa63
draft
Jun 12, 2019
2eb40d6
embeded param does not delete stack when destructed
Jun 12, 2019
6d4c82c
Merge branch 'platform_view_transform' into platform_view_transform_u…
Jun 12, 2019
7d1d0e5
copy constructor for Mutator
Jun 12, 2019
46098a4
more tests
Jun 12, 2019
fa1248a
remove the move to
Jun 12, 2019
dd0eed8
merge
Jun 12, 2019
50843a0
share ptr for vector_
Jun 12, 2019
31ec192
Merge branch 'master' into platform_view_transform
Jun 12, 2019
3b6b3b0
formatting
Jun 12, 2019
b4b59ca
fixes unittest
Jun 12, 2019
ee57e42
Update BUILD.gn
iskakaushik Jun 12, 2019
979d251
license file fix
Jun 12, 2019
b45ad9a
some review fixes
Jun 13, 2019
b40944e
another fix
Jun 13, 2019
701afe5
formatting
Jun 14, 2019
551445d
add todo
Jun 14, 2019
8622d13
move back some accidentally deleted code
Jun 14, 2019
d46290e
draft
Jun 19, 2019
8f86532
refactoring
Jun 19, 2019
a5b7f8a
review fixes
Jun 20, 2019
b0d3110
formatting
Jun 20, 2019
ff442ee
let submit frame handle adding the head if detached
Jun 20, 2019
5f0a513
Merge branch 'master' into platform_view_transform
Jun 20, 2019
455a495
logs
Jun 20, 2019
cf36c46
move insert back uiview logic into recontruct method
Jun 21, 2019
fd498ee
rever unrelated changes
Jun 21, 2019
78acc1f
review fixes
Jun 24, 2019
aeb56e0
Merge branch 'master' into platform_view_transform
Jun 24, 2019
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
44 changes: 44 additions & 0 deletions flow/embedded_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,51 @@

namespace flutter {

ExternalViewEmbedder::ExternalViewEmbedder()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

revert

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

: transformStack(std::make_shared<FlutterEmbededViewTransformStack>()){};

bool ExternalViewEmbedder::SubmitFrame(GrContext* context) {
return false;
};

#pragma mark - FlutterEmbededViewTransformStack

void FlutterEmbededViewTransformStack::pushClipRect(const SkRect& rect) {
FlutterEmbededViewTransformElement element =
FlutterEmbededViewTransformElement();
element.setType(clip_rect);
element.setRect(rect);
vector_.push_back(element);

Copy link
Copy Markdown
Contributor

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

@cyanglaz cyanglaz Jun 6, 2019

Copy link
Copy Markdown
Contributor Author

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.

};

void FlutterEmbededViewTransformStack::pushClipRRect(const SkRRect& rrect) {
FlutterEmbededViewTransformElement element =
FlutterEmbededViewTransformElement();
element.setType(clip_rrect);
element.setRRect(rrect);
vector_.push_back(element);
};

void FlutterEmbededViewTransformStack::pushTransform(const SkMatrix& matrix) {
FlutterEmbededViewTransformElement element =
FlutterEmbededViewTransformElement();
element.setType(transform);
element.setMatrix(matrix);
vector_.push_back(element);
};

void FlutterEmbededViewTransformStack::pop() {
vector_.pop_back();
}

std::vector<FlutterEmbededViewTransformElement>::reverse_iterator
FlutterEmbededViewTransformStack::rbegin() {
return vector_.rbegin();
}

std::vector<FlutterEmbededViewTransformElement>::reverse_iterator
FlutterEmbededViewTransformStack::rend() {
return vector_.rend();
}

} // namespace flutter
96 changes: 95 additions & 1 deletion flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,37 @@

#include "flutter/fml/memory/ref_counted.h"
#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkPath.h"
#include "third_party/skia/include/core/SkPoint.h"
#include "third_party/skia/include/core/SkRRect.h"
#include "third_party/skia/include/core/SkRect.h"
#include "third_party/skia/include/core/SkSize.h"

namespace flutter {

class FlutterEmbededViewTransformStack;
class FlutterEmbededViewTransformElement;

class EmbeddedViewParams {
public:
SkPoint offsetPixels;
SkSize sizePoints;
std::shared_ptr<FlutterEmbededViewTransformStack> transformStack;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why a shared pointer?

Copy link
Copy Markdown
Contributor Author

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?


friend bool operator==(const EmbeddedViewParams& lhs,
Comment thread
cyanglaz marked this conversation as resolved.
Outdated
const EmbeddedViewParams& rhs) {
return lhs.offsetPixels == rhs.offsetPixels &&
lhs.sizePoints == rhs.sizePoints &&
lhs.transformStack == rhs.transformStack;
}
};

// This is only used on iOS when running in a non headless mode,
// in this case ExternalViewEmbedder is a reference to the
// FlutterPlatformViewsController which is owned by FlutterViewController.
class ExternalViewEmbedder {
public:
ExternalViewEmbedder() = default;
ExternalViewEmbedder();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

revert

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


virtual void BeginFrame(SkISize frame_size) = 0;

Expand All @@ -42,8 +56,88 @@ class ExternalViewEmbedder {
virtual ~ExternalViewEmbedder() = default;

FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder);

#pragma mark - transforms

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

std::shared_ptr<FlutterEmbededViewTransformStack> transformStack;

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done


}; // ExternalViewEmbedder

class FlutterEmbededViewTransformStack {

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

public:
void pushClipRect(const SkRect& rect);
void pushClipRRect(const SkRRect& rrect);
void pushClipPath(const SkPath& path);

void pushTransform(const SkMatrix& matrix);

// Removes the `FlutterEmbededViewTransformElement` on the top of the stack
// and destroys it.
void pop();

// Returns the iterator points to the bottom of the stack.

Copy link
Copy Markdown
Contributor

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)

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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();
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

std::vector<FlutterEmbededViewTransformElement>::reverse_iterator rbegin();
// Returns the iterator points to the top of the stack.
std::vector<FlutterEmbededViewTransformElement>::reverse_iterator rend();

bool operator==(const FlutterEmbededViewTransformStack& other) const {
return vector_ == other.vector_;
}

private:
std::vector<FlutterEmbededViewTransformElement> vector_;
}; // FlutterEmbededViewTransformStack

enum FlutterEmbededViewTransformType {
clip_rect,
clip_rrect,
clip_path,
transform
};

class FlutterEmbededViewTransformElement {
public:
void setType(const FlutterEmbededViewTransformType type) { type_ = type; }
void setRect(const SkRect& rect) { rect_ = rect; }
void setRRect(const SkRRect& rrect) { rrect_ = rrect; }
void setMatrix(const SkMatrix& matrix) { matrix_ = matrix; }

FlutterEmbededViewTransformType type() { return type_; }
SkRect rect() { return rect_; }
SkRRect rrect() { return rrect_; }
SkPath path() { return path_; }
SkMatrix matrix() { return matrix_; }

bool operator==(const FlutterEmbededViewTransformElement& other) const {
if (type_ != other.type_) {
return false;
}
if (type_ == clip_rect && rect_ == other.rect_) {
return true;
}
if (type_ == clip_rect && rrect_ == other.rrect_) {
return true;
}
if (type_ == clip_path && path_ == other.path_) {
return true;
}
if (type_ == transform && matrix_ == other.matrix_) {
return true;
}
return false;
}

bool isClipType() {
return type_ == clip_rect || type_ == clip_rrect || type_ == clip_path;
}

private:
FlutterEmbededViewTransformType type_;
SkRect rect_;
SkRRect rrect_;
SkPath path_;
SkMatrix matrix_;
}; // FlutterEmbededViewTransformElement

} // namespace flutter

#endif // FLUTTER_FLOW_EMBEDDED_VIEWS_H_
2 changes: 2 additions & 0 deletions flow/layers/clip_rect_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

if (clip_behavior_ == Clip::antiAliasWithSaveLayer) {
context.internal_nodes_canvas->saveLayer(clip_rect_, nullptr);
}
PaintChildren(context);
if (clip_behavior_ == Clip::antiAliasWithSaveLayer) {
context.internal_nodes_canvas->restore();
}
context.view_embedder->transformStack->pop();
}

} // namespace flutter
2 changes: 2 additions & 0 deletions flow/layers/clip_rrect_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ void ClipRRectLayer::Paint(PaintContext& context) const {
SkAutoCanvasRestore save(context.internal_nodes_canvas, true);
context.internal_nodes_canvas->clipRRect(clip_rrect_,
clip_behavior_ != Clip::hardEdge);
context.view_embedder->transformStack->pushClipRRect(clip_rrect_);
if (clip_behavior_ == Clip::antiAliasWithSaveLayer) {
context.internal_nodes_canvas->saveLayer(paint_bounds(), nullptr);
}
PaintChildren(context);
context.view_embedder->transformStack->pop();
if (clip_behavior_ == Clip::antiAliasWithSaveLayer) {
context.internal_nodes_canvas->restore();
}
Expand Down
1 change: 1 addition & 0 deletions flow/layers/platform_view_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void PlatformViewLayer::Paint(PaintContext& context) const {
params.offsetPixels =
SkPoint::Make(transform.getTranslateX(), transform.getTranslateY());
params.sizePoints = size_;
params.transformStack = context.view_embedder->transformStack;

SkCanvas* canvas =
context.view_embedder->CompositeEmbeddedView(view_id_, params);
Expand Down
2 changes: 2 additions & 0 deletions flow/layers/transform_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ void TransformLayer::Paint(PaintContext& context) const {

SkAutoCanvasRestore save(context.internal_nodes_canvas, true);
context.internal_nodes_canvas->concat(transform_);
context.view_embedder->transformStack->pushTransform(transform_);
PaintChildren(context);
context.view_embedder->transformStack->pop();
}

} // namespace flutter
Loading