qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] ui/gtk-egl: blitting partial guest fb to the proper scan


From: Dongwon Kim
Subject: Re: [PATCH 2/2] ui/gtk-egl: blitting partial guest fb to the proper scanout surface
Date: Tue, 20 Jul 2021 16:29:39 -0700
User-agent: Mutt/1.9.4 (2018-02-28)

On Sun, Jul 18, 2021 at 11:35:35PM -0700, Kasireddy, Vivek wrote:
> Hi DW,
> 
> > eb_fb_blit needs more parameters which describe x and y offsets and width
> > and height of the actual scanout to specify the size and cordination of
> > partial image to blit in the guest fb in case the guest fb contains multiple
> > display outputs.
> > 
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  hw/display/virtio-gpu-udmabuf.c |  4 ++--
> >  include/ui/egl-helpers.h        |  2 +-
> >  ui/egl-headless.c               |  2 +-
> >  ui/egl-helpers.c                | 10 ++++++----
> >  ui/gtk-egl.c                    |  7 ++++---
> >  ui/sdl2-gl.c                    |  2 +-
> >  6 files changed, 15 insertions(+), 12 deletions(-)
> > 
> > diff --git a/hw/display/virtio-gpu-udmabuf.c 
> > b/hw/display/virtio-gpu-udmabuf.c
> [Kasireddy, Vivek] You might not want to mix virtio-gpu and UI changes in the 
> same patch.
[DW] Yeah, I will split it.

> 
> > index a64194c6de..3ea6e76371 100644
> > --- a/hw/display/virtio-gpu-udmabuf.c
> > +++ b/hw/display/virtio-gpu-udmabuf.c
> > @@ -186,8 +186,8 @@ static VGPUDMABuf
> >      dmabuf->buf.stride = fb->stride;
> >      dmabuf->buf.x = r->x;
> >      dmabuf->buf.y = r->y;
> > -    dmabuf->buf.scanout_width;
> > -    dmabuf->buf.scanout_height;
> > +    dmabuf->buf.scanout_width = r->width;
> > +    dmabuf->buf.scanout_height = r->height;
> >      dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
> >      dmabuf->buf.fd = res->dmabuf_fd;
> > 
> > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> > index f1bf8f97fc..e21118501e 100644
> > --- a/include/ui/egl-helpers.h
> > +++ b/include/ui/egl-helpers.h
> > @@ -26,7 +26,7 @@ void egl_fb_setup_default(egl_fb *fb, int width, int 
> > height);
> >  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_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, 
> > bool flip);
> >  void egl_fb_read(DisplaySurface *dst, egl_fb *src);
> > 
> >  void egl_texture_blit(QemuGLShader *gls, egl_fb *dst, egl_fb *src, bool 
> > flip);
> > diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> > index da377a74af..bdf10fec84 100644
> > --- a/ui/egl-headless.c
> > +++ b/ui/egl-headless.c
> > @@ -144,7 +144,7 @@ static void egl_scanout_flush(DisplayChangeListener 
> > *dcl,
> >                            1.0, 1.0);
> >      } else {
> >          /* no cursor -> use simple framebuffer blit */
> > -        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, edpy->y_0_top);
> > +        egl_fb_blit(&edpy->blit_fb, &edpy->guest_fb, x, y, w, h, 
> > edpy->y_0_top);
> >      }
> > 
> >      egl_fb_read(edpy->ds, &edpy->blit_fb);
> > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > index 6d0cb2b5cb..2af3dcc0a6 100644
> > --- a/ui/egl-helpers.c
> > +++ b/ui/egl-helpers.c
> > @@ -88,16 +88,18 @@ void egl_fb_setup_new_tex(egl_fb *fb, int width, int 
> > height)
> >      egl_fb_setup_for_tex(fb, width, height, texture, true);
> >  }
> > 
> > -void egl_fb_blit(egl_fb *dst, egl_fb *src, bool flip)
> > +void egl_fb_blit(egl_fb *dst, egl_fb *src, int x, int y, int w, int h, 
> > bool flip)
> [Kasireddy, Vivek] Instead of explicitly passing x, y, w, h to egl_fb_blit, 
> would you be not
> be able to use the dmabuf member that would be added to egl_fb that would 
> contain x, y, w and h:
> https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg06746.html

[DW] sounds like a valid idea but wouldn't it be making this function
too specific? I think it is reasonable to specify the offset and the
size of blitted area.. although I agree that having too many parameters
don't look good.

> 
> 
> >  {
> >      GLuint y1, y2;
> > 
> >      glBindFramebuffer(GL_READ_FRAMEBUFFER, src->framebuffer);
> >      glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst->framebuffer);
> >      glViewport(0, 0, dst->width, dst->height);
> > -    y1 = flip ? src->height : 0;
> > -    y2 = flip ? 0 : src->height;
> > -    glBlitFramebuffer(0, y1, src->width, y2,
> > +    w = (x + w) > src->width ? src->width - x : w;
> > +    h = (y + h) > src->height ? src->height - y : h;
> > +    y1 = flip ? h + y : y;
> > +    y2 = flip ? y : h + y;
> > +    glBlitFramebuffer(x, y1, x + w, y2,
> [Kasireddy, Vivek] While you are at it, could you please create new local 
> variables x1, y1, x2, y2
> to store the above values and pass them to glBlitFramebuffer to improve the 
> readability of this code? 
[DW] I will think about making this look more undertandable.

> 
> Thanks,
> Vivek
> >                        0, 0, dst->width, dst->height,
> >                        GL_COLOR_BUFFER_BIT, GL_LINEAR);
> >  }
> > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
> > index 2a2e6d3a17..ceb52b1045 100644
> > --- a/ui/gtk-egl.c
> > +++ b/ui/gtk-egl.c
> > @@ -73,7 +73,7 @@ void gd_egl_draw(VirtualConsole *vc)
> >      wh = gdk_window_get_height(window);
> > 
> >      if (vc->gfx.scanout_mode) {
> > -        gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
> > +       gd_egl_scanout_flush(&vc->gfx.dcl, vc->gfx.x, vc->gfx.y, vc->gfx.w, 
> > vc->gfx.h);
> > 
> >          vc->gfx.scale_x = (double)ww / vc->gfx.w;
> >          vc->gfx.scale_y = (double)wh / vc->gfx.h;
> > @@ -216,7 +216,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
> > 
> >      gd_egl_scanout_texture(dcl, dmabuf->texture,
> >                             false, dmabuf->width, dmabuf->height,
> > -                           0, 0, dmabuf->width, dmabuf->height);
> > +                           dmabuf->x, dmabuf->y, dmabuf->scanout_width,
> > +                           dmabuf->scanout_height);
> >  #endif
> >  }
> > 
> > @@ -286,7 +287,7 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
> >                            vc->gfx.cursor_x, vc->gfx.cursor_y,
> >                            vc->gfx.scale_x, vc->gfx.scale_y);
> >      } else {
> > -        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top);
> > +        egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, x, y, w, h, 
> > !vc->gfx.y0_top);
> >      }
> > 
> >      eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
> > diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> > index a21d2deed9..67bc8b5f4e 100644
> > --- a/ui/sdl2-gl.c
> > +++ b/ui/sdl2-gl.c
> > @@ -238,7 +238,7 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
> > 
> >      SDL_GetWindowSize(scon->real_window, &ww, &wh);
> >      egl_fb_setup_default(&scon->win_fb, ww, wh);
> > -    egl_fb_blit(&scon->win_fb, &scon->guest_fb, !scon->y0_top);
> > +    egl_fb_blit(&scon->win_fb, &scon->guest_fb, x, y, w, h, !scon->y0_top);
> > 
> >      SDL_GL_SwapWindow(scon->real_window);
> >      graphic_hw_gl_flushed(dcl->con);
> > --
> > 2.17.1
> > 
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]