Skip to content

Commit d14d250

Browse files
[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.
1 parent 6b93cf9 commit d14d250

File tree

5 files changed

+24
-18
lines changed

5 files changed

+24
-18
lines changed

engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ bool ReactorGLES::RegisterCleanupCallback(const HandleGLES& handle,
200200
return false;
201201
}
202202

203-
HandleGLES ReactorGLES::CreateUntrackedHandle(HandleType type) {
203+
HandleGLES ReactorGLES::CreateUntrackedHandle(HandleType type) const {
204204
FML_DCHECK(CanReactOnCurrentThread());
205205
auto new_handle = HandleGLES::Create(type);
206206
std::optional<ReactorGLES::GLStorage> gl_handle =

engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ class ReactorGLES {
180180
/// creating/accessing the handle.
181181
/// @param type The type of handle to create.
182182
/// @return The reactor handle.
183-
HandleGLES CreateUntrackedHandle(HandleType type);
183+
HandleGLES CreateUntrackedHandle(HandleType type) const;
184184

185185
//----------------------------------------------------------------------------
186186
/// @brief Collect a reactor handle.

engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
211211
}
212212
#endif // IMPELLER_DEBUG
213213

214-
GLuint fbo = GL_NONE;
215214
TextureGLES& color_gles = TextureGLES::Cast(*pass_data.color_attachment);
216215
const bool is_default_fbo = color_gles.IsWrapped();
217216

@@ -222,14 +221,21 @@ void RenderPassGLES::ResetGLState(const ProcTableGLES& gl) {
222221
}
223222
} else {
224223
// Create and bind an offscreen FBO.
225-
GLuint cached_fbo = color_gles.GetCachedFBO();
226-
if (cached_fbo != GL_NONE) {
227-
fbo = cached_fbo;
228-
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo);
224+
if (!color_gles.GetCachedFBO().IsDead()) {
225+
auto fbo = reactor.GetGLHandle(color_gles.GetCachedFBO());
226+
if (!fbo.has_value()) {
227+
return false;
228+
}
229+
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo.value());
229230
} else {
230-
gl.GenFramebuffers(1u, &fbo);
231-
color_gles.SetCachedFBO(fbo);
232-
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo);
231+
HandleGLES cached_fbo =
232+
reactor.CreateUntrackedHandle(HandleType::kFrameBuffer);
233+
color_gles.SetCachedFBO(cached_fbo);
234+
auto fbo = reactor.GetGLHandle(cached_fbo);
235+
if (!fbo.has_value()) {
236+
return false;
237+
}
238+
gl.BindFramebuffer(GL_FRAMEBUFFER, fbo.value());
233239

234240
if (!color_gles.SetAsFramebufferAttachment(
235241
GL_FRAMEBUFFER, TextureGLES::AttachmentType::kColor0)) {

engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,8 @@ TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
219219
// |Texture|
220220
TextureGLES::~TextureGLES() {
221221
reactor_->CollectHandle(handle_);
222-
if (cached_fbo_ != GL_NONE) {
223-
reactor_->GetProcTable().DeleteFramebuffers(1, &cached_fbo_);
222+
if (!cached_fbo_.IsDead()) {
223+
reactor_->CollectHandle(cached_fbo_);
224224
}
225225
}
226226

@@ -659,11 +659,11 @@ std::optional<HandleGLES> TextureGLES::GetSyncFence() const {
659659
return fence_;
660660
}
661661

662-
void TextureGLES::SetCachedFBO(GLuint fbo) {
662+
void TextureGLES::SetCachedFBO(HandleGLES fbo) {
663663
cached_fbo_ = fbo;
664664
}
665665

666-
GLuint TextureGLES::GetCachedFBO() const {
666+
const HandleGLES& TextureGLES::GetCachedFBO() const {
667667
return cached_fbo_;
668668
}
669669

engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,10 @@ class TextureGLES final : public Texture,
136136
///
137137
/// The color0 texture used by the 2D renderer will use this texture
138138
/// object to store the associated FBO the first time it is used.
139-
void SetCachedFBO(GLuint fbo);
139+
void SetCachedFBO(HandleGLES fbo);
140140

141-
/// Retrieve the cached FBO object, or GL_NONE if there is no object.
142-
GLuint GetCachedFBO() const;
141+
/// Retrieve the cached FBO object, or a dead handle if there is no object.
142+
const HandleGLES& GetCachedFBO() const;
143143

144144
// Visible for testing.
145145
std::optional<HandleGLES> GetSyncFence() const;
@@ -155,7 +155,7 @@ class TextureGLES final : public Texture,
155155
mutable std::bitset<6> slices_initialized_ = 0;
156156
const bool is_wrapped_;
157157
const std::optional<GLuint> wrapped_fbo_;
158-
GLuint cached_fbo_ = GL_NONE;
158+
HandleGLES cached_fbo_ = HandleGLES::DeadHandle();
159159
bool is_valid_ = false;
160160

161161
TextureGLES(std::shared_ptr<ReactorGLES> reactor,

0 commit comments

Comments
 (0)