Revert "Add RAII wrapper for EGLSurface (flutter#18849)" (flutter#18971)

This reverts commit d8fe71f.

Author: Emmanuel Garcia

shell/platform/android/android_context_gl.cc

@@ -105,14 +105,6 @@ static bool TeardownContext(EGLDisplay display, EGLContext context) {
   return true;
 }
 
-AndroidEGLSurface::AndroidEGLSurface(EGLSurface surface, EGLDisplay display)
-    : surface(surface), display_(display) {}
-
-AndroidEGLSurface::~AndroidEGLSurface() {
-  auto result = eglDestroySurface(display_, surface);
-  FML_DCHECK(result == EGL_TRUE);
-}
-
 AndroidContextGL::AndroidContextGL(
     AndroidRenderingAPI rendering_api,
     fml::RefPtr<AndroidEnvironmentGL> environment)
@@ -169,28 +161,25 @@ AndroidContextGL::~AndroidContextGL() {
   }
 }
 
-std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOnscreenSurface(
+EGLSurface AndroidContextGL::CreateOnscreenSurface(
     fml::RefPtr<AndroidNativeWindow> window) const {
   EGLDisplay display = environment_->Display();
 
   const EGLint attribs[] = {EGL_NONE};
 
-  EGLSurface surface = eglCreateWindowSurface(
+  return eglCreateWindowSurface(
       display, config_, reinterpret_cast<EGLNativeWindowType>(window->handle()),
       attribs);
-  return std::make_unique<AndroidEGLSurface>(surface, display);
 }
 
-std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOffscreenSurface()
-    const {
+EGLSurface AndroidContextGL::CreateOffscreenSurface() const {
   // We only ever create pbuffer surfaces for background resource loading
   // contexts. We never bind the pbuffer to anything.
   EGLDisplay display = environment_->Display();
 
   const EGLint attribs[] = {EGL_WIDTH, 1, EGL_HEIGHT, 1, EGL_NONE};
 
-  EGLSurface surface = eglCreatePbufferSurface(display, config_, attribs);
-  return std::make_unique<AndroidEGLSurface>(surface, display);
+  return eglCreatePbufferSurface(display, config_, attribs);
 }
 
 fml::RefPtr<AndroidEnvironmentGL> AndroidContextGL::Environment() const {
@@ -201,21 +190,19 @@ bool AndroidContextGL::IsValid() const {
   return valid_;
 }
 
-bool AndroidContextGL::MakeCurrent(
-    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
-  if (eglMakeCurrent(environment_->Display(), surface_wrapper->surface,
-                     surface_wrapper->surface, context_) != EGL_TRUE) {
+bool AndroidContextGL::MakeCurrent(EGLSurface& surface) {
+  if (eglMakeCurrent(environment_->Display(), surface, surface, context_) !=
+      EGL_TRUE) {
     FML_LOG(ERROR) << "Could not make the context current";
     LogLastEGLError();
     return false;
   }
   return true;
 }
 
-bool AndroidContextGL::ResourceMakeCurrent(
-    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
-  if (eglMakeCurrent(environment_->Display(), surface_wrapper->surface,
-                     surface_wrapper->surface, resource_context_) != EGL_TRUE) {
+bool AndroidContextGL::ResourceMakeCurrent(EGLSurface& surface) {
+  if (eglMakeCurrent(environment_->Display(), surface, surface,
+                     resource_context_) != EGL_TRUE) {
     FML_LOG(ERROR) << "Could not make the context current";
     LogLastEGLError();
     return false;
@@ -236,26 +223,30 @@ bool AndroidContextGL::ClearCurrent() {
   return true;
 }
 
-bool AndroidContextGL::SwapBuffers(
-    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
+bool AndroidContextGL::SwapBuffers(EGLSurface& surface) {
   TRACE_EVENT0("flutter", "AndroidContextGL::SwapBuffers");
-  return eglSwapBuffers(environment_->Display(), surface_wrapper->surface);
+  return eglSwapBuffers(environment_->Display(), surface);
 }
 
-SkISize AndroidContextGL::GetSize(
-    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
+SkISize AndroidContextGL::GetSize(EGLSurface& surface) {
   EGLint width = 0;
   EGLint height = 0;
 
-  if (!eglQuerySurface(environment_->Display(), surface_wrapper->surface,
-                       EGL_WIDTH, &width) ||
-      !eglQuerySurface(environment_->Display(), surface_wrapper->surface,
-                       EGL_HEIGHT, &height)) {
+  if (!eglQuerySurface(environment_->Display(), surface, EGL_WIDTH, &width) ||
+      !eglQuerySurface(environment_->Display(), surface, EGL_HEIGHT, &height)) {
     FML_LOG(ERROR) << "Unable to query EGL surface size";
     LogLastEGLError();
     return SkISize::Make(0, 0);
   }
   return SkISize::Make(width, height);
 }
 
+bool AndroidContextGL::TeardownSurface(EGLSurface& surface) {
+  if (surface != EGL_NO_SURFACE) {
+    return eglDestroySurface(environment_->Display(), surface) == EGL_TRUE;
+  }
+
+  return true;
+}
+
 }  // namespace flutter

shell/platform/android/android_context_gl.h

@@ -16,23 +16,6 @@
 
 namespace flutter {
 
-//------------------------------------------------------------------------------
-/// Holds an `EGLSurface` reference.
-///
-///
-/// This can be used in conjuction to unique_ptr to provide better guarantees
-/// about the lifespam of the `EGLSurface` object.
-///
-struct AndroidEGLSurface {
-  AndroidEGLSurface(EGLSurface surface, EGLDisplay display);
-  ~AndroidEGLSurface();
-
-  const EGLSurface surface;
-
- private:
-  const EGLDisplay display_;
-};
-
 //------------------------------------------------------------------------------
 /// The Android context is used by `AndroidSurfaceGL` to create and manage
 /// EGL surfaces.
@@ -51,18 +34,24 @@ class AndroidContextGL : public AndroidContext {
   /// @brief      Allocates an new EGL window surface that is used for on-screen
   ///             pixels.
   ///
+  /// @attention  Consumers must tear down the surface by calling
+  ///             `AndroidContextGL::TeardownSurface`.
+  ///
   /// @return     The window surface.
   ///
-  std::unique_ptr<AndroidEGLSurface> CreateOnscreenSurface(
+  EGLSurface CreateOnscreenSurface(
       fml::RefPtr<AndroidNativeWindow> window) const;
 
   //----------------------------------------------------------------------------
   /// @brief      Allocates an 1x1 pbuffer surface that is used for making the
   ///             offscreen current for texture uploads.
   ///
+  /// @attention  Consumers must tear down the surface by calling
+  ///             `AndroidContextGL::TeardownSurface`.
+  ///
   /// @return     The pbuffer surface.
   ///
-  std::unique_ptr<AndroidEGLSurface> CreateOffscreenSurface() const;
+  EGLSurface CreateOffscreenSurface() const;
 
   //----------------------------------------------------------------------------
   /// @return     The Android environment that contains a reference to the
@@ -88,28 +77,35 @@ class AndroidContextGL : public AndroidContext {
   ///
   /// @return     Whether the surface was made current.
   ///
-  bool MakeCurrent(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
+  bool MakeCurrent(EGLSurface& surface);
 
   //----------------------------------------------------------------------------
   /// @brief      Binds the resource EGLContext context to the current rendering
   ///             thread and to the draw and read surface.
   ///
   /// @return     Whether the surface was made current.
   ///
-  bool ResourceMakeCurrent(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
+  bool ResourceMakeCurrent(EGLSurface& surface);
 
   //----------------------------------------------------------------------------
   /// @brief      This only applies to on-screen surfaces such as those created
   ///             by `AndroidContextGL::CreateOnscreenSurface`.
   ///
   /// @return     Whether the EGL surface color buffer was swapped.
   ///
-  bool SwapBuffers(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
+  bool SwapBuffers(EGLSurface& surface);
 
   //----------------------------------------------------------------------------
   /// @return     The size of an `EGLSurface`.
   ///
-  SkISize GetSize(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
+  SkISize GetSize(EGLSurface& surface);
+
+  //----------------------------------------------------------------------------
+  /// @brief      Destroys an `EGLSurface`.
+  ///
+  /// @return     Whether the surface was destroyed.
+  ///
+  bool TeardownSurface(EGLSurface& surface);
 
  private:
   fml::RefPtr<AndroidEnvironmentGL> environment_;

shell/platform/android/android_surface_gl.cc

@@ -20,13 +20,22 @@ AndroidSurfaceGL::AndroidSurfaceGL(
       std::static_pointer_cast<AndroidContextGL>(android_context);
   // Acquire the offscreen surface.
   offscreen_surface_ = android_context_->CreateOffscreenSurface();
-  if (offscreen_surface_->surface == EGL_NO_SURFACE) {
+  if (offscreen_surface_ == EGL_NO_SURFACE) {
     offscreen_surface_ = nullptr;
   }
   external_view_embedder_ = std::make_unique<AndroidExternalViewEmbedder>();
 }
 
-AndroidSurfaceGL::~AndroidSurfaceGL() = default;
+AndroidSurfaceGL::~AndroidSurfaceGL() {
+  if (offscreen_surface_) {
+    android_context_->TeardownSurface(offscreen_surface_);
+    offscreen_surface_ = nullptr;
+  }
+  if (onscreen_surface_) {
+    android_context_->TeardownSurface(onscreen_surface_);
+    onscreen_surface_ = nullptr;
+  }
+}
 
 void AndroidSurfaceGL::TeardownOnScreenContext() {
   android_context_->ClearCurrent();
@@ -49,24 +58,25 @@ bool AndroidSurfaceGL::OnScreenSurfaceResize(const SkISize& size) {
   FML_DCHECK(onscreen_surface_);
   FML_DCHECK(native_window_);
 
-  if (size == android_context_->GetSize(std::move(onscreen_surface_))) {
+  if (size == android_context_->GetSize(onscreen_surface_)) {
     return true;
   }
 
   android_context_->ClearCurrent();
+  android_context_->TeardownSurface(onscreen_surface_);
 
   onscreen_surface_ = android_context_->CreateOnscreenSurface(native_window_);
-  if (onscreen_surface_->surface == EGL_NO_SURFACE) {
+  if (!onscreen_surface_ || onscreen_surface_ == EGL_NO_SURFACE) {
     FML_LOG(ERROR) << "Unable to create EGL window surface on resize.";
     return false;
   }
-  android_context_->MakeCurrent(std::move(onscreen_surface_));
+  android_context_->MakeCurrent(onscreen_surface_);
   return true;
 }
 
 bool AndroidSurfaceGL::ResourceContextMakeCurrent() {
   FML_DCHECK(IsValid());
-  return android_context_->ResourceMakeCurrent(std::move(offscreen_surface_));
+  return android_context_->ResourceMakeCurrent(offscreen_surface_);
 }
 
 bool AndroidSurfaceGL::ResourceContextClearCurrent() {
@@ -81,7 +91,7 @@ bool AndroidSurfaceGL::SetNativeWindow(
   native_window_ = window;
   // Create the onscreen surface.
   onscreen_surface_ = android_context_->CreateOnscreenSurface(window);
-  if (onscreen_surface_->surface == EGL_NO_SURFACE) {
+  if (onscreen_surface_ == EGL_NO_SURFACE) {
     return false;
   }
   return true;
@@ -91,7 +101,7 @@ std::unique_ptr<GLContextResult> AndroidSurfaceGL::GLContextMakeCurrent() {
   FML_DCHECK(IsValid());
   FML_DCHECK(onscreen_surface_);
   auto default_context_result = std::make_unique<GLContextDefaultResult>(
-      android_context_->MakeCurrent(std::move(onscreen_surface_)));
+      android_context_->MakeCurrent(onscreen_surface_));
   return std::move(default_context_result);
 }
 
@@ -103,7 +113,7 @@ bool AndroidSurfaceGL::GLContextClearCurrent() {
 bool AndroidSurfaceGL::GLContextPresent() {
   FML_DCHECK(IsValid());
   FML_DCHECK(onscreen_surface_);
-  return android_context_->SwapBuffers(std::move(onscreen_surface_));
+  return android_context_->SwapBuffers(onscreen_surface_);
 }
 
 intptr_t AndroidSurfaceGL::GLContextFBO() const {

shell/platform/android/android_surface_gl.h

@@ -64,8 +64,8 @@ class AndroidSurfaceGL final : public GPUSurfaceGLDelegate,
   fml::RefPtr<AndroidNativeWindow> native_window_;
   std::unique_ptr<AndroidExternalViewEmbedder> external_view_embedder_;
   std::shared_ptr<AndroidContextGL> android_context_;
-  std::unique_ptr<AndroidEGLSurface> onscreen_surface_;
-  std::unique_ptr<AndroidEGLSurface> offscreen_surface_;
+  EGLSurface onscreen_surface_;
+  EGLSurface offscreen_surface_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(AndroidSurfaceGL);
 };