RE: [PATCH] ui/egl-helpers: Fix FBO recreation and prevent texture accidental deletion

["Kim, Dongwon" <dongwon.kim@intel.com>]

Hi Marc-André,

> -----Original Message-----
> From: Marc-André Lureau <marcandre.lureau@gmail.com>
> Sent: Tuesday, March 17, 2026 5:51 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH] ui/egl-helpers: Fix FBO recreation and prevent texture
> accidental deletion
> 
> Hi
> 
> On Tue, Mar 3, 2026 at 5:14 AM <dongwon.kim@intel.com> wrote:
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > When egl_fb_setup_for_tex is called, we must handle cases where the
> > texture ID is reused across different GL contexts.
> >
> > Texture Preservation - If the new texture ID matches the cached ID, we
> > must skip egl_fb_delete_texture to avoid destroying the texture we are
> > about to use.
> >
> > FBO Recreation - Because FBOs are context-local and not shared, a
> > cached FBO ID from a previous context is invalid. We must generate a
> > new FBO handle if we are re-validating the same texture ID in a
> > potentially new context.
> >
> > This prevents stale FBO usage and unintended texture deletion during
> > context transitions.
> >
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  ui/egl-helpers.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c index
> > e3f2872cc1..7ac64f3ba8 100644
> > --- a/ui/egl-helpers.c
> > +++ b/ui/egl-helpers.c
> > @@ -111,15 +111,23 @@ void egl_fb_setup_default(egl_fb *fb, int width,
> > int height, int x, int y)  void egl_fb_setup_for_tex(egl_fb *fb, int width, int
> height,
> >                            GLuint texture, bool delete)  {
> > -    egl_fb_delete_texture(fb);
> > +    if (fb->texture != texture) {
> > +        egl_fb_delete_texture(fb);
> > +    }
> 
> That change makes sense, and prevents reusing a destroyed texture.
> 
> > +
> > +    /*
> > +     * If fb->texture == texture, the existing fb->framebuffer is tied to
> > +     * a previous GL context. Since FBOs are not shared across contexts,
> > +     * we must create a new FBO for the current context.
> > +     */
> > +    if (!fb->framebuffer || (fb->texture == texture)) {
> > +        glGenFramebuffers(1, &fb->framebuffer);
> > +    }
> 
> This, I don't understand. Assuming the framebuffer is used in a different
> context, shouldn't it generate a new framebuffer regardless of the texture?

So this condition "fb->texture == texture" is used to determine whether the existing framebuffer
and texture (fb->framebuffer and fb->texture) are destroyed context's. But I just realized this can't be
always true as there will be cases where textures are different but context switching has happened.
I initially came with this idea to avoid adding too much for tracking context (i.e fb->context)
but I guess this is what we should do. I will take a look.

> 
> Also, aren't these leaking framebuffers?

Old context is destroyed at this point already so the assumption is that all resources are unreferenced. 

> 
> Can you identify cases where egl_fb_setup_for_tex() is reused in a different
> context? shouldn't we prevent this instead?

This happens when the user close the untabified window (gd_tab_window_close), which is an
asynchronous event.

> 
> thanks
> 
> >
> >      fb->width = width;
> >      fb->height = height;
> >      fb->texture = texture;
> >      fb->delete_texture = delete;
> > -    if (!fb->framebuffer) {
> > -        glGenFramebuffers(1, &fb->framebuffer);
> > -    }
> >
> >      glBindFramebuffer(GL_FRAMEBUFFER_EXT, fb->framebuffer);
> >      glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT,
> > GL_COLOR_ATTACHMENT0_EXT,
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Marc-André Lureau