From 32edab412c5471a52e8f158253077db3ee85f038 Mon Sep 17 00:00:00 2001 From: Daniel Nicoara Date: Mon, 13 Jan 2020 15:31:32 -0500 Subject: [PATCH 1/3] Fix embedder mutation ordering The mutator list sent to the embedder is in reverse order from the MutatorsStack. The root display transformation then needs to be appended at the end of the list. Given that these transforms are applied to a layer in display space the list needs to be reversed as well such that transforms can be applied in the right order to get from display space to surface space. --- shell/platform/embedder/embedder_layers.cc | 30 ++++--- shell/platform/embedder/fixtures/main.dart | 17 ++++ .../embedder/tests/embedder_unittests.cc | 84 +++++++++++++++++++ 3 files changed, 115 insertions(+), 16 deletions(-) diff --git a/shell/platform/embedder/embedder_layers.cc b/shell/platform/embedder/embedder_layers.cc index dc284e22746e9..c88fa56fe127d 100644 --- a/shell/platform/embedder/embedder_layers.cc +++ b/shell/platform/embedder/embedder_layers.cc @@ -112,20 +112,6 @@ void EmbedderLayers::PushPlatformViewLayer( std::vector mutations_array; - if (std::distance(mutators.Bottom(), mutators.Top()) > 0) { - // If there are going to be any mutations, they must first take into - // account the transformation for the device pixel ratio and root surface - // transformation. - auto base_xformation = - SkMatrix::Concat(root_surface_transformation_, - SkMatrix::MakeScale(device_pixel_ratio_)); - if (!base_xformation.isIdentity()) { - mutations_array.push_back( - mutations_referenced_.emplace_back(ConvertMutation(base_xformation)) - .get()); - } - } - for (auto i = mutators.Bottom(); i != mutators.Top(); ++i) { const auto& mutator = *i; switch (mutator->GetType()) { @@ -164,10 +150,22 @@ void EmbedderLayers::PushPlatformViewLayer( } } - if (mutations_array.size() > 0) { + if (!mutations_array.empty()) { + // If there are going to be any mutations, they must first take into + // account the transformation for the device pixel ratio and root surface + // transformation. + auto base_xformation = + SkMatrix::Concat(root_surface_transformation_, + SkMatrix::MakeScale(device_pixel_ratio_)); + if (!base_xformation.isIdentity()) { + mutations_array.push_back( + mutations_referenced_.emplace_back(ConvertMutation(base_xformation)) + .get()); + } + auto mutations = std::make_unique>( - mutations_array); + mutations_array.rbegin(), mutations_array.rend()); mutations_arrays_referenced_.emplace_back(std::move(mutations)); view.mutations_count = mutations_array.size(); diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart index 72430ebd08678..6368c25afc913 100644 --- a/shell/platform/embedder/fixtures/main.dart +++ b/shell/platform/embedder/fixtures/main.dart @@ -662,6 +662,23 @@ void scene_builder_with_clips() { window.scheduleFrame(); } +@pragma('vm:entry-point') +void scene_builder_with_complex_clips() { + window.onBeginFrame = (Duration duration) { + SceneBuilder builder = SceneBuilder(); + + builder.pushClipRect(Rect.fromLTRB(0.0, 0.0, 1024.0, 600.0)); + builder.pushOffset(512.0, 0.0); + builder.pushClipRect(Rect.fromLTRB(0.0, 0.0, 512.0, 600.0)); + builder.pushOffset(-256.0, 0.0); + builder.pushClipRect(Rect.fromLTRB(0.0, 0.0, 1024.0, 600.0)); + builder.addPlatformView(42, width: 1024.0, height: 600.0); + + builder.addPicture(Offset(0.0, 0.0), CreateGradientBox(Size(1024.0, 600.0))); + window.render(builder.build()); + }; + window.scheduleFrame(); +} void sendObjectToNativeCode(dynamic object) native 'SendObjectToNativeCode'; diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index fe192ca00a72c..7a89345fb8fa9 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -3606,6 +3606,90 @@ TEST_F(EmbedderTest, ClipsAreCorrectlyCalculated) { latch.Wait(); } +TEST_F(EmbedderTest, ComplexClipsAreCorrectlyCalculated) { + auto& context = GetEmbedderContext(); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(1024, 600)); + builder.SetCompositor(); + builder.SetDartEntrypoint("scene_builder_with_complex_clips"); + + const auto root_surface_transformation = + SkMatrix().preTranslate(0, 1024).preRotate(-90, 0, 0); + + context.SetRootSurfaceTransformation(root_surface_transformation); + + fml::AutoResetWaitableEvent latch; + context.GetCompositor().SetNextPresentCallback( + [&](const FlutterLayer** layers, size_t layers_count) { + ASSERT_EQ(layers_count, 3u); + + { + FlutterPlatformView platform_view = *layers[1]->platform_view; + platform_view.struct_size = sizeof(platform_view); + platform_view.identifier = 42; + + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypePlatformView; + layer.platform_view = &platform_view; + layer.size = FlutterSizeMake(600.0, 1024.0); + layer.offset = FlutterPointMake(0.0, -256.0); + + FML_LOG(ERROR) << (*layers[1]); + ASSERT_EQ(*layers[1], layer); + + const auto** mutations = platform_view.mutations; + + ASSERT_EQ(mutations[0]->type, + kFlutterPlatformViewMutationTypeTransformation); + ASSERT_EQ(SkMatrixMake(mutations[0]->transformation), + root_surface_transformation); + + ASSERT_EQ(mutations[1]->type, + kFlutterPlatformViewMutationTypeClipRect); + ASSERT_EQ(SkRectMake(mutations[1]->clip_rect), + SkRect::MakeLTRB(0.0, 0.0, 1024.0, 600.0)); + + ASSERT_EQ(mutations[2]->type, + kFlutterPlatformViewMutationTypeTransformation); + ASSERT_EQ(SkMatrixMake(mutations[2]->transformation), + SkMatrix::MakeTrans(512.0, 0.0)); + + ASSERT_EQ(mutations[3]->type, + kFlutterPlatformViewMutationTypeClipRect); + ASSERT_EQ(SkRectMake(mutations[3]->clip_rect), + SkRect::MakeLTRB(0.0, 0.0, 512.0, 600.0)); + + ASSERT_EQ(mutations[4]->type, + kFlutterPlatformViewMutationTypeTransformation); + ASSERT_EQ(SkMatrixMake(mutations[4]->transformation), + SkMatrix::MakeTrans(-256.0, 0.0)); + + ASSERT_EQ(mutations[5]->type, + kFlutterPlatformViewMutationTypeClipRect); + ASSERT_EQ(SkRectMake(mutations[5]->clip_rect), + SkRect::MakeLTRB(0.0, 0.0, 1024.0, 600.0)); + } + + latch.Signal(); + }); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 400; + event.height = 300; + event.pixel_ratio = 1.0; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + + latch.Wait(); +} + + TEST_F(EmbedderTest, ObjectsCanBePostedViaPorts) { auto& context = GetEmbedderContext(); EmbedderConfigBuilder builder(context); From 9131adab86f0f573196e4c0fdfe5d31e6bf3edf6 Mon Sep 17 00:00:00 2001 From: Daniel Nicoara Date: Tue, 14 Jan 2020 10:50:56 -0500 Subject: [PATCH 2/3] Dart format --- shell/platform/embedder/tests/embedder_unittests.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 7a89345fb8fa9..6c3ae1ce59a73 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -3689,7 +3689,6 @@ TEST_F(EmbedderTest, ComplexClipsAreCorrectlyCalculated) { latch.Wait(); } - TEST_F(EmbedderTest, ObjectsCanBePostedViaPorts) { auto& context = GetEmbedderContext(); EmbedderConfigBuilder builder(context); From 6c60626b35b83ca07211b009f1c78e5018e95d64 Mon Sep 17 00:00:00 2001 From: Daniel Nicoara Date: Tue, 14 Jan 2020 17:03:35 -0500 Subject: [PATCH 3/3] Remove device scale factor This is already in the mutator stack. --- shell/platform/embedder/embedder_layers.cc | 11 ++++------- shell/platform/embedder/tests/embedder_unittests.cc | 6 ++---- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/shell/platform/embedder/embedder_layers.cc b/shell/platform/embedder/embedder_layers.cc index c88fa56fe127d..d9e86ca1e049c 100644 --- a/shell/platform/embedder/embedder_layers.cc +++ b/shell/platform/embedder/embedder_layers.cc @@ -152,14 +152,11 @@ void EmbedderLayers::PushPlatformViewLayer( if (!mutations_array.empty()) { // If there are going to be any mutations, they must first take into - // account the transformation for the device pixel ratio and root surface - // transformation. - auto base_xformation = - SkMatrix::Concat(root_surface_transformation_, - SkMatrix::MakeScale(device_pixel_ratio_)); - if (!base_xformation.isIdentity()) { + // account the root surface transformation. + if (!root_surface_transformation_.isIdentity()) { mutations_array.push_back( - mutations_referenced_.emplace_back(ConvertMutation(base_xformation)) + mutations_referenced_ + .emplace_back(ConvertMutation(root_surface_transformation_)) .get()); } diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 6c3ae1ce59a73..cd0b536c2acc4 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -3234,7 +3234,7 @@ TEST_F(EmbedderTest, PlatformViewMutatorsAreValidWithPixelRatio) { FlutterPlatformView platform_view = *layers[1]->platform_view; platform_view.struct_size = sizeof(platform_view); platform_view.identifier = 42; - platform_view.mutations_count = 4; + platform_view.mutations_count = 3; FlutterLayer layer = {}; layer.struct_size = sizeof(layer); @@ -3369,8 +3369,7 @@ TEST_F(EmbedderTest, case kFlutterPlatformViewMutationTypeTransformation: mutation.type = kFlutterPlatformViewMutationTypeTransformation; mutation.transformation = - FlutterTransformationMake(SkMatrix::Concat( - root_surface_transformation, SkMatrix::MakeScale(2.0))); + FlutterTransformationMake(root_surface_transformation); break; } @@ -3636,7 +3635,6 @@ TEST_F(EmbedderTest, ComplexClipsAreCorrectlyCalculated) { layer.size = FlutterSizeMake(600.0, 1024.0); layer.offset = FlutterPointMake(0.0, -256.0); - FML_LOG(ERROR) << (*layers[1]); ASSERT_EQ(*layers[1], layer); const auto** mutations = platform_view.mutations;