diff mbox series

ui/egl: fix framebuffer reads

Message ID 20190909073911.24787-1-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series ui/egl: fix framebuffer reads | expand

Commit Message

Gerd Hoffmann Sept. 9, 2019, 7:39 a.m. UTC 
Fix egl_fb_read() to use the (destination) surface size instead of the
(source) framebuffer source for glReadPixels.  Pass the DisplaySurface
instead of the pixeldata pointer to egl_fb_read() to make this possible.

With that in place framebuffer reads work fine even if the surface and
framebuffer sizes don't match, so we can remove the guest-triggerable
asserts in egl_scanout_flush().

Buglink: https://bugzilla.redhat.com//show_bug.cgi?id=1749659
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/egl-helpers.h | 2 +-
 ui/egl-headless.c        | 4 +---
 ui/egl-helpers.c         | 6 +++---
 3 files changed, 5 insertions(+), 7 deletions(-)

Comments

Marc-André Lureau Sept. 9, 2019, 7:53 a.m. UTC | #1 
Hi

On Mon, Sep 9, 2019 at 11:40 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Fix egl_fb_read() to use the (destination) surface size instead of the
> (source) framebuffer source for glReadPixels.  Pass the DisplaySurface
> instead of the pixeldata pointer to egl_fb_read() to make this possible.
>
> With that in place framebuffer reads work fine even if the surface and
> framebuffer sizes don't match, so we can remove the guest-triggerable
> asserts in egl_scanout_flush().
>
> Buglink: https://bugzilla.redhat.com//show_bug.cgi?id=1749659
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

And if the the destinaton surface is larger than the source:
GLREADPIXELS(3G)
"Values for pixels that lie outside the window connected to the
current GL context are undefined."

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  include/ui/egl-helpers.h | 2 +-
>  ui/egl-headless.c        | 4 +---
>  ui/egl-helpers.c         | 6 +++---
>  3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> index dad19e9873fb..94a4b3e6f3bd 100644
> --- a/include/ui/egl-helpers.h
> +++ b/include/ui/egl-helpers.h
> @@ -25,7 +25,7 @@ void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
>                            GLuint texture, bool delete);
>  void egl_fb_setup_new_tex(egl_fb *fb, int width, int height);
>  void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip);
> -void egl_fb_read(void *dst, egl_fb *src);
> +void egl_fb_read(DisplaySurface *dst, egl_fb *src);
>
>  void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip);
>  void egl_texture_blend(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip,
> diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> index 05b2e7d7b17f..fe2a0d1eab98 100644
> --- a/ui/egl-headless.c
> +++ b/ui/egl-headless.c
> @@ -133,8 +133,6 @@ static void egl_scanout_flush(DisplayChangeListener *dcl,
>      if (!edpy->guest_fb.texture || !edpy->ds) {
>          return;
>      }
> -    assert(surface_width(edpy->ds)  == edpy->guest_fb.width);
> -    assert(surface_height(edpy->ds) == edpy->guest_fb.height);
>      assert(surface_format(edpy->ds) == PIXMAN_x8r8g8b8);
>
>      if (edpy->cursor_fb.texture) {
> @@ -149,7 +147,7 @@ static void egl_scanout_flush(DisplayChangeListener *dcl,
>          egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top);
>      }
>
> -    egl_fb_read(surface_data(edpy->ds), &edpy->blit_fb);
> +    egl_fb_read(edpy->ds, &edpy->blit_fb);
>      dpy_gfx_update(edpy->dcl.con, x, y, w, h);
>  }
>
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index edc53f6d3464..7c530c2825be 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -102,12 +102,12 @@ void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
>                        GL_COLOR_BUFFER_BIT, GL_LINEAR);
>  }
>
> -void egl_fb_read(void *dst, egl_fb *src)
> +void egl_fb_read(DisplaySurface *dst, egl_fb *src)
>  {
>      glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
>      glReadBuffer(GL_COLOR_ATTACHMENT0_EXT);
> -    glReadPixels(0, 0, src->width, src->height,
> -                 GL_BGRA, GL_UNSIGNED_BYTE, dst);
> +    glReadPixels(0, 0, surface_width(dst), surface_height(dst),
> +                 GL_BGRA, GL_UNSIGNED_BYTE, surface_data(dst));
>  }
>
>  void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip)
> --
> 2.18.1
>
>
diff mbox series

Patch

diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index dad19e9873fb..94a4b3e6f3bd 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -25,7 +25,7 @@  void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
                           GLuint texture, bool delete);
 void egl_fb_setup_new_tex(egl_fb *fb, int width, int height);
 void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip);
-void egl_fb_read(void *dst, egl_fb *src);
+void egl_fb_read(DisplaySurface *dst, egl_fb *src);
 
 void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip);
 void egl_texture_blend(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip,
diff --git a/ui/egl-headless.c b/ui/egl-headless.c
index 05b2e7d7b17f..fe2a0d1eab98 100644
--- a/ui/egl-headless.c
+++ b/ui/egl-headless.c
@@ -133,8 +133,6 @@  static void egl_scanout_flush(DisplayChangeListener *dcl,
     if (!edpy->guest_fb.texture || !edpy->ds) {
         return;
     }
-    assert(surface_width(edpy->ds)  == edpy->guest_fb.width);
-    assert(surface_height(edpy->ds) == edpy->guest_fb.height);
     assert(surface_format(edpy->ds) == PIXMAN_x8r8g8b8);
 
     if (edpy->cursor_fb.texture) {
@@ -149,7 +147,7 @@  static void egl_scanout_flush(DisplayChangeListener *dcl,
         egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top);
     }
 
-    egl_fb_read(surface_data(edpy->ds), &edpy->blit_fb);
+    egl_fb_read(edpy->ds, &edpy->blit_fb);
     dpy_gfx_update(edpy->dcl.con, x, y, w, h);
 }
 
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index edc53f6d3464..7c530c2825be 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -102,12 +102,12 @@  void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
                       GL_COLOR_BUFFER_BIT, GL_LINEAR);
 }
 
-void egl_fb_read(void *dst, egl_fb *src)
+void egl_fb_read(DisplaySurface *dst, egl_fb *src)
 {
     glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
     glReadBuffer(GL_COLOR_ATTACHMENT0_EXT);
-    glReadPixels(0, 0, src->width, src->height,
-                 GL_BGRA, GL_UNSIGNED_BYTE, dst);
+    glReadPixels(0, 0, surface_width(dst), surface_height(dst),
+                 GL_BGRA, GL_UNSIGNED_BYTE, surface_data(dst));
 }
 
 void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool flip)