From ad071c64f9cfb1d3860a8e9574a9a55c486816e7 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Tue, 26 Oct 2021 18:19:12 +0200 Subject: [PATCH] =?UTF-8?q?cogl:=20Rebind=20the=20EGL=E2=80=AFimage=20when?= =?UTF-8?q?=20handling=20damage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Cogl gained support for importing pixmaps, I think there was a misunderstanding that there is a difference in how it works in GLX and EGL where GLX needs to rebind the pixmap in order to guarantee that changes are reflected in the texture after it detects damage, whereas with EGL it doesn’t. The GLX spec makes it pretty clear that it does need to rebind whereas the EGL spec is a bit harder to follow. As a fallout from Mesa MR 12869, it seems like the compositor really does need to rebind the image to comply with the spec. Notably, in OES_EGL_image_external there is: "Binding (or re-binding if already bound) an external texture by calling BindTexture after all modifications are complete guarantees that sampling done in future draw calls will return values corresponding to the values in the buffer at or after the time that BindTexture is called." So this commit changes the x11_damage_notify handler for EGL to lazily queue a rebind like GLX does. The code that binds the image while allocating the texture has been moved into a reusable helper function. It seems like there is a bit of a layering violation when accessing the GL driver internals from the EGL winsys code, but I noticed that the GLX code also includes the driver GL headers and otherwise it seems pretty tricky to do properly. Part-of: --- .../driver/gl/cogl-texture-2d-gl-private.h | 7 ++++ cogl/cogl/driver/gl/cogl-texture-2d-gl.c | 38 ++++++++++++++----- cogl/cogl/winsys/cogl-winsys-egl-x11.c | 32 ++++++++++++++++ 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/cogl/cogl/driver/gl/cogl-texture-2d-gl-private.h b/cogl/cogl/driver/gl/cogl-texture-2d-gl-private.h index c10637627..574c10fb0 100644 --- a/cogl/cogl/driver/gl/cogl-texture-2d-gl-private.h +++ b/cogl/cogl/driver/gl/cogl-texture-2d-gl-private.h @@ -60,6 +60,13 @@ _cogl_texture_2d_gl_new_from_bitmap (CoglBitmap *bmp, gboolean can_convert_in_place, GError **error); +#if defined (COGL_HAS_EGL_SUPPORT) +gboolean +cogl_texture_2d_gl_bind_egl_image (CoglTexture2D *tex_2d, + EGLImageKHR image, + GError **error); +#endif + #if defined (COGL_HAS_EGL_SUPPORT) && defined (EGL_KHR_image_base) CoglTexture2D * _cogl_egl_texture_2d_gl_new_from_image (CoglContext *ctx, diff --git a/cogl/cogl/driver/gl/cogl-texture-2d-gl.c b/cogl/cogl/driver/gl/cogl-texture-2d-gl.c index 4862dbe2e..23111eabf 100644 --- a/cogl/cogl/driver/gl/cogl-texture-2d-gl.c +++ b/cogl/cogl/driver/gl/cogl-texture-2d-gl.c @@ -274,18 +274,11 @@ allocate_from_egl_image (CoglTexture2D *tex_2d, tex_2d->gl_texture = ctx->texture_driver->gen (ctx, GL_TEXTURE_2D, internal_format); - _cogl_bind_gl_texture_transient (GL_TEXTURE_2D, - tex_2d->gl_texture); - _cogl_gl_util_clear_gl_errors (ctx); - ctx->glEGLImageTargetTexture2D (GL_TEXTURE_2D, loader->src.egl_image.image); - if (_cogl_gl_util_get_error (ctx) != GL_NO_ERROR) + if (!cogl_texture_2d_gl_bind_egl_image (tex_2d, + loader->src.egl_image.image, + error)) { - g_set_error_literal (error, - COGL_TEXTURE_ERROR, - COGL_TEXTURE_ERROR_BAD_PARAMETER, - "Could not create a CoglTexture2D from a given " - "EGLImage"); GE( ctx, glDeleteTextures (1, &tex_2d->gl_texture) ); return FALSE; } @@ -360,6 +353,31 @@ allocate_custom_egl_image_external (CoglTexture2D *tex_2d, return TRUE; } +gboolean +cogl_texture_2d_gl_bind_egl_image (CoglTexture2D *tex_2d, + EGLImageKHR image, + GError **error) +{ + CoglContext *ctx = COGL_TEXTURE (tex_2d)->context; + + _cogl_bind_gl_texture_transient (GL_TEXTURE_2D, + tex_2d->gl_texture); + _cogl_gl_util_clear_gl_errors (ctx); + + ctx->glEGLImageTargetTexture2D (GL_TEXTURE_2D, image); + if (_cogl_gl_util_get_error (ctx) != GL_NO_ERROR) + { + g_set_error_literal (error, + COGL_TEXTURE_ERROR, + COGL_TEXTURE_ERROR_BAD_PARAMETER, + "Could not bind the given EGLImage to a " + "CoglTexture2D"); + return FALSE; + } + + return TRUE; +} + CoglTexture2D * cogl_texture_2d_new_from_egl_image_external (CoglContext *ctx, int width, diff --git a/cogl/cogl/winsys/cogl-winsys-egl-x11.c b/cogl/cogl/winsys/cogl-winsys-egl-x11.c index abe740a21..bef385a55 100644 --- a/cogl/cogl/winsys/cogl-winsys-egl-x11.c +++ b/cogl/cogl/winsys/cogl-winsys-egl-x11.c @@ -43,6 +43,7 @@ #include "cogl-renderer-private.h" #include "cogl-texture-pixmap-x11-private.h" #include "cogl-texture-2d-private.h" +#include "driver/gl/cogl-texture-2d-gl-private.h" #include "cogl-texture-2d.h" #include "cogl-poll-private.h" #include "winsys/cogl-onscreen-egl.h" @@ -62,6 +63,7 @@ typedef struct _CoglTexturePixmapEGL { EGLImageKHR image; CoglTexture *texture; + gboolean bind_tex_image_queued; } CoglTexturePixmapEGL; #endif @@ -496,6 +498,9 @@ _cogl_winsys_texture_pixmap_x11_create (CoglTexturePixmapX11 *tex_pixmap) COGL_EGL_IMAGE_FLAG_NONE, NULL)); + /* The image is initially bound as part of the creation */ + egl_tex_pixmap->bind_tex_image_queued = FALSE; + tex_pixmap->winsys = egl_tex_pixmap; return TRUE; @@ -530,15 +535,42 @@ _cogl_winsys_texture_pixmap_x11_update (CoglTexturePixmapX11 *tex_pixmap, CoglTexturePixmapStereoMode stereo_mode, gboolean needs_mipmap) { + CoglTexturePixmapEGL *egl_tex_pixmap = tex_pixmap->winsys; + CoglTexture2D *tex_2d; + GError *error = NULL; + if (needs_mipmap) return FALSE; + if (egl_tex_pixmap->bind_tex_image_queued) + { + COGL_NOTE (TEXTURE_PIXMAP, "Rebinding GLXPixmap for %p", tex_pixmap); + + tex_2d = COGL_TEXTURE_2D (egl_tex_pixmap->texture); + + if (cogl_texture_2d_gl_bind_egl_image (tex_2d, + egl_tex_pixmap->image, + &error)) + { + egl_tex_pixmap->bind_tex_image_queued = FALSE; + } + else + { + g_warning ("Failed to rebind EGLImage to CoglTexture2D: %s", + error->message); + g_error_free (error); + } + } + return TRUE; } static void _cogl_winsys_texture_pixmap_x11_damage_notify (CoglTexturePixmapX11 *tex_pixmap) { + CoglTexturePixmapEGL *egl_tex_pixmap = tex_pixmap->winsys; + + egl_tex_pixmap->bind_tex_image_queued = TRUE; } static CoglTexture *