From 50cae024a3fbaf62f81dcab83a7c87d36d49de84 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 10 Jun 2020 17:22:20 -0700 Subject: [PATCH] Reland: Add RAII wrapper for EGLSurface (#18977) --- shell/platform/android/android_context_gl.cc | 99 ++++++++++---------- shell/platform/android/android_context_gl.h | 92 +++++++++--------- shell/platform/android/android_surface_gl.cc | 28 ++---- shell/platform/android/android_surface_gl.h | 4 +- 4 files changed, 108 insertions(+), 115 deletions(-) diff --git a/shell/platform/android/android_context_gl.cc b/shell/platform/android/android_context_gl.cc index c1ac748e077291..e72b5be5e7898f 100644 --- a/shell/platform/android/android_context_gl.cc +++ b/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 environment) @@ -161,25 +202,29 @@ AndroidContextGL::~AndroidContextGL() { } } -EGLSurface AndroidContextGL::CreateOnscreenSurface( +std::unique_ptr AndroidContextGL::CreateOnscreenSurface( fml::RefPtr window) const { EGLDisplay display = environment_->Display(); const EGLint attribs[] = {EGL_NONE}; - return eglCreateWindowSurface( + EGLSurface surface = eglCreateWindowSurface( display, config_, reinterpret_cast(window->handle()), attribs); + return std::make_unique(surface, display, context_); } -EGLSurface AndroidContextGL::CreateOffscreenSurface() const { +std::unique_ptr 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(surface, display, + resource_context_); } fml::RefPtr 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 diff --git a/shell/platform/android/android_context_gl.h b/shell/platform/android/android_context_gl.h index 606ffe829444bf..6b68d295a0835d 100644 --- a/shell/platform/android/android_context_gl.h +++ b/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 CreateOnscreenSurface( fml::RefPtr 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 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 environment_; EGLConfig config_; diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 59e5eacffac2b1..28011c99abae8f 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -20,22 +20,13 @@ AndroidSurfaceGL::AndroidSurfaceGL( std::static_pointer_cast(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(); } -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 AndroidSurfaceGL::GLContextMakeCurrent() { FML_DCHECK(IsValid()); FML_DCHECK(onscreen_surface_); auto default_context_result = std::make_unique( - 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 { diff --git a/shell/platform/android/android_surface_gl.h b/shell/platform/android/android_surface_gl.h index 58188630fb178d..1db19d115c4896 100644 --- a/shell/platform/android/android_surface_gl.h +++ b/shell/platform/android/android_surface_gl.h @@ -64,8 +64,8 @@ class AndroidSurfaceGL final : public GPUSurfaceGLDelegate, fml::RefPtr native_window_; std::unique_ptr external_view_embedder_; std::shared_ptr android_context_; - EGLSurface onscreen_surface_; - EGLSurface offscreen_surface_; + std::unique_ptr onscreen_surface_; + std::unique_ptr offscreen_surface_; FML_DISALLOW_COPY_AND_ASSIGN(AndroidSurfaceGL); };