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

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

[dongwon.kim]

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);
+    }
+
+    /*
+     * 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);
+    }
 
     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

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

[Marc-André Lureau]

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?

Also, aren't these leaking framebuffers?

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

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

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

[Kim, Dongwon]

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