Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] cache and reuse openGL framebuffer attachments. #56746

Merged
merged 3 commits into from
Nov 22, 2024
Merged

[Impeller] cache and reuse openGL framebuffer attachments. #56746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6571616
cache and reuse FBOs
jonahwilliams Nov 22, 2024
d426def
move to impl.
jonahwilliams Nov 22, 2024
7f535c0
++
jonahwilliams Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 29 additions & 28 deletions impeller/renderer/backend/gles/render_pass_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from looking at profiles binding this framebuffer actually triggers a driver workload to flush from fb0.

gl.DeleteFramebuffers(1u, &fbo);
}
});

TextureGLES& color_gles = TextureGLES::Cast(*pass_data.color_attachment);
const bool is_default_fbo = color_gles.IsWrapped();

Expand All @@ -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;
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions impeller/renderer/backend/gles/texture_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,9 @@ TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
// |Texture|
TextureGLES::~TextureGLES() {
reactor_->CollectHandle(handle_);
if (cached_fbo_ != GL_NONE) {
reactor_->GetProcTable().DeleteFramebuffers(1, &cached_fbo_);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit fuzzy on how reactor is supposed to work. Are you supposed to schedule this to be deleted with it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only if its stored in the reactor as a Handle, which this one isn't

}
}

// |Texture|
Expand Down Expand Up @@ -645,4 +648,12 @@ std::optional<HandleGLES> TextureGLES::GetSyncFence() const {
return fence_;
}

void TextureGLES::SetCachedFBO(GLuint fbo) {
cached_fbo_ = fbo;
}

GLuint TextureGLES::GetCachedFBO() const {
return cached_fbo_;
}

} // namespace impeller
10 changes: 10 additions & 0 deletions impeller/renderer/backend/gles/texture_gles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,15 @@ class TextureGLES final : public Texture,
///
void SetFence(HandleGLES fence);

/// 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);

/// Retrieve the cached FBO object, or GL_NONE if there is no object.
GLuint GetCachedFBO() const;

// Visible for testing.
std::optional<HandleGLES> GetSyncFence() const;

Expand All @@ -141,6 +150,7 @@ class TextureGLES final : public Texture,
mutable std::bitset<6> slices_initialized_ = 0;
const bool is_wrapped_;
const std::optional<GLuint> wrapped_fbo_;
GLuint cached_fbo_ = GL_NONE;
bool is_valid_ = false;

TextureGLES(std::shared_ptr<ReactorGLES> reactor,
Expand Down