Revert "[Windows] Introduce egl::Surface and egl::WindowSurface"

ci/licenses_golden/licenses_flutter

@@ -7242,10 +7242,6 @@ ORIGIN: ../../../flutter/shell/platform/windows/egl/manager.cc + ../../../flutte
 ORIGIN: ../../../flutter/shell/platform/windows/egl/manager.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/egl/proc_table.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/egl/proc_table.h + ../../../flutter/LICENSE
-ORIGIN: ../../../flutter/shell/platform/windows/egl/surface.cc + ../../../flutter/LICENSE
-ORIGIN: ../../../flutter/shell/platform/windows/egl/surface.h + ../../../flutter/LICENSE
-ORIGIN: ../../../flutter/shell/platform/windows/egl/window_surface.cc + ../../../flutter/LICENSE
-ORIGIN: ../../../flutter/shell/platform/windows/egl/window_surface.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/event_watcher.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/event_watcher.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/external_texture.h + ../../../flutter/LICENSE
@@ -10121,10 +10117,6 @@ FILE: ../../../flutter/shell/platform/windows/egl/manager.cc
 FILE: ../../../flutter/shell/platform/windows/egl/manager.h
 FILE: ../../../flutter/shell/platform/windows/egl/proc_table.cc
 FILE: ../../../flutter/shell/platform/windows/egl/proc_table.h
-FILE: ../../../flutter/shell/platform/windows/egl/surface.cc
-FILE: ../../../flutter/shell/platform/windows/egl/surface.h
-FILE: ../../../flutter/shell/platform/windows/egl/window_surface.cc
-FILE: ../../../flutter/shell/platform/windows/egl/window_surface.h
 FILE: ../../../flutter/shell/platform/windows/event_watcher.cc
 FILE: ../../../flutter/shell/platform/windows/event_watcher.h
 FILE: ../../../flutter/shell/platform/windows/external_texture.h

shell/platform/windows/BUILD.gn

@@ -59,10 +59,6 @@ source_set("flutter_windows_source") {
     "egl/manager.h",
     "egl/proc_table.cc",
     "egl/proc_table.h",
-    "egl/surface.cc",
-    "egl/surface.h",
-    "egl/window_surface.cc",
-    "egl/window_surface.h",
     "event_watcher.cc",
     "event_watcher.h",
     "external_texture.h",
@@ -213,7 +209,6 @@ executable("flutter_windows_unittests") {
     "testing/egl/mock_context.h",
     "testing/egl/mock_manager.h",
     "testing/egl/mock_proc_table.h",
-    "testing/egl/mock_window_surface.h",
     "testing/engine_modifier.h",
     "testing/flutter_window_test.cc",
     "testing/flutter_window_test.h",

shell/platform/windows/compositor_opengl.cc

@@ -97,11 +97,6 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
     return false;
   }
 
-  if (!engine_->egl_manager()->surface() ||
-      !engine_->egl_manager()->surface()->IsValid()) {
-    return false;
-  }
-
   // Clear the view if there are no layers to present.
   if (layers_count == 0) {
     // Normally the compositor is initialized when the first backing store is
@@ -133,7 +128,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
     return false;
   }
 
-  if (!engine_->egl_manager()->surface()->MakeCurrent()) {
+  if (!engine_->egl_manager()->MakeCurrent()) {
     return false;
   }
 
@@ -159,7 +154,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
                        GL_NEAREST            // filter
   );
 
-  if (!engine_->egl_manager()->surface()->SwapBuffers()) {
+  if (!engine_->egl_manager()->SwapBuffers()) {
     return false;
   }
 
@@ -170,7 +165,7 @@ bool CompositorOpenGL::Present(const FlutterLayer** layers,
 bool CompositorOpenGL::Initialize() {
   FML_DCHECK(!is_initialized_);
 
-  if (!engine_->egl_manager()->surface()->MakeCurrent()) {
+  if (!engine_->egl_manager()->MakeCurrent()) {
     return false;
   }
 
@@ -191,14 +186,14 @@ bool CompositorOpenGL::ClearSurface() {
   // Resize the surface if needed.
   engine_->view()->OnEmptyFrameGenerated();
 
-  if (!engine_->egl_manager()->surface()->MakeCurrent()) {
+  if (!engine_->egl_manager()->MakeCurrent()) {
     return false;
   }
 
   gl_->ClearColor(0.0f, 0.0f, 0.0f, 0.0f);
   gl_->Clear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
 
-  if (!engine_->egl_manager()->surface()->SwapBuffers()) {
+  if (!engine_->egl_manager()->SwapBuffers()) {
     return false;
   }
 

shell/platform/windows/compositor_opengl_unittests.cc

@@ -10,7 +10,6 @@
 #include "flutter/shell/platform/windows/egl/manager.h"
 #include "flutter/shell/platform/windows/flutter_windows_view.h"
 #include "flutter/shell/platform/windows/testing/egl/mock_manager.h"
-#include "flutter/shell/platform/windows/testing/egl/mock_window_surface.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_window_binding_handler.h"
@@ -66,14 +65,10 @@ class CompositorOpenGLTest : public WindowsTest {
  protected:
   FlutterWindowsEngine* engine() { return engine_.get(); }
   egl::MockManager* egl_manager() { return egl_manager_; }
-  egl::MockWindowSurface* surface() { return surface_.get(); }
 
   void UseHeadlessEngine() {
     auto egl_manager = std::make_unique<egl::MockManager>();
     egl_manager_ = egl_manager.get();
-    surface_ = std::make_unique<egl::MockWindowSurface>();
-
-    EXPECT_CALL(*egl_manager_, surface).WillRepeatedly(Return(surface_.get()));
 
     FlutterWindowsEngineBuilder builder{GetContext()};
 
@@ -97,7 +92,6 @@ class CompositorOpenGLTest : public WindowsTest {
  private:
   std::unique_ptr<FlutterWindowsEngine> engine_;
   std::unique_ptr<FlutterWindowsView> view_;
-  std::unique_ptr<egl::MockWindowSurface> surface_;
   egl::MockManager* egl_manager_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(CompositorOpenGLTest);
@@ -113,7 +107,7 @@ TEST_F(CompositorOpenGLTest, CreateBackingStore) {
   FlutterBackingStoreConfig config = {};
   FlutterBackingStore backing_store = {};
 
-  EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
+  EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
   ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
   ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
 }
@@ -126,7 +120,7 @@ TEST_F(CompositorOpenGLTest, InitializationFailure) {
   FlutterBackingStoreConfig config = {};
   FlutterBackingStore backing_store = {};
 
-  EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(false));
+  EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(false));
   EXPECT_FALSE(compositor.CreateBackingStore(config, &backing_store));
 }
 
@@ -138,16 +132,16 @@ TEST_F(CompositorOpenGLTest, Present) {
   FlutterBackingStoreConfig config = {};
   FlutterBackingStore backing_store = {};
 
-  EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
+  EXPECT_CALL(*egl_manager(), 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;
 
-  EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
-  EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true));
+  EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
+  EXPECT_CALL(*egl_manager(), SwapBuffers).WillOnce(Return(true));
   EXPECT_TRUE(compositor.Present(&layer_ptr, 1));
 
   ASSERT_TRUE(compositor.CollectBackingStore(&backing_store));
@@ -160,8 +154,10 @@ TEST_F(CompositorOpenGLTest, PresentEmpty) {
 
   // The context will be bound twice: first to initialize the compositor, second
   // to clear the surface.
-  EXPECT_CALL(*surface(), MakeCurrent).Times(2).WillRepeatedly(Return(true));
-  EXPECT_CALL(*surface(), SwapBuffers).WillOnce(Return(true));
+  EXPECT_CALL(*egl_manager(), MakeCurrent)
+      .Times(2)
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*egl_manager(), SwapBuffers).WillOnce(Return(true));
   EXPECT_TRUE(compositor.Present(nullptr, 0));
 }
 
@@ -173,7 +169,7 @@ TEST_F(CompositorOpenGLTest, HeadlessPresentIgnored) {
   FlutterBackingStoreConfig config = {};
   FlutterBackingStore backing_store = {};
 
-  EXPECT_CALL(*surface(), MakeCurrent).WillOnce(Return(true));
+  EXPECT_CALL(*egl_manager(), MakeCurrent).WillOnce(Return(true));
   ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store));
 
   FlutterLayer layer = {};

shell/platform/windows/egl/context.h

@@ -14,7 +14,7 @@ namespace egl {
 
 // An EGL context to interact with OpenGL.
 //
-// This enables automatic error logging and mocking.
+// This enables automatic eror logging and mocking.
 //
 // Flutter Windows uses this to create render and resource contexts.
 class Context {

shell/platform/windows/egl/manager.cc

@@ -263,78 +263,116 @@ bool Manager::IsValid() const {
   return is_valid_;
 }
 
-bool Manager::CreateWindowSurface(HWND hwnd, size_t width, size_t height) {
-  FML_DCHECK(surface_ == nullptr || !surface_->IsValid());
-
+bool Manager::CreateSurface(HWND hwnd, EGLint width, EGLint height) {
   if (!hwnd || !is_valid_) {
     return false;
   }
 
+  EGLSurface surface = EGL_NO_SURFACE;
+
   // Disable ANGLE's automatic surface resizing and provide an explicit size.
   // The surface will need to be destroyed and re-created if the HWND is
   // resized.
-  const EGLint surface_attributes[] = {EGL_FIXED_SIZE_ANGLE,
-                                       EGL_TRUE,
-                                       EGL_WIDTH,
-                                       static_cast<EGLint>(width),
-                                       EGL_HEIGHT,
-                                       static_cast<EGLint>(height),
-                                       EGL_NONE};
-
-  auto const surface = ::eglCreateWindowSurface(
-      display_, config_, static_cast<EGLNativeWindowType>(hwnd),
-      surface_attributes);
+  const EGLint surface_attributes[] = {
+      EGL_FIXED_SIZE_ANGLE, EGL_TRUE, EGL_WIDTH, width,
+      EGL_HEIGHT,           height,   EGL_NONE};
+
+  surface = ::eglCreateWindowSurface(display_, config_,
+                                     static_cast<EGLNativeWindowType>(hwnd),
+                                     surface_attributes);
   if (surface == EGL_NO_SURFACE) {
     LogEGLError("Surface creation failed.");
     return false;
   }
 
-  surface_ = std::make_unique<WindowSurface>(
-      display_, render_context_->GetHandle(), surface, width, height);
+  surface_width_ = width;
+  surface_height_ = height;
+  surface_ = surface;
   return true;
 }
 
-void Manager::ResizeWindowSurface(HWND hwnd, size_t width, size_t height) {
-  FML_CHECK(surface_ != nullptr);
-
-  auto const existing_width = surface_->width();
-  auto const existing_height = surface_->height();
-  auto const existing_vsync = surface_->vsync_enabled();
-
+void Manager::ResizeSurface(HWND hwnd,
+                            EGLint width,
+                            EGLint height,
+                            bool vsync_enabled) {
+  EGLint existing_width, existing_height;
+  GetSurfaceDimensions(&existing_width, &existing_height);
   if (width != existing_width || height != existing_height) {
+    surface_width_ = width;
+    surface_height_ = height;
+
     // TODO: Destroying the surface and re-creating it is expensive.
     // Ideally this would use ANGLE's automatic surface sizing instead.
     // See: https://github.com/flutter/flutter/issues/79427
-    if (!surface_->Destroy()) {
-      FML_LOG(ERROR) << "Manager::ResizeSurface failed to destroy surface";
-      return;
-    }
-
-    if (!CreateWindowSurface(hwnd, width, height)) {
+    render_context_->ClearCurrent();
+    DestroySurface();
+    if (!CreateSurface(hwnd, width, height)) {
       FML_LOG(ERROR) << "Manager::ResizeSurface failed to create surface";
-      return;
     }
+  }
 
-    if (!surface_->SetVSyncEnabled(existing_vsync)) {
-      // Surfaces block until the v-blank by default.
-      // Failing to update the vsync might result in unnecessary blocking.
-      // This regresses performance but not correctness.
-      FML_LOG(ERROR) << "Manager::ResizeSurface failed to set vsync";
-    }
+  SetVSyncEnabled(vsync_enabled);
+}
+
+void Manager::GetSurfaceDimensions(EGLint* width, EGLint* height) {
+  if (surface_ == EGL_NO_SURFACE || !is_valid_) {
+    *width = 0;
+    *height = 0;
+    return;
+  }
+
+  // This avoids eglQuerySurface as ideally surfaces would be automatically
+  // sized by ANGLE to avoid expensive surface destroy & re-create. With
+  // automatic sizing, ANGLE could resize the surface before Flutter asks it to,
+  // which would break resize redraw synchronization.
+  *width = surface_width_;
+  *height = surface_height_;
+}
+
+void Manager::DestroySurface() {
+  if (display_ != EGL_NO_DISPLAY && surface_ != EGL_NO_SURFACE) {
+    ::eglDestroySurface(display_, surface_);
   }
+  surface_ = EGL_NO_SURFACE;
 }
 
 bool Manager::HasContextCurrent() {
   return ::eglGetCurrentContext() != EGL_NO_CONTEXT;
 }
 
+bool Manager::MakeCurrent() {
+  return (::eglMakeCurrent(display_, surface_, surface_,
+                           render_context_->GetHandle()) == EGL_TRUE);
+}
+
+bool Manager::SwapBuffers() {
+  return (::eglSwapBuffers(display_, surface_));
+}
+
 EGLSurface Manager::CreateSurfaceFromHandle(EGLenum handle_type,
                                             EGLClientBuffer handle,
                                             const EGLint* attributes) const {
   return ::eglCreatePbufferFromClientBuffer(display_, handle_type, handle,
                                             config_, attributes);
 }
 
+void Manager::SetVSyncEnabled(bool enabled) {
+  if (!MakeCurrent()) {
+    LogEGLError("Unable to make surface current to update the swap interval");
+    return;
+  }
+
+  // OpenGL swap intervals can be used to prevent screen tearing.
+  // If enabled, the raster thread blocks until the v-blank.
+  // This is unnecessary if DWM composition is enabled.
+  // See: https://www.khronos.org/opengl/wiki/Swap_Interval
+  // See: https://learn.microsoft.com/windows/win32/dwm/composition-ovw
+  if (::eglSwapInterval(display_, enabled ? 1 : 0) != EGL_TRUE) {
+    LogEGLError("Unable to update the swap interval");
+    return;
+  }
+}
+
 bool Manager::GetDevice(ID3D11Device** device) {
   if (!resolved_device_) {
     if (!InitializeDevice()) {
@@ -354,9 +392,5 @@ Context* Manager::resource_context() const {
   return resource_context_.get();
 }
 
-WindowSurface* Manager::surface() const {
-  return surface_.get();
-}
-
 }  // namespace egl
 }  // namespace flutter