From 4972ddf22dffca385af0f35ac3403d827ca6671a Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Thu, 7 Mar 2024 16:14:11 -0800 Subject: [PATCH] [Windows] Move to new present callback --- shell/platform/windows/compositor.h | 4 ++- shell/platform/windows/compositor_opengl.cc | 7 ++-- shell/platform/windows/compositor_opengl.h | 4 ++- .../windows/compositor_opengl_unittests.cc | 36 ++++++++++++++++--- shell/platform/windows/compositor_software.cc | 7 ++-- shell/platform/windows/compositor_software.h | 4 ++- .../windows/compositor_software_unittests.cc | 30 ++++++++++++++-- .../windows/flutter_windows_engine.cc | 10 +++--- 8 files changed, 79 insertions(+), 23 deletions(-) diff --git a/shell/platform/windows/compositor.h b/shell/platform/windows/compositor.h index c7691f53e8510..8ba7737844e3d 100644 --- a/shell/platform/windows/compositor.h +++ b/shell/platform/windows/compositor.h @@ -31,7 +31,9 @@ class Compositor { virtual bool CollectBackingStore(const FlutterBackingStore* store) = 0; // Present Flutter content and platform views onto the view. - virtual bool Present(const FlutterLayer** layers, size_t layers_count) = 0; + virtual bool Present(FlutterViewId view_id, + const FlutterLayer** layers, + size_t layers_count) = 0; }; } // namespace flutter diff --git a/shell/platform/windows/compositor_opengl.cc b/shell/platform/windows/compositor_opengl.cc index 58f5fab64e1f4..3cf5e9e802c85 100644 --- a/shell/platform/windows/compositor_opengl.cc +++ b/shell/platform/windows/compositor_opengl.cc @@ -92,11 +92,10 @@ bool CompositorOpenGL::CollectBackingStore(const FlutterBackingStore* store) { return true; } -bool CompositorOpenGL::Present(const FlutterLayer** layers, +bool CompositorOpenGL::Present(FlutterViewId view_id, + const FlutterLayer** layers, size_t layers_count) { - // TODO(loicsharma): Remove implicit view assumption. - // https://github.com/flutter/flutter/issues/142845 - FlutterWindowsView* view = engine_->view(kImplicitViewId); + FlutterWindowsView* view = engine_->view(view_id); if (!view) { return false; } diff --git a/shell/platform/windows/compositor_opengl.h b/shell/platform/windows/compositor_opengl.h index e1ed15293f921..1ea5c5045dfec 100644 --- a/shell/platform/windows/compositor_opengl.h +++ b/shell/platform/windows/compositor_opengl.h @@ -28,7 +28,9 @@ class CompositorOpenGL : public Compositor { bool CollectBackingStore(const FlutterBackingStore* store) override; /// |Compositor| - bool Present(const FlutterLayer** layers, size_t layers_count) override; + bool Present(FlutterViewId view_id, + const FlutterLayer** layers, + size_t layers_count) override; private: // The Flutter engine that manages the views to render. diff --git a/shell/platform/windows/compositor_opengl_unittests.cc b/shell/platform/windows/compositor_opengl_unittests.cc index 7e36fcadd9a2e..c8396984bfd85 100644 --- a/shell/platform/windows/compositor_opengl_unittests.cc +++ b/shell/platform/windows/compositor_opengl_unittests.cc @@ -68,6 +68,7 @@ class CompositorOpenGLTest : public WindowsTest { protected: FlutterWindowsEngine* engine() { return engine_.get(); } + FlutterWindowsView* view() { return view_.get(); } egl::MockManager* egl_manager() { return egl_manager_; } egl::MockContext* render_context() { return render_context_.get(); } egl::MockWindowSurface* surface() { return surface_; } @@ -168,7 +169,7 @@ TEST_F(CompositorOpenGLTest, Present) { EXPECT_CALL(*surface(), IsValid).WillRepeatedly(Return(true)); EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true)); EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true)); - EXPECT_TRUE(compositor.Present(&layer_ptr, 1)); + EXPECT_TRUE(compositor.Present(view()->view_id(), &layer_ptr, 1)); ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); } @@ -184,7 +185,7 @@ TEST_F(CompositorOpenGLTest, PresentEmpty) { EXPECT_CALL(*surface(), IsValid).WillRepeatedly(Return(true)); EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true)); EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true)); - EXPECT_TRUE(compositor.Present(nullptr, 0)); + EXPECT_TRUE(compositor.Present(view()->view_id(), nullptr, 0)); } TEST_F(CompositorOpenGLTest, HeadlessPresentIgnored) { @@ -203,7 +204,32 @@ TEST_F(CompositorOpenGLTest, HeadlessPresentIgnored) { layer.backing_store = &backing_store; const FlutterLayer* layer_ptr = &layer; - EXPECT_FALSE(compositor.Present(&layer_ptr, 1)); + EXPECT_FALSE(compositor.Present(kImplicitViewId, &layer_ptr, 1)); + + ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); +} + +TEST_F(CompositorOpenGLTest, UnknownViewIgnored) { + UseEngineWithView(); + + auto compositor = CompositorOpenGL{engine(), kMockResolver}; + + FlutterBackingStoreConfig config = {}; + FlutterBackingStore backing_store = {}; + + EXPECT_CALL(*render_context(), MakeCurrent).WillOnce(Return(true)); + ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store)); + + FlutterLayer layer = {}; + layer.type = kFlutterLayerContentTypeBackingStore; + layer.backing_store = &backing_store; + const FlutterLayer* layer_ptr = &layer; + + FlutterViewId unknown_view_id = 123; + ASSERT_NE(view()->view_id(), unknown_view_id); + ASSERT_EQ(engine()->view(unknown_view_id), nullptr); + + EXPECT_FALSE(compositor.Present(unknown_view_id, &layer_ptr, 1)); ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); } @@ -224,7 +250,9 @@ TEST_F(CompositorOpenGLTest, NoSurfaceIgnored) { layer.backing_store = &backing_store; const FlutterLayer* layer_ptr = &layer; - EXPECT_FALSE(compositor.Present(&layer_ptr, 1)); + EXPECT_FALSE(compositor.Present(view()->view_id(), &layer_ptr, 1)); + + ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); } } // namespace testing diff --git a/shell/platform/windows/compositor_software.cc b/shell/platform/windows/compositor_software.cc index 23166589abbaa..d171be71c5b04 100644 --- a/shell/platform/windows/compositor_software.cc +++ b/shell/platform/windows/compositor_software.cc @@ -38,11 +38,10 @@ bool CompositorSoftware::CollectBackingStore(const FlutterBackingStore* store) { return true; } -bool CompositorSoftware::Present(const FlutterLayer** layers, +bool CompositorSoftware::Present(FlutterViewId view_id, + const FlutterLayer** layers, size_t layers_count) { - // TODO(loicsharma): Remove implicit view assumption. - // https://github.com/flutter/flutter/issues/142845 - FlutterWindowsView* view = engine_->view(kImplicitViewId); + FlutterWindowsView* view = engine_->view(view_id); if (!view) { return false; } diff --git a/shell/platform/windows/compositor_software.h b/shell/platform/windows/compositor_software.h index c4e39111b3c1f..cdc5781673ccb 100644 --- a/shell/platform/windows/compositor_software.h +++ b/shell/platform/windows/compositor_software.h @@ -24,7 +24,9 @@ class CompositorSoftware : public Compositor { bool CollectBackingStore(const FlutterBackingStore* store) override; /// |Compositor| - bool Present(const FlutterLayer** layers, size_t layers_count) override; + bool Present(FlutterViewId view_id, + const FlutterLayer** layers, + size_t layers_count) override; private: FlutterWindowsEngine* engine_; diff --git a/shell/platform/windows/compositor_software_unittests.cc b/shell/platform/windows/compositor_software_unittests.cc index a639855537f37..f8f78ab1a621f 100644 --- a/shell/platform/windows/compositor_software_unittests.cc +++ b/shell/platform/windows/compositor_software_unittests.cc @@ -104,7 +104,7 @@ TEST_F(CompositorSoftwareTest, Present) { const FlutterLayer* layer_ptr = &layer; EXPECT_CALL(*view(), PresentSoftwareBitmap).WillOnce(Return(true)); - EXPECT_TRUE(compositor.Present(&layer_ptr, 1)); + EXPECT_TRUE(compositor.Present(view()->view_id(), &layer_ptr, 1)); ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); } @@ -115,7 +115,31 @@ TEST_F(CompositorSoftwareTest, PresentEmpty) { auto compositor = CompositorSoftware{engine()}; EXPECT_CALL(*view(), ClearSoftwareBitmap).WillOnce(Return(true)); - EXPECT_TRUE(compositor.Present(nullptr, 0)); + EXPECT_TRUE(compositor.Present(view()->view_id(), nullptr, 0)); +} + +TEST_F(CompositorSoftwareTest, UnknownViewIgnored) { + UseEngineWithView(); + + auto compositor = CompositorSoftware{engine()}; + + FlutterBackingStoreConfig config = {}; + FlutterBackingStore backing_store = {}; + + ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store)); + + FlutterLayer layer = {}; + layer.type = kFlutterLayerContentTypeBackingStore; + layer.backing_store = &backing_store; + const FlutterLayer* layer_ptr = &layer; + + FlutterViewId unknown_view_id = 123; + ASSERT_NE(view()->view_id(), unknown_view_id); + ASSERT_EQ(engine()->view(unknown_view_id), nullptr); + + EXPECT_FALSE(compositor.Present(unknown_view_id, &layer_ptr, 1)); + + ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); } TEST_F(CompositorSoftwareTest, HeadlessPresentIgnored) { @@ -133,7 +157,7 @@ TEST_F(CompositorSoftwareTest, HeadlessPresentIgnored) { layer.backing_store = &backing_store; const FlutterLayer* layer_ptr = &layer; - EXPECT_FALSE(compositor.Present(&layer_ptr, 1)); + EXPECT_FALSE(compositor.Present(kImplicitViewId, &layer_ptr, 1)); ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); } diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 8cafb0fcf497e..988e1f47954c6 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -414,12 +414,12 @@ bool FlutterWindowsEngine::Run(std::string_view entrypoint) { return host->compositor_->CollectBackingStore(backing_store); }; - compositor.present_layers_callback = [](const FlutterLayer** layers, - size_t layers_count, - void* user_data) -> bool { - auto host = static_cast(user_data); + compositor.present_view_callback = + [](const FlutterPresentViewInfo* info) -> bool { + auto host = static_cast(info->user_data); - return host->compositor_->Present(layers, layers_count); + return host->compositor_->Present(info->view_id, info->layers, + info->layers_count); }; args.compositor = &compositor;