Reland: Add RAII wrapper for EGLSurface (flutter#18977)

Author: Emmanuel Garcia

shell/platform/android/android_context_gl.cc

@@ -105,6 +105,47 @@ static bool TeardownContext(EGLDisplay display, EGLContext context) {
   return true;
 }
 
+AndroidEGLSurface::AndroidEGLSurface(EGLSurface surface,
+                                     EGLDisplay display,
+                                     EGLContext context)
+    : surface_(surface), display_(display), context_(context) {}
+
+AndroidEGLSurface::~AndroidEGLSurface() {
+  auto result = eglDestroySurface(display_, surface_);
+  FML_DCHECK(result == EGL_TRUE);
+}
+
+bool AndroidEGLSurface::IsValid() const {
+  return surface_ != EGL_NO_SURFACE;
+}
+
+bool AndroidEGLSurface::MakeCurrent() {
+  if (eglMakeCurrent(display_, surface_, surface_, context_) != EGL_TRUE) {
+    FML_LOG(ERROR) << "Could not make the context current";
+    LogLastEGLError();
+    return false;
+  }
+  return true;
+}
+
+bool AndroidEGLSurface::SwapBuffers() {
+  TRACE_EVENT0("flutter", "AndroidContextGL::SwapBuffers");
+  return eglSwapBuffers(display_, surface_);
+}
+
+SkISize AndroidEGLSurface::GetSize() const {
+  EGLint width = 0;
+  EGLint height = 0;
+
+  if (!eglQuerySurface(display_, surface_, EGL_WIDTH, &width) ||
+      !eglQuerySurface(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);
+}
+
 AndroidContextGL::AndroidContextGL(
     AndroidRenderingAPI rendering_api,
     fml::RefPtr<AndroidEnvironmentGL> environment)
@@ -161,25 +202,29 @@ AndroidContextGL::~AndroidContextGL() {
   }
 }
 
-EGLSurface AndroidContextGL::CreateOnscreenSurface(
+std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOnscreenSurface(
     fml::RefPtr<AndroidNativeWindow> window) const {
   EGLDisplay display = environment_->Display();
 
   const EGLint attribs[] = {EGL_NONE};
 
-  return eglCreateWindowSurface(
+  EGLSurface surface = eglCreateWindowSurface(
       display, config_, reinterpret_cast<EGLNativeWindowType>(window->handle()),
       attribs);
+  return std::make_unique<AndroidEGLSurface>(surface, display, context_);
 }
 
-EGLSurface AndroidContextGL::CreateOffscreenSurface() const {
+std::unique_ptr<AndroidEGLSurface> 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};
 
-  return eglCreatePbufferSurface(display, config_, attribs);
+  EGLSurface surface = eglCreatePbufferSurface(display, config_, attribs);
+  return std::make_unique<AndroidEGLSurface>(surface, display,
+                                             resource_context_);
 }
 
 fml::RefPtr<AndroidEnvironmentGL> AndroidContextGL::Environment() const {
@@ -190,26 +235,6 @@ bool AndroidContextGL::IsValid() const {
   return valid_;
 }
 
-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(EGLSurface& surface) {
-  if (eglMakeCurrent(environment_->Display(), surface, surface,
-                     resource_context_) != EGL_TRUE) {
-    FML_LOG(ERROR) << "Could not make the context current";
-    LogLastEGLError();
-    return false;
-  }
-  return true;
-}
-
 bool AndroidContextGL::ClearCurrent() {
   if (eglGetCurrentContext() != context_) {
     return true;
@@ -223,30 +248,4 @@ bool AndroidContextGL::ClearCurrent() {
   return true;
 }
 
-bool AndroidContextGL::SwapBuffers(EGLSurface& surface) {
-  TRACE_EVENT0("flutter", "AndroidContextGL::SwapBuffers");
-  return eglSwapBuffers(environment_->Display(), surface);
-}
-
-SkISize AndroidContextGL::GetSize(EGLSurface& surface) {
-  EGLint width = 0;
-  EGLint height = 0;
-
-  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,6 +16,52 @@
 
 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.
+///
+class AndroidEGLSurface {
+ public:
+  AndroidEGLSurface(EGLSurface surface, EGLDisplay display, EGLContext context);
+  ~AndroidEGLSurface();
+
+  //----------------------------------------------------------------------------
+  /// @return     Whether the current `EGLSurface` reference is valid. That is,
+  /// if
+  ///             the surface doesn't point to `EGL_NO_SURFACE`.
+  ///
+  bool IsValid() const;
+
+  //----------------------------------------------------------------------------
+  /// @brief      Binds the EGLContext context to the current rendering thread
+  ///             and to the draw and read surface.
+  ///
+  /// @return     Whether the surface was made current.
+  ///
+  bool MakeCurrent();
+
+  //----------------------------------------------------------------------------
+  /// @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();
+
+  //----------------------------------------------------------------------------
+  /// @return     The size of an `EGLSurface`.
+  ///
+  SkISize GetSize() const;
+
+ private:
+  const EGLSurface surface_;
+  const EGLDisplay display_;
+  const EGLContext context_;
+};
+
 //------------------------------------------------------------------------------
 /// The Android context is used by `AndroidSurfaceGL` to create and manage
 /// EGL surfaces.
@@ -34,24 +80,18 @@ 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.
   ///
-  EGLSurface CreateOnscreenSurface(
+  std::unique_ptr<AndroidEGLSurface> 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.
   ///
-  EGLSurface CreateOffscreenSurface() const;
+  std::unique_ptr<AndroidEGLSurface> CreateOffscreenSurface() const;
 
   //----------------------------------------------------------------------------
   /// @return     The Android environment that contains a reference to the
@@ -71,42 +111,6 @@ class AndroidContextGL : public AndroidContext {
   ///
   bool ClearCurrent();
 
-  //----------------------------------------------------------------------------
-  /// @brief      Binds the EGLContext context to the current rendering thread
-  ///             and to the draw and read surface.
-  ///
-  /// @return     Whether the surface was made current.
-  ///
-  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(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(EGLSurface& surface);
-
-  //----------------------------------------------------------------------------
-  /// @return     The size of an `EGLSurface`.
-  ///
-  SkISize GetSize(EGLSurface& surface);
-
-  //----------------------------------------------------------------------------
-  /// @brief      Destroys an `EGLSurface`.
-  ///
-  /// @return     Whether the surface was destroyed.
-  ///
-  bool TeardownSurface(EGLSurface& surface);
-
  private:
   fml::RefPtr<AndroidEnvironmentGL> environment_;
   EGLConfig config_;

shell/platform/android/android_surface_gl.cc

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