From c684df44f8fd451f72a7ae3d444ca0fd92f40a98 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Thu, 18 Jan 2024 12:29:18 -0800 Subject: [PATCH 1/2] [Windows] Refactor resize complete logic --- .../platform/windows/angle_surface_manager.cc | 2 +- .../platform/windows/angle_surface_manager.h | 2 +- shell/platform/windows/compositor_opengl.cc | 14 ++++++-- .../windows/compositor_opengl_unittests.cc | 17 ++-------- shell/platform/windows/flutter_window.cc | 4 --- shell/platform/windows/flutter_window.h | 3 -- .../windows/flutter_window_unittests.cc | 1 - .../platform/windows/flutter_windows_view.cc | 32 ++++++------------- shell/platform/windows/flutter_windows_view.h | 16 +++------- .../testing/mock_angle_surface_manager.h | 2 ++ .../testing/mock_window_binding_handler.h | 1 - .../platform/windows/window_binding_handler.h | 3 -- 12 files changed, 34 insertions(+), 63 deletions(-) diff --git a/shell/platform/windows/angle_surface_manager.cc b/shell/platform/windows/angle_surface_manager.cc index b8e998d3718a6..193e0d1f778f6 100644 --- a/shell/platform/windows/angle_surface_manager.cc +++ b/shell/platform/windows/angle_surface_manager.cc @@ -329,7 +329,7 @@ bool AngleSurfaceManager::MakeResourceCurrent() { egl_resource_context_) == EGL_TRUE); } -EGLBoolean AngleSurfaceManager::SwapBuffers() { +bool AngleSurfaceManager::SwapBuffers() { return (eglSwapBuffers(egl_display_, render_surface_)); } diff --git a/shell/platform/windows/angle_surface_manager.h b/shell/platform/windows/angle_surface_manager.h index 1d6ead21ed006..3f0afbb6c5ca6 100644 --- a/shell/platform/windows/angle_surface_manager.h +++ b/shell/platform/windows/angle_surface_manager.h @@ -77,7 +77,7 @@ class AngleSurfaceManager { // Swaps the front and back buffers of the DX11 swapchain backing surface if // not null. - EGLBoolean SwapBuffers(); + virtual bool SwapBuffers(); // Creates a |EGLSurface| from the provided handle. EGLSurface CreateSurfaceFromHandle(EGLenum handle_type, diff --git a/shell/platform/windows/compositor_opengl.cc b/shell/platform/windows/compositor_opengl.cc index 4dd4f7be6bd80..75070e75c01d4 100644 --- a/shell/platform/windows/compositor_opengl.cc +++ b/shell/platform/windows/compositor_opengl.cc @@ -154,7 +154,12 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers, GL_NEAREST // filter ); - return engine_->view()->SwapBuffers(); + if (!engine_->surface_manager()->SwapBuffers()) { + return false; + } + + engine_->view()->OnFramePresented(); + return true; } bool CompositorOpenGL::Initialize() { @@ -188,7 +193,12 @@ bool CompositorOpenGL::ClearSurface() { gl_->ClearColor(0.0f, 0.0f, 0.0f, 0.0f); gl_->Clear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT); - return engine_->view()->SwapBuffers(); + if (!engine_->surface_manager()->SwapBuffers()) { + return false; + } + + engine_->view()->OnFramePresented(); + return true; } } // namespace flutter diff --git a/shell/platform/windows/compositor_opengl_unittests.cc b/shell/platform/windows/compositor_opengl_unittests.cc index 74c26d8f39e17..0923f951d09cc 100644 --- a/shell/platform/windows/compositor_opengl_unittests.cc +++ b/shell/platform/windows/compositor_opengl_unittests.cc @@ -11,6 +11,7 @@ #include "flutter/shell/platform/windows/flutter_windows_view.h" #include "flutter/shell/platform/windows/testing/engine_modifier.h" #include "flutter/shell/platform/windows/testing/flutter_windows_engine_builder.h" +#include "flutter/shell/platform/windows/testing/mock_angle_surface_manager.h" #include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h" #include "flutter/shell/platform/windows/testing/windows_test.h" #include "gmock/gmock.h" @@ -56,24 +57,12 @@ const impeller::ProcTableGLES::Resolver kMockResolver = [](const char* name) { } }; -class MockAngleSurfaceManager : public AngleSurfaceManager { - public: - MockAngleSurfaceManager() : AngleSurfaceManager(false) {} - - MOCK_METHOD(bool, MakeCurrent, (), (override)); - - private: - FML_DISALLOW_COPY_AND_ASSIGN(MockAngleSurfaceManager); -}; - class MockFlutterWindowsView : public FlutterWindowsView { public: MockFlutterWindowsView(std::unique_ptr window) : FlutterWindowsView(std::move(window)) {} virtual ~MockFlutterWindowsView() = default; - MOCK_METHOD(bool, SwapBuffers, (), (override)); - private: FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsView); }; @@ -163,7 +152,7 @@ TEST_F(CompositorOpenGLTest, Present) { const FlutterLayer* layer_ptr = &layer; EXPECT_CALL(*surface_manager(), MakeCurrent).WillOnce(Return(true)); - EXPECT_CALL(*view(), SwapBuffers).WillOnce(Return(true)); + EXPECT_CALL(*surface_manager(), SwapBuffers).WillOnce(Return(true)); EXPECT_TRUE(compositor.Present(&layer_ptr, 1)); ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); @@ -179,7 +168,7 @@ TEST_F(CompositorOpenGLTest, PresentEmpty) { EXPECT_CALL(*surface_manager(), MakeCurrent) .Times(2) .WillRepeatedly(Return(true)); - EXPECT_CALL(*view(), SwapBuffers).WillOnce(Return(true)); + EXPECT_CALL(*surface_manager(), SwapBuffers).WillOnce(Return(true)); EXPECT_TRUE(compositor.Present(nullptr, 0)); } diff --git a/shell/platform/windows/flutter_window.cc b/shell/platform/windows/flutter_window.cc index b7630340ecfd5..63299b891cbfd 100644 --- a/shell/platform/windows/flutter_window.cc +++ b/shell/platform/windows/flutter_window.cc @@ -168,10 +168,6 @@ float FlutterWindow::GetDpiScale() { return static_cast(GetCurrentDPI()) / static_cast(base_dpi); } -bool FlutterWindow::IsVisible() { - return IsWindowVisible(GetWindowHandle()); -} - PhysicalWindowBounds FlutterWindow::GetPhysicalWindowBounds() { return {GetCurrentWidth(), GetCurrentHeight()}; } diff --git a/shell/platform/windows/flutter_window.h b/shell/platform/windows/flutter_window.h index e461476d39175..ef992cde41c63 100644 --- a/shell/platform/windows/flutter_window.h +++ b/shell/platform/windows/flutter_window.h @@ -158,9 +158,6 @@ class FlutterWindow : public KeyboardManager::WindowDelegate, // |FlutterWindowBindingHandler| virtual float GetDpiScale() override; - // |FlutterWindowBindingHandler| - virtual bool IsVisible() override; - // |FlutterWindowBindingHandler| virtual PhysicalWindowBounds GetPhysicalWindowBounds() override; diff --git a/shell/platform/windows/flutter_window_unittests.cc b/shell/platform/windows/flutter_window_unittests.cc index c920f5a97ef60..36972b33217f9 100644 --- a/shell/platform/windows/flutter_window_unittests.cc +++ b/shell/platform/windows/flutter_window_unittests.cc @@ -66,7 +66,6 @@ class MockFlutterWindow : public FlutterWindow { MOCK_METHOD(void, OnSetCursor, (), (override)); MOCK_METHOD(float, GetScrollOffsetMultiplier, (), (override)); MOCK_METHOD(float, GetDpiScale, (), (override)); - MOCK_METHOD(bool, IsVisible, (), (override)); MOCK_METHOD(void, UpdateCursorRect, (const Rect&), (override)); MOCK_METHOD(void, OnResetImeComposing, (), (override)); MOCK_METHOD(UINT, Win32DispatchMessage, (UINT, WPARAM, LPARAM), (override)); diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 13c10ed4491d6..6a4807fb5006f 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -590,26 +590,20 @@ void FlutterWindowsView::SendPointerEventWithData( } } -bool FlutterWindowsView::SwapBuffers() { - // Called on an engine-controlled (non-platform) thread. +void FlutterWindowsView::OnFramePresented() { + // Called on the engine's raster thread. std::unique_lock lock(resize_mutex_); switch (resize_status_) { - // SwapBuffer requests during resize are ignored until the frame with the - // right dimensions has been generated. This is marked with - // kFrameGenerated resize status. case ResizeState::kResizeStarted: - return false; + // The caller must first call |OnFrameGenerated| or + // |OnEmptyFrameGenerated| before calling this method. This status + // indicates the caller did not call these methods or ignored their + // result. + FML_UNREACHABLE(); case ResizeState::kFrameGenerated: { - bool visible = binding_handler_->IsVisible(); - bool swap_buffers_result; - // For visible windows swap the buffers while resize handler is waiting. - // For invisible windows unblock the handler first and then swap buffers. - // SwapBuffers waits for vsync and there's no point doing that for - // invisible windows. - if (visible) { - swap_buffers_result = engine_->surface_manager()->SwapBuffers(); - } + // A frame was generated for a pending resize. + // Unblock the platform thread. resize_status_ = ResizeState::kDone; lock.unlock(); resize_cv_.notify_all(); @@ -617,15 +611,9 @@ bool FlutterWindowsView::SwapBuffers() { // Blocking the raster thread until DWM flushes alleviates glitches where // previous size surface is stretched over current size view. windows_proc_table_->DwmFlush(); - - if (!visible) { - swap_buffers_result = engine_->surface_manager()->SwapBuffers(); - } - return swap_buffers_result; } case ResizeState::kDone: - default: - return engine_->surface_manager()->SwapBuffers(); + return; } } diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index cacc845bf614d..2dba87f25f993 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -62,17 +62,6 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { // Tells the engine to generate a new frame void ForceRedraw(); - // Swap the view's surface buffers. Must be called on the engine's raster - // thread. Returns true if the buffers were swapped. - // - // |OnFrameGenerated| or |OnEmptyFrameGenerated| must be called before this - // method. - // - // If the view is resizing, this returns false if the frame is not the target - // size. Otherwise, it unblocks the platform thread and blocks the raster - // thread until the v-blank. - virtual bool SwapBuffers(); - // Callback to clear a previously presented software bitmap. virtual bool ClearSoftwareBitmap(); @@ -103,6 +92,11 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { // |height| match the target size. bool OnFrameGenerated(size_t width, size_t height); + // Called on the raster thread after |CompositorOpenGL| presents a frame. + // + // This completes a view resize if one is pending. + virtual void OnFramePresented(); + // Sets the cursor that should be used when the mouse is over the Flutter // content. See mouse_cursor.dart for the values and meanings of cursor_name. void UpdateFlutterCursor(const std::string& cursor_name); diff --git a/shell/platform/windows/testing/mock_angle_surface_manager.h b/shell/platform/windows/testing/mock_angle_surface_manager.h index 6a8f365fe1d56..23b42d5a12b29 100644 --- a/shell/platform/windows/testing/mock_angle_surface_manager.h +++ b/shell/platform/windows/testing/mock_angle_surface_manager.h @@ -25,6 +25,8 @@ class MockAngleSurfaceManager : public AngleSurfaceManager { MOCK_METHOD(bool, ClearCurrent, (), (override)); MOCK_METHOD(void, SetVSyncEnabled, (bool), (override)); + MOCK_METHOD(bool, SwapBuffers, (), (override)); + private: FML_DISALLOW_COPY_AND_ASSIGN(MockAngleSurfaceManager); }; diff --git a/shell/platform/windows/testing/mock_window_binding_handler.h b/shell/platform/windows/testing/mock_window_binding_handler.h index 22185f057ac4c..b53cfcbc07cec 100644 --- a/shell/platform/windows/testing/mock_window_binding_handler.h +++ b/shell/platform/windows/testing/mock_window_binding_handler.h @@ -22,7 +22,6 @@ class MockWindowBindingHandler : public WindowBindingHandler { MOCK_METHOD(void, SetView, (WindowBindingHandlerDelegate * view), (override)); MOCK_METHOD(HWND, GetWindowHandle, (), (override)); MOCK_METHOD(float, GetDpiScale, (), (override)); - MOCK_METHOD(bool, IsVisible, (), (override)); MOCK_METHOD(PhysicalWindowBounds, GetPhysicalWindowBounds, (), (override)); MOCK_METHOD(void, UpdateFlutterCursor, diff --git a/shell/platform/windows/window_binding_handler.h b/shell/platform/windows/window_binding_handler.h index 29f4c6b8eba24..d0ef4565babb9 100644 --- a/shell/platform/windows/window_binding_handler.h +++ b/shell/platform/windows/window_binding_handler.h @@ -51,9 +51,6 @@ class WindowBindingHandler { // Returns the scale factor for the backing window. virtual float GetDpiScale() = 0; - // Returns whether the HWND is currently visible. - virtual bool IsVisible() = 0; - // Returns the bounds of the backing window in physical pixels. virtual PhysicalWindowBounds GetPhysicalWindowBounds() = 0; From 308daf02b959e3ff6fed19404a18db5a3248245d Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Thu, 18 Jan 2024 12:38:23 -0800 Subject: [PATCH 2/2] Clean up more! --- .../windows/compositor_opengl_unittests.cc | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/shell/platform/windows/compositor_opengl_unittests.cc b/shell/platform/windows/compositor_opengl_unittests.cc index 0923f951d09cc..caf602777519e 100644 --- a/shell/platform/windows/compositor_opengl_unittests.cc +++ b/shell/platform/windows/compositor_opengl_unittests.cc @@ -57,16 +57,6 @@ const impeller::ProcTableGLES::Resolver kMockResolver = [](const char* name) { } }; -class MockFlutterWindowsView : public FlutterWindowsView { - public: - MockFlutterWindowsView(std::unique_ptr window) - : FlutterWindowsView(std::move(window)) {} - virtual ~MockFlutterWindowsView() = default; - - private: - FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsView); -}; - class CompositorOpenGLTest : public WindowsTest { public: CompositorOpenGLTest() = default; @@ -74,7 +64,6 @@ class CompositorOpenGLTest : public WindowsTest { protected: FlutterWindowsEngine* engine() { return engine_.get(); } - MockFlutterWindowsView* view() { return view_.get(); } MockAngleSurfaceManager* surface_manager() { return surface_manager_; } void UseHeadlessEngine() { @@ -95,14 +84,14 @@ class CompositorOpenGLTest : public WindowsTest { EXPECT_CALL(*window.get(), SetView).Times(1); EXPECT_CALL(*window.get(), GetWindowHandle).WillRepeatedly(Return(nullptr)); - view_ = std::make_unique(std::move(window)); + view_ = std::make_unique(std::move(window)); engine_->SetView(view_.get()); } private: std::unique_ptr engine_; - std::unique_ptr view_; + std::unique_ptr view_; MockAngleSurfaceManager* surface_manager_; FML_DISALLOW_COPY_AND_ASSIGN(CompositorOpenGLTest);