Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add GetBoundingRectAfterMutations to EmbeddedViewParams to calculate the final bounding rect for platform view #19170

Merged
cyanglaz merged 8 commits into from
Jun 22, 2020
Merged

Add GetBoundingRectAfterMutations to EmbeddedViewParams to calculate the final bounding rect for platform view #19170

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
86ba0b1
GetBoundingRectAfterMutations
Jun 19, 2020
2752b19
link todo to issue
Jun 19, 2020
617ead7
fix typo
Jun 19, 2020
60d9b7b
fix unittest typo
Jun 19, 2020
9eb1d3a
cache make bounding rect
Jun 20, 2020
822d11c
fix license golden
Jun 20, 2020
ff81b85
add docs
Jun 20, 2020
15056ff
formatting
Jun 20, 2020
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
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ FILE: ../../../flutter/common/task_runners.cc
FILE: ../../../flutter/common/task_runners.h
FILE: ../../../flutter/flow/compositor_context.cc
FILE: ../../../flutter/flow/compositor_context.h
FILE: ../../../flutter/flow/embedded_view_params_unittests.cc
FILE: ../../../flutter/flow/embedded_views.cc
FILE: ../../../flutter/flow/embedded_views.h
FILE: ../../../flutter/flow/gl_context_switch.cc
Expand Down
1 change: 1 addition & 0 deletions flow/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ executable("flow_unittests") {
testonly = true

sources = [
"embedded_view_params_unittests.cc",
"flow_run_all_unittests.cc",
"flow_test_utils.cc",
"flow_test_utils.h",
Expand Down
98 changes: 98 additions & 0 deletions flow/embedded_view_params_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/flow/embedded_views.h"
#include "flutter/fml/logging.h"
#include "gtest/gtest.h"

namespace flutter {
namespace testing {

TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithNoMutations) {
MutatorsStack stack;
SkMatrix matrix;

EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
const SkRect& rect = params.finalBoundingRect();
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), 0));
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0));
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 1));
}

TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithScale) {
MutatorsStack stack;
SkMatrix matrix = SkMatrix::Scale(2, 2);
stack.PushTransform(matrix);

EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
const SkRect& rect = params.finalBoundingRect();
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), 0));
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0));
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 2));
ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 2));
}

TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithTranslate) {
MutatorsStack stack;
SkMatrix matrix = SkMatrix::MakeTrans(1, 1);
stack.PushTransform(matrix);

EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
const SkRect& rect = params.finalBoundingRect();
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), 1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 1));
}

TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithRotation90) {
MutatorsStack stack;
SkMatrix matrix;
matrix.setRotate(90);
stack.PushTransform(matrix);

EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
const SkRect& rect = params.finalBoundingRect();

FML_DLOG(ERROR) << rect.x();
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0));
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 1));
}

TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithRotation45) {
MutatorsStack stack;
SkMatrix matrix;
matrix.setRotate(45);
stack.PushTransform(matrix);

EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
const SkRect& rect = params.finalBoundingRect();
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -sqrt(2) / 2));
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0));
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), sqrt(2)));
ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), sqrt(2)));
}

TEST(EmbeddedViewParams,
GetBoundingRectAfterMutationsWithTranslateScaleAndRotation) {
SkMatrix matrix = SkMatrix::MakeTrans(2, 2);
matrix.preScale(3, 3);
matrix.preRotate(90);

MutatorsStack stack;
stack.PushTransform(matrix);

EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
const SkRect& rect = params.finalBoundingRect();
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 2));
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 3));
ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 3));
}

} // namespace testing
} // namespace flutter
45 changes: 36 additions & 9 deletions flow/embedded_views.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,21 +186,48 @@ class EmbeddedViewParams {
public:
EmbeddedViewParams() = default;

EmbeddedViewParams(SkMatrix matrix,
SkSize size_points,
MutatorsStack mutators_stack)
: matrix_(matrix),
size_points_(size_points),
mutators_stack_(mutators_stack) {
offset_pixels_ =
SkPoint::Make(matrix.getTranslateX(), matrix.getTranslateY());
SkPath path;
SkRect starting_rect = SkRect::MakeSize(size_points);
path.addRect(starting_rect);
path.transform(matrix);
final_bounding_rect_ = path.computeTightBounds();
}

EmbeddedViewParams(const EmbeddedViewParams& other) {
offsetPixels = other.offsetPixels;
sizePoints = other.sizePoints;
mutatorsStack = other.mutatorsStack;
offset_pixels_ = other.offset_pixels_;
size_points_ = other.size_points_;
mutators_stack_ = other.mutators_stack_;
matrix_ = other.matrix_;
final_bounding_rect_ = other.final_bounding_rect_;
};

SkPoint offsetPixels;
SkSize sizePoints;
MutatorsStack mutatorsStack;
const SkPoint& offsetPixels() const { return offset_pixels_; };
const SkSize& sizePoints() const { return size_points_; };
const MutatorsStack& mutatorsStack() const { return mutators_stack_; };
const SkRect& finalBoundingRect() const { return final_bounding_rect_; }

@blasten blasten Jun 20, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: could we document these methods?

@cyanglaz cyanglaz Jun 20, 2020

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


bool operator==(const EmbeddedViewParams& other) const {
return offsetPixels == other.offsetPixels &&
sizePoints == other.sizePoints &&
mutatorsStack == other.mutatorsStack;
return offset_pixels_ == other.offset_pixels_ &&
size_points_ == other.size_points_ &&
mutators_stack_ == other.mutators_stack_ &&
final_bounding_rect_ == other.final_bounding_rect_ &&
matrix_ == other.matrix_;
}

private:
SkMatrix matrix_;
SkPoint offset_pixels_;
SkSize size_points_;
MutatorsStack mutators_stack_;
SkRect final_bounding_rect_;
};

enum class PostPrerollResult { kResubmitFrame, kSuccess };
Expand Down
7 changes: 2 additions & 5 deletions flow/layers/platform_view_layer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,8 @@ void PlatformViewLayer::Preroll(PrerollContext* context,
}
context->has_platform_view = true;
std::unique_ptr<EmbeddedViewParams> params =
std::make_unique<EmbeddedViewParams>();
params->offsetPixels =
SkPoint::Make(matrix.getTranslateX(), matrix.getTranslateY());
params->sizePoints = size_;
params->mutatorsStack = context->mutators_stack;
std::make_unique<EmbeddedViewParams>(matrix, size_,
context->mutators_stack);
context->view_embedder->PrerollCompositeEmbeddedView(view_id_,
std::move(params));
}
Expand Down
7 changes: 4 additions & 3 deletions shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,13 +407,14 @@

void FlutterPlatformViewsController::CompositeWithParams(int view_id,
const EmbeddedViewParams& params) {
CGRect frame = CGRectMake(0, 0, params.sizePoints.width(), params.sizePoints.height());
CGRect frame = CGRectMake(0, 0, params.sizePoints().width(), params.sizePoints().height());
UIView* touchInterceptor = touch_interceptors_[view_id].get();
touchInterceptor.layer.transform = CATransform3DIdentity;
touchInterceptor.frame = frame;
touchInterceptor.alpha = 1;

int currentClippingCount = CountClips(params.mutatorsStack);
const MutatorsStack& mutatorStack = params.mutatorsStack();
int currentClippingCount = CountClips(mutatorStack);
int previousClippingCount = clip_count_[view_id];
if (currentClippingCount != previousClippingCount) {
clip_count_[view_id] = currentClippingCount;
Expand All @@ -424,7 +425,7 @@
ReconstructClipViewsChain(currentClippingCount, touchInterceptor, oldPlatformViewRoot);
root_views_[view_id] = fml::scoped_nsobject<UIView>([newPlatformViewRoot retain]);
}
ApplyMutators(params.mutatorsStack, touchInterceptor);
ApplyMutators(mutatorStack, touchInterceptor);
}

SkCanvas* FlutterPlatformViewsController::CompositeEmbeddedView(int view_id) {
Expand Down
10 changes: 5 additions & 5 deletions shell/platform/embedder/embedder_layers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ void EmbedderLayers::PushPlatformViewLayer(
view.struct_size = sizeof(FlutterPlatformView);
view.identifier = identifier;

const auto& mutators = params.mutatorsStack;
const auto& mutators = params.mutatorsStack();

std::vector<const FlutterPlatformViewMutation*> mutations_array;

Expand Down Expand Up @@ -180,10 +180,10 @@ void EmbedderLayers::PushPlatformViewLayer(
layer.platform_view = platform_views_referenced_.back().get();

const auto layer_bounds =
SkRect::MakeXYWH(params.offsetPixels.x(), //
params.offsetPixels.y(), //
params.sizePoints.width() * device_pixel_ratio_, //
params.sizePoints.height() * device_pixel_ratio_ //
SkRect::MakeXYWH(params.offsetPixels().x(), //

@blasten blasten Jun 20, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this just finalBoundingRect() now?

@cyanglaz cyanglaz Jun 20, 2020

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 think the size is not the same as the bounding rect. The size points are the original size before applying mutation.

@cyanglaz cyanglaz Jun 20, 2020

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.

@chinmaygarde I thought params.sizePoints did not consider scaling and rotation at all? Does embedder free from scaling and rotation layers?

@chinmaygarde chinmaygarde Jun 20, 2020

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 the embedder is supposed to apply the scaling and rotation themselves from the data present in the mutator stack to get the final width and height. I believe there is a test for this in the embedder unit-tests harness and this should be documented in the header.

@cyanglaz cyanglaz Jun 20, 2020

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 meant this rect returned from this method does not include rotation and scaling. Want to make sure if the original size used here is intended.

@chinmaygarde chinmaygarde Jun 21, 2020

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.

Yes. It is.

params.offsetPixels().y(), //
params.sizePoints().width() * device_pixel_ratio_, //
params.sizePoints().height() * device_pixel_ratio_ //
);

const auto transformed_layer_bounds =
Expand Down