From adb2b2e83ca798b36b0f090683fe511a163aac67 Mon Sep 17 00:00:00 2001 From: Emircan Uysaler Date: Tue, 15 Mar 2022 11:20:47 -0400 Subject: [PATCH] [fuchsia] Remove kFlatlandDefaultViewportSize (flutter/engine#31964) --- .../flatland_external_view_embedder.cc | 47 +++++--- .../flutter/flatland_external_view_embedder.h | 2 + ...atland_external_view_embedder_unittests.cc | 109 ++++++++++++++++++ 3 files changed, 143 insertions(+), 15 deletions(-) diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc index ef140343701..388a8feb3ab 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc @@ -188,7 +188,17 @@ void FlatlandExternalViewEmbedder::SubmitFrame( FML_CHECK(view_mutators.total_transform == view_params.transformMatrix()); - // TODO(fxbug.dev/94000): Handle clips. + if (viewport.pending_create_viewport_callback) { + if (view_size.fWidth && view_size.fHeight) { + viewport.pending_create_viewport_callback(view_size); + viewport.size = view_size; + } else { + FML_DLOG(WARNING) + << "Failed to create viewport because width or height is zero."; + } + } + + // TODO(fxbug.dev/64201): Handle clips. // Set transform for the viewport. // TODO(fxbug.dev/94000): Handle scaling. @@ -343,20 +353,24 @@ void FlatlandExternalViewEmbedder::CreateView( FlatlandViewCreatedCallback on_view_bound) { FML_CHECK(flatland_views_.find(view_id) == flatland_views_.end()); - FlatlandView new_view = {.transform_id = flatland_->NextTransformId(), - .viewport_id = flatland_->NextContentId()}; + const auto transform_id = flatland_->NextTransformId(); + const auto viewport_id = flatland_->NextContentId(); + FlatlandView new_view = {.transform_id = transform_id, + .viewport_id = viewport_id}; flatland_->flatland()->CreateTransform(new_view.transform_id); - fuchsia::ui::composition::ViewportProperties properties; - // TODO(fxbug.dev/94000): Investigate if it is possible to avoid using a - // default size by finding the size before creation. - properties.set_logical_size( - {kFlatlandDefaultViewportSize, kFlatlandDefaultViewportSize}); fuchsia::ui::composition::ChildViewWatcherPtr child_view_watcher; - flatland_->flatland()->CreateViewport( - new_view.viewport_id, {zx::channel((zx_handle_t)view_id)}, - std::move(properties), child_view_watcher.NewRequest()); - flatland_->flatland()->SetContent(new_view.transform_id, - new_view.viewport_id); + new_view.pending_create_viewport_callback = + [this, transform_id, viewport_id, view_id, + child_view_watcher_request = + child_view_watcher.NewRequest()](const SkSize& size) mutable { + fuchsia::ui::composition::ViewportProperties properties; + properties.set_logical_size({static_cast(size.fWidth), + static_cast(size.fHeight)}); + flatland_->flatland()->CreateViewport( + viewport_id, {zx::channel((zx_handle_t)view_id)}, + std::move(properties), std::move(child_view_watcher_request)); + flatland_->flatland()->SetContent(transform_id, viewport_id); + }; on_view_created(); on_view_bound(new_view.viewport_id, std::move(child_view_watcher)); @@ -371,7 +385,9 @@ void FlatlandExternalViewEmbedder::DestroyView( auto viewport_id = flatland_view->second.viewport_id; auto transform_id = flatland_view->second.transform_id; - flatland_->flatland()->ReleaseViewport(viewport_id, [](auto) {}); + if (!flatland_view->second.pending_create_viewport_callback) { + flatland_->flatland()->ReleaseViewport(viewport_id, [](auto) {}); + } auto itr = std::find_if(child_transforms_.begin(), child_transforms_.end(), [transform_id](fuchsia::ui::composition::TransformId id) { @@ -395,7 +411,8 @@ void FlatlandExternalViewEmbedder::SetViewProperties( auto found = flatland_views_.find(view_id); FML_CHECK(found != flatland_views_.end()); - // TODO(fxbug.dev/94000): Set occlusion_hint, hit_testable and focusable. + // TODO(fxbug.dev/94000): Set occlusion_hint, hit_testable and focusable. Note + // that pending_create_viewport_callback might not have run at this point. } void FlatlandExternalViewEmbedder::Reset() { diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h index 960191f5d26..edcae120aec 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h @@ -7,6 +7,7 @@ #include #include +#include #include #include // For uint32_t & uint64_t @@ -161,6 +162,7 @@ class FlatlandExternalViewEmbedder final fuchsia::ui::composition::ContentId viewport_id; ViewMutators mutators; SkSize size = SkSize::MakeEmpty(); + fit::callback pending_create_viewport_callback; }; struct FlatlandLayer { diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_external_view_embedder_unittests.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_external_view_embedder_unittests.cc index 4531107c76e..288ba0eb03c 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_external_view_embedder_unittests.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_external_view_embedder_unittests.cc @@ -721,4 +721,113 @@ TEST_F(FlatlandExternalViewEmbedderTest, SceneWithOneView_NoOverlay) { {IsImageLayer(frame_size, kFirstLayerBlendMode, 1)})); } +TEST_F(FlatlandExternalViewEmbedderTest, + SceneWithOneView_DestroyBeforeDrawing) { + fuchsia::ui::composition::ParentViewportWatcherPtr parent_viewport_watcher; + fuchsia::ui::views::ViewportCreationToken viewport_creation_token; + fuchsia::ui::views::ViewCreationToken view_creation_token; + fuchsia::ui::views::ViewRef view_ref; + auto view_creation_token_status = zx::channel::create( + 0u, &viewport_creation_token.value, &view_creation_token.value); + ASSERT_EQ(view_creation_token_status, ZX_OK); + auto view_ref_pair = scenic::ViewRefPair::New(); + view_ref_pair.view_ref.Clone(&view_ref); + + // Create the `FlatlandExternalViewEmbedder` and pump the message loop until + // the initial scene graph is setup. + FlatlandExternalViewEmbedder external_view_embedder( + std::move(view_creation_token), + fuchsia::ui::views::ViewIdentityOnCreation{ + .view_ref = std::move(view_ref_pair.view_ref), + .view_ref_control = std::move(view_ref_pair.control_ref), + }, + fuchsia::ui::composition::ViewBoundProtocols{}, + parent_viewport_watcher.NewRequest(), flatland_connection(), + fake_surface_producer()); + flatland_connection()->Present(); + loop().RunUntilIdle(); + fake_flatland().FireOnNextFrameBeginEvent(WithPresentCredits(1u)); + loop().RunUntilIdle(); + EXPECT_THAT(fake_flatland().graph(), + IsFlutterGraph(parent_viewport_watcher, viewport_creation_token, + view_ref)); + + // Create the view before drawing the scene. + auto [child_view_token, child_viewport_token] = ViewTokenPair::New(); + const uint32_t child_view_id = child_viewport_token.value.get(); + external_view_embedder.CreateView( + child_view_id, []() {}, + [](fuchsia::ui::composition::ContentId, + fuchsia::ui::composition::ChildViewWatcherPtr) {}); + + // Draw the scene without the view. The scene graph shouldn't change yet. + const SkISize frame_size_signed = SkISize::Make(512, 512); + const fuchsia::math::SizeU frame_size{ + static_cast(frame_size_signed.width()), + static_cast(frame_size_signed.height())}; + DrawSimpleFrame( + external_view_embedder, frame_size_signed, 1.f, [](SkCanvas* canvas) { + const SkSize canvas_size = SkSize::Make(canvas->imageInfo().width(), + canvas->imageInfo().height()); + SkPaint rect_paint; + rect_paint.setColor(SK_ColorGREEN); + canvas->translate(canvas_size.width() / 4.f, + canvas_size.height() / 2.f); + canvas->drawRect(SkRect::MakeWH(canvas_size.width() / 32.f, + canvas_size.height() / 32.f), + rect_paint); + }); + + // Pump the message loop. The scene updates should propagate to flatland. + loop().RunUntilIdle(); + fake_flatland().FireOnNextFrameBeginEvent(WithPresentCredits(1u)); + loop().RunUntilIdle(); + EXPECT_THAT( + fake_flatland().graph(), + IsFlutterGraph(parent_viewport_watcher, viewport_creation_token, view_ref, + /*layers*/ + {IsImageLayer(frame_size, kFirstLayerBlendMode, 1)})); + + // Destroy the view. The scene graph shouldn't change yet. + external_view_embedder.DestroyView( + child_view_id, [](fuchsia::ui::composition::ContentId) {}); + EXPECT_THAT( + fake_flatland().graph(), + IsFlutterGraph(parent_viewport_watcher, viewport_creation_token, view_ref, + /*layers*/ + {IsImageLayer(frame_size, kFirstLayerBlendMode, 1)})); + + // Draw another frame without the view and change the size. The scene graph + // shouldn't change yet. + const SkISize new_frame_size_signed = SkISize::Make(256, 256); + const fuchsia::math::SizeU new_frame_size{ + static_cast(new_frame_size_signed.width()), + static_cast(new_frame_size_signed.height())}; + DrawSimpleFrame( + external_view_embedder, new_frame_size_signed, 1.f, [](SkCanvas* canvas) { + const SkSize canvas_size = SkSize::Make(canvas->imageInfo().width(), + canvas->imageInfo().height()); + SkPaint rect_paint; + rect_paint.setColor(SK_ColorGREEN); + canvas->translate(canvas_size.width() / 4.f, + canvas_size.height() / 2.f); + canvas->drawRect(SkRect::MakeWH(canvas_size.width() / 32.f, + canvas_size.height() / 32.f), + rect_paint); + }); + EXPECT_THAT( + fake_flatland().graph(), + IsFlutterGraph(parent_viewport_watcher, viewport_creation_token, view_ref, + /*layers*/ + {IsImageLayer(frame_size, kFirstLayerBlendMode, 1)})); + + // Pump the message loop. The scene updates should propagate to flatland. + loop().RunUntilIdle(); + EXPECT_THAT( + fake_flatland().graph(), + IsFlutterGraph(parent_viewport_watcher, viewport_creation_token, + view_ref, /*layers*/ + {IsImageLayer(new_frame_size, kFirstLayerBlendMode, 1)})); +} + } // namespace flutter_runner::testing