From 6571616d44c133bf757387ae439117d6e6d60e0d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 21 Nov 2024 21:07:49 -0800 Subject: [PATCH 1/3] cache and reuse FBOs --- .../renderer/backend/gles/render_pass_gles.cc | 57 ++++++++++--------- .../renderer/backend/gles/texture_gles.cc | 3 + impeller/renderer/backend/gles/texture_gles.h | 5 ++ 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 32228d3d87830..de308954075c0 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -207,13 +207,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { } GLuint fbo = GL_NONE; - fml::ScopedCleanupClosure delete_fbo([&gl, &fbo]() { - if (fbo != GL_NONE) { - gl.BindFramebuffer(GL_FRAMEBUFFER, GL_NONE); - gl.DeleteFramebuffers(1u, &fbo); - } - }); - TextureGLES& color_gles = TextureGLES::Cast(*pass_data.color_attachment); const bool is_default_fbo = color_gles.IsWrapped(); @@ -224,32 +217,40 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) { } } else { // Create and bind an offscreen FBO. - gl.GenFramebuffers(1u, &fbo); - gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); - - if (!color_gles.SetAsFramebufferAttachment( - GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) { - return false; - } + auto cached_fbo = color_gles.GetCachedFBO(); + if (cached_fbo != GL_NONE) { + fbo = cached_fbo; + gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); + } else { + gl.GenFramebuffers(1u, &fbo); + color_gles.SetCachedFBO(fbo); + gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); - if (auto depth = TextureGLES::Cast(pass_data.depth_attachment.get())) { - if (!depth->SetAsFramebufferAttachment( - GL_FRAMEBUFFER, TextureGLES::AttachmentType::kDepth)) { + if (!color_gles.SetAsFramebufferAttachment( + GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) { return false; } - } - if (auto stencil = TextureGLES::Cast(pass_data.stencil_attachment.get())) { - if (!stencil->SetAsFramebufferAttachment( - GL_FRAMEBUFFER, TextureGLES::AttachmentType::kStencil)) { - return false; + + if (auto depth = TextureGLES::Cast(pass_data.depth_attachment.get())) { + if (!depth->SetAsFramebufferAttachment( + GL_FRAMEBUFFER, TextureGLES::AttachmentType::kDepth)) { + return false; + } + } + if (auto stencil = + TextureGLES::Cast(pass_data.stencil_attachment.get())) { + if (!stencil->SetAsFramebufferAttachment( + GL_FRAMEBUFFER, TextureGLES::AttachmentType::kStencil)) { + return false; + } } - } - auto status = gl.CheckFramebufferStatus(GL_FRAMEBUFFER); - if (status != GL_FRAMEBUFFER_COMPLETE) { - VALIDATION_LOG << "Could not create a complete frambuffer: " - << DebugToFramebufferError(status); - return false; + auto status = gl.CheckFramebufferStatus(GL_FRAMEBUFFER); + if (status != GL_FRAMEBUFFER_COMPLETE) { + VALIDATION_LOG << "Could not create a complete frambuffer: " + << DebugToFramebufferError(status); + return false; + } } } diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 1b4b47f74b373..0567b9f4fab83 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -217,6 +217,9 @@ TextureGLES::TextureGLES(std::shared_ptr reactor, // |Texture| TextureGLES::~TextureGLES() { reactor_->CollectHandle(handle_); + if (cached_fbo_ != GL_NONE) { + reactor_->GetProcTable().DeleteFramebuffers(1, &cached_fbo_); + } } // |Texture| diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h index f5fbed558ea1f..edf59ed708388 100644 --- a/impeller/renderer/backend/gles/texture_gles.h +++ b/impeller/renderer/backend/gles/texture_gles.h @@ -130,6 +130,10 @@ class TextureGLES final : public Texture, /// void SetFence(HandleGLES fence); + void SetCachedFBO(GLuint fbo) { cached_fbo_ = fbo; } + + GLuint GetCachedFBO() const { return cached_fbo_; } + // Visible for testing. std::optional GetSyncFence() const; @@ -141,6 +145,7 @@ class TextureGLES final : public Texture, mutable std::bitset<6> slices_initialized_ = 0; const bool is_wrapped_; const std::optional wrapped_fbo_; + GLuint cached_fbo_ = GL_NONE; bool is_valid_ = false; TextureGLES(std::shared_ptr reactor, From d426def3e87f717e3e1df21f73f4c90f93fde6ae Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 21 Nov 2024 21:13:03 -0800 Subject: [PATCH 2/3] move to impl. --- impeller/renderer/backend/gles/texture_gles.cc | 5 +++++ impeller/renderer/backend/gles/texture_gles.h | 9 +++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 0567b9f4fab83..17723f2ab98d1 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -648,4 +648,9 @@ std::optional TextureGLES::GetSyncFence() const { return fence_; } +void TextureGLES::SetCachedFBO(GLuint fbo) { cached_fbo_ = fbo; } + +GLuint TextureGLES::GetCachedFBO() const { return cached_fbo_; } + + } // namespace impeller diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h index edf59ed708388..36f569379f70e 100644 --- a/impeller/renderer/backend/gles/texture_gles.h +++ b/impeller/renderer/backend/gles/texture_gles.h @@ -130,9 +130,14 @@ class TextureGLES final : public Texture, /// void SetFence(HandleGLES fence); - void SetCachedFBO(GLuint fbo) { cached_fbo_ = fbo; } + /// Store the FBO object for recycling in the 2D renderer. + /// + /// The color0 texture used by the 2D renderer will use this texture + /// object to store the associated FBO the first time it is used. + void SetCachedFBO(GLuint fbo); - GLuint GetCachedFBO() const { return cached_fbo_; } + /// Retrieve the cached FBO object, or GL_NONE if there is no object. + GLuint GetCachedFBO() const; // Visible for testing. std::optional GetSyncFence() const; From 7f535c02859a0b42419ba63a512ae7983764bc39 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 21 Nov 2024 21:16:01 -0800 Subject: [PATCH 3/3] ++ --- impeller/renderer/backend/gles/texture_gles.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 17723f2ab98d1..682e2cffd136a 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -648,9 +648,12 @@ std::optional TextureGLES::GetSyncFence() const { return fence_; } -void TextureGLES::SetCachedFBO(GLuint fbo) { cached_fbo_ = fbo; } - -GLuint TextureGLES::GetCachedFBO() const { return cached_fbo_; } +void TextureGLES::SetCachedFBO(GLuint fbo) { + cached_fbo_ = fbo; +} +GLuint TextureGLES::GetCachedFBO() const { + return cached_fbo_; +} } // namespace impeller