[PATCH v3 4/6] ui/console-gl: Add a helper to create a texture with linear memory layout

Vivek Kasireddy posted 6 patches 6 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 4/6] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Vivek Kasireddy 6 months, 2 weeks ago
There are cases where we do not want the memory layout of a texture to
be tiled as the component processing the texture would not know how to
de-tile either via software or hardware. Therefore, ensuring that the
memory backing the texture has a linear layout is absolutely necessary
in these situations.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/ui/console.h |  2 ++
 ui/console-gl.c      | 28 ++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/ui/console.h b/include/ui/console.h
index 46b3128185..fa2dd53e2e 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -422,6 +422,8 @@ bool console_gl_check_format(DisplayChangeListener *dcl,
                              pixman_format_code_t format);
 void surface_gl_create_texture(QemuGLShader *gls,
                                DisplaySurface *surface);
+void surface_gl_create_texture_from_fd(DisplaySurface *surface,
+                                       int fd, GLuint *texture);
 void surface_gl_update_texture(QemuGLShader *gls,
                                DisplaySurface *surface,
                                int x, int y, int w, int h);
diff --git a/ui/console-gl.c b/ui/console-gl.c
index 103b954017..f83012fed4 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -25,6 +25,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "ui/console.h"
 #include "ui/shader.h"
 
@@ -96,6 +97,33 @@ void surface_gl_create_texture(QemuGLShader *gls,
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
 }
 
+void surface_gl_create_texture_from_fd(DisplaySurface *surface,
+                                       int fd, GLuint *texture)
+{
+    unsigned long size = surface_stride(surface) * surface_height(surface);
+    GLuint mem_obj;
+
+    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
+        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
+        return;
+    }
+
+#ifdef GL_EXT_memory_object_fd
+    glCreateMemoryObjectsEXT(1, &mem_obj);
+    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
+    if (glGetError() != GL_NO_ERROR) {
+        error_report("spice: cannot import memory object from fd");
+        return;
+    }
+
+    glGenTextures(1, texture);
+    glBindTexture(GL_TEXTURE_2D, *texture);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT, GL_LINEAR_TILING_EXT);
+    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, surface_width(surface),
+                         surface_height(surface), mem_obj, 0);
+#endif
+}
+
 void surface_gl_update_texture(QemuGLShader *gls,
                                DisplaySurface *surface,
                                int x, int y, int w, int h)
-- 
2.49.0


Re: [PATCH v3 4/6] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Dmitry Osipenko 6 months, 2 weeks ago
On 4/29/25 09:08, Vivek Kasireddy wrote:
> +#ifdef GL_EXT_memory_object_fd
> +    glCreateMemoryObjectsEXT(1, &mem_obj);
> +    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
> +    if (glGetError() != GL_NO_ERROR) {
> +        error_report("spice: cannot import memory object from fd");
> +        return;
> +    }

glGetError() retains error once error occurred. Calling glGetError()
returns last error code and clears the error state. Hence you should add
a dummy glGetError() call before glCreateMemoryObjectsEXT() to clear the
potential stale GL errors, otherwise your code will handle unrelated GL
errors and report them as memfd import failures.

-- 
Best regards,
Dmitry
RE: [PATCH v3 4/6] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Kasireddy, Vivek 6 months, 2 weeks ago
Hi Dmitry,

> Subject: Re: [PATCH v3 4/6] ui/console-gl: Add a helper to create a texture
> with linear memory layout
> 
> On 4/29/25 09:08, Vivek Kasireddy wrote:
> > +#ifdef GL_EXT_memory_object_fd
> > +    glCreateMemoryObjectsEXT(1, &mem_obj);
> > +    glImportMemoryFdEXT(mem_obj, size,
> GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
> > +    if (glGetError() != GL_NO_ERROR) {
> > +        error_report("spice: cannot import memory object from fd");
> > +        return;
> > +    }
> 
> glGetError() retains error once error occurred. Calling glGetError()
> returns last error code and clears the error state. Hence you should add
> a dummy glGetError() call before glCreateMemoryObjectsEXT() to clear the
> potential stale GL errors, otherwise your code will handle unrelated GL
> errors and report them as memfd import failures.
Thank you for explaining how glGetError() works. I'll fix this in the next version.

Thanks,
Vivek

> 
> --
> Best regards,
> Dmitry
Re: [PATCH v3 4/6] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Marc-André Lureau 6 months, 2 weeks ago
Hi

On Tue, Apr 29, 2025 at 10:14 AM Vivek Kasireddy
<vivek.kasireddy@intel.com> wrote:
>
> There are cases where we do not want the memory layout of a texture to
> be tiled as the component processing the texture would not know how to
> de-tile either via software or hardware. Therefore, ensuring that the
> memory backing the texture has a linear layout is absolutely necessary
> in these situations.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Cc: Frediano Ziglio <freddy77@gmail.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  include/ui/console.h |  2 ++
>  ui/console-gl.c      | 28 ++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 46b3128185..fa2dd53e2e 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -422,6 +422,8 @@ bool console_gl_check_format(DisplayChangeListener *dcl,
>                               pixman_format_code_t format);
>  void surface_gl_create_texture(QemuGLShader *gls,
>                                 DisplaySurface *surface);
> +void surface_gl_create_texture_from_fd(DisplaySurface *surface,
> +                                       int fd, GLuint *texture);
>  void surface_gl_update_texture(QemuGLShader *gls,
>                                 DisplaySurface *surface,
>                                 int x, int y, int w, int h);
> diff --git a/ui/console-gl.c b/ui/console-gl.c
> index 103b954017..f83012fed4 100644
> --- a/ui/console-gl.c
> +++ b/ui/console-gl.c
> @@ -25,6 +25,7 @@
>   * THE SOFTWARE.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>  #include "ui/console.h"
>  #include "ui/shader.h"
>
> @@ -96,6 +97,33 @@ void surface_gl_create_texture(QemuGLShader *gls,
>      glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>  }
>
> +void surface_gl_create_texture_from_fd(DisplaySurface *surface,
> +                                       int fd, GLuint *texture)
> +{
> +    unsigned long size = surface_stride(surface) * surface_height(surface);
> +    GLuint mem_obj;
> +
> +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
> +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
> +        return;
> +    }
> +
> +#ifdef GL_EXT_memory_object_fd
> +    glCreateMemoryObjectsEXT(1, &mem_obj);
> +    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
> +    if (glGetError() != GL_NO_ERROR) {
> +        error_report("spice: cannot import memory object from fd");
> +        return;
> +    }
> +
> +    glGenTextures(1, texture);
> +    glBindTexture(GL_TEXTURE_2D, *texture);
> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT, GL_LINEAR_TILING_EXT);
> +    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, surface_width(surface),
> +                         surface_height(surface), mem_obj, 0);
> +#endif

I suggest making the function return a "bool" for success. return
*texture > 0 && glGetError() == GL_NO_ERROR  for example.

> +}
> +
>  void surface_gl_update_texture(QemuGLShader *gls,
>                                 DisplaySurface *surface,
>                                 int x, int y, int w, int h)
> --
> 2.49.0
>
>


-- 
Marc-André Lureau
RE: [PATCH v3 4/6] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Kasireddy, Vivek 6 months, 2 weeks ago
Hi Marc-Andre,

> 
> On Tue, Apr 29, 2025 at 10:14 AM Vivek Kasireddy
> <vivek.kasireddy@intel.com> wrote:
> >
> > There are cases where we do not want the memory layout of a texture to
> > be tiled as the component processing the texture would not know how to
> > de-tile either via software or hardware. Therefore, ensuring that the
> > memory backing the texture has a linear layout is absolutely necessary
> > in these situations.
> >
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Cc: Frediano Ziglio <freddy77@gmail.com>
> > Cc: Dongwon Kim <dongwon.kim@intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >  include/ui/console.h |  2 ++
> >  ui/console-gl.c      | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 46b3128185..fa2dd53e2e 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -422,6 +422,8 @@ bool
> console_gl_check_format(DisplayChangeListener *dcl,
> >                               pixman_format_code_t format);
> >  void surface_gl_create_texture(QemuGLShader *gls,
> >                                 DisplaySurface *surface);
> > +void surface_gl_create_texture_from_fd(DisplaySurface *surface,
> > +                                       int fd, GLuint *texture);
> >  void surface_gl_update_texture(QemuGLShader *gls,
> >                                 DisplaySurface *surface,
> >                                 int x, int y, int w, int h);
> > diff --git a/ui/console-gl.c b/ui/console-gl.c
> > index 103b954017..f83012fed4 100644
> > --- a/ui/console-gl.c
> > +++ b/ui/console-gl.c
> > @@ -25,6 +25,7 @@
> >   * THE SOFTWARE.
> >   */
> >  #include "qemu/osdep.h"
> > +#include "qemu/error-report.h"
> >  #include "ui/console.h"
> >  #include "ui/shader.h"
> >
> > @@ -96,6 +97,33 @@ void surface_gl_create_texture(QemuGLShader *gls,
> >      glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
> GL_LINEAR);
> >  }
> >
> > +void surface_gl_create_texture_from_fd(DisplaySurface *surface,
> > +                                       int fd, GLuint *texture)
> > +{
> > +    unsigned long size = surface_stride(surface) * surface_height(surface);
> > +    GLuint mem_obj;
> > +
> > +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
> > +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
> > +        return;
> > +    }
> > +
> > +#ifdef GL_EXT_memory_object_fd
> > +    glCreateMemoryObjectsEXT(1, &mem_obj);
> > +    glImportMemoryFdEXT(mem_obj, size,
> GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
> > +    if (glGetError() != GL_NO_ERROR) {
> > +        error_report("spice: cannot import memory object from fd");
> > +        return;
> > +    }
> > +
> > +    glGenTextures(1, texture);
> > +    glBindTexture(GL_TEXTURE_2D, *texture);
> > +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT,
> GL_LINEAR_TILING_EXT);
> > +    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8,
> surface_width(surface),
> > +                         surface_height(surface), mem_obj, 0);
> > +#endif
> 
> I suggest making the function return a "bool" for success. return
> *texture > 0 && glGetError() == GL_NO_ERROR  for example.
Sure, will do that in the next version.

Thanks,
Vivek

> 
> > +}
> > +
> >  void surface_gl_update_texture(QemuGLShader *gls,
> >                                 DisplaySurface *surface,
> >                                 int x, int y, int w, int h)
> > --
> > 2.49.0
> >
> >
> 
> 
> --
> Marc-André Lureau