[Impeller] Store the TextureGLES cached framebuffer object as a reactor handle (#164761)

TextureGLES references may be owned by garbage collected objects. If a
GC drops the last reference to a TextureGLES, then the TextureGLES
destructor will run on a thread that does not have an EGL context. That
will cause failures when the destructor tries to delete the cached FBO
held by the TextureGLES.

This PR replaces the raw FBO handle with a ReactorGLES untracked handle.
The ReactorGLES will schedule deletion of the underlying FBO on a thread
that can call GLES APIs.
This commit is contained in:
Jason Simmons 2025-03-07 18:50:23 +00:00 committed by GitHub
parent 6b93cf93c1
commit d14d2505f3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 24 additions and 18 deletions

View File

@ -200,7 +200,7 @@ bool ReactorGLES::RegisterCleanupCallback(const HandleGLES& handle,
return false; return false;
} }
HandleGLES ReactorGLES::CreateUntrackedHandle(HandleType type) { HandleGLES ReactorGLES::CreateUntrackedHandle(HandleType type) const {
FML_DCHECK(CanReactOnCurrentThread()); FML_DCHECK(CanReactOnCurrentThread());
auto new_handle = HandleGLES::Create(type); auto new_handle = HandleGLES::Create(type);
std::optional<ReactorGLES::GLStorage> gl_handle = std::optional<ReactorGLES::GLStorage> gl_handle =

View File

@ -180,7 +180,7 @@ class ReactorGLES {
/// creating/accessing the handle. /// creating/accessing the handle.
/// @param type The type of handle to create. /// @param type The type of handle to create.
/// @return The reactor handle. /// @return The reactor handle.
HandleGLES CreateUntrackedHandle(HandleType type); HandleGLES CreateUntrackedHandle(HandleType type) const;
//---------------------------------------------------------------------------- //----------------------------------------------------------------------------
/// @brief Collect a reactor handle. /// @brief Collect a reactor handle.

View File

@ -211,7 +211,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
} }
#endif // IMPELLER_DEBUG #endif // IMPELLER_DEBUG
GLuint fbo = GL_NONE;
TextureGLES& color_gles = TextureGLES::Cast(*pass_data.color_attachment); TextureGLES& color_gles = TextureGLES::Cast(*pass_data.color_attachment);
const bool is_default_fbo = color_gles.IsWrapped(); const bool is_default_fbo = color_gles.IsWrapped();
@ -222,14 +221,21 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
} }
} else { } else {
// Create and bind an offscreen FBO. // Create and bind an offscreen FBO.
GLuint cached_fbo = color_gles.GetCachedFBO(); if (!color_gles.GetCachedFBO().IsDead()) {
if (cached_fbo != GL_NONE) { auto fbo = reactor.GetGLHandle(color_gles.GetCachedFBO());
fbo = cached_fbo; if (!fbo.has_value()) {
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); return false;
}
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo.value());
} else { } else {
gl.GenFramebuffers(1u, &fbo); HandleGLES cached_fbo =
color_gles.SetCachedFBO(fbo); reactor.CreateUntrackedHandle(HandleType::kFrameBuffer);
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo); color_gles.SetCachedFBO(cached_fbo);
auto fbo = reactor.GetGLHandle(cached_fbo);
if (!fbo.has_value()) {
return false;
}
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo.value());
if (!color_gles.SetAsFramebufferAttachment( if (!color_gles.SetAsFramebufferAttachment(
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) { GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {

View File

@ -219,8 +219,8 @@ TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
// |Texture| // |Texture|
TextureGLES::~TextureGLES() { TextureGLES::~TextureGLES() {
reactor_->CollectHandle(handle_); reactor_->CollectHandle(handle_);
if (cached_fbo_ != GL_NONE) { if (!cached_fbo_.IsDead()) {
reactor_->GetProcTable().DeleteFramebuffers(1, &cached_fbo_); reactor_->CollectHandle(cached_fbo_);
} }
} }
@ -659,11 +659,11 @@ std::optional<HandleGLES> TextureGLES::GetSyncFence() const {
return fence_; return fence_;
} }
void TextureGLES::SetCachedFBO(GLuint fbo) { void TextureGLES::SetCachedFBO(HandleGLES fbo) {
cached_fbo_ = fbo; cached_fbo_ = fbo;
} }
GLuint TextureGLES::GetCachedFBO() const { const HandleGLES& TextureGLES::GetCachedFBO() const {
return cached_fbo_; return cached_fbo_;
} }

View File

@ -136,10 +136,10 @@ class TextureGLES final : public Texture,
/// ///
/// The color0 texture used by the 2D renderer will use this texture /// The color0 texture used by the 2D renderer will use this texture
/// object to store the associated FBO the first time it is used. /// object to store the associated FBO the first time it is used.
void SetCachedFBO(GLuint fbo); void SetCachedFBO(HandleGLES fbo);
/// Retrieve the cached FBO object, or GL_NONE if there is no object. /// Retrieve the cached FBO object, or a dead handle if there is no object.
GLuint GetCachedFBO() const; const HandleGLES& GetCachedFBO() const;
// Visible for testing. // Visible for testing.
std::optional<HandleGLES> GetSyncFence() const; std::optional<HandleGLES> GetSyncFence() const;
@ -155,7 +155,7 @@ class TextureGLES final : public Texture,
mutable std::bitset<6> slices_initialized_ = 0; mutable std::bitset<6> slices_initialized_ = 0;
const bool is_wrapped_; const bool is_wrapped_;
const std::optional<GLuint> wrapped_fbo_; const std::optional<GLuint> wrapped_fbo_;
GLuint cached_fbo_ = GL_NONE; HandleGLES cached_fbo_ = HandleGLES::DeadHandle();
bool is_valid_ = false; bool is_valid_ = false;
TextureGLES(std::shared_ptr<ReactorGLES> reactor, TextureGLES(std::shared_ptr<ReactorGLES> reactor,