From cbff03ae1ee6996b3da258d459c22e072e31a581 Mon Sep 17 00:00:00 2001 From: dnicoara Date: Wed, 15 Jan 2020 13:35:15 -0500 Subject: [PATCH] Fix embedder mutation order (flutter/engine#15566) 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. Bug: b/143612326 --- .../platform/embedder/embedder_layers.cc | 27 +++--- .../platform/embedder/fixtures/main.dart | 17 ++++ .../embedder/tests/embedder_unittests.cc | 87 ++++++++++++++++++- 3 files changed, 112 insertions(+), 19 deletions(-) diff --git a/engine/src/flutter/shell/platform/embedder/embedder_layers.cc b/engine/src/flutter/shell/platform/embedder/embedder_layers.cc index dc284e22746..d9e86ca1e04 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_layers.cc +++ b/engine/src/flutter/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,19 @@ 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 root surface transformation. + if (!root_surface_transformation_.isIdentity()) { + mutations_array.push_back( + mutations_referenced_ + .emplace_back(ConvertMutation(root_surface_transformation_)) + .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/engine/src/flutter/shell/platform/embedder/fixtures/main.dart b/engine/src/flutter/shell/platform/embedder/fixtures/main.dart index 72430ebd086..6368c25afc9 100644 --- a/engine/src/flutter/shell/platform/embedder/fixtures/main.dart +++ b/engine/src/flutter/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/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc index fe192ca00a7..cd0b536c2ac 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc +++ b/engine/src/flutter/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; } @@ -3606,6 +3605,88 @@ 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); + + 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);