From: Dongwon Kim <dongwon.kim@intel.com>

Drawing guest display frames can't be completed while the VC is not in
visible state, which could result in timeout in both the host and the
guest especially when using blob scanout. Therefore it is needed to
update and track the visiblity status of the VC and unblock the pipeline
in case when VC becomes invisible (e.g. windows minimization, switching
among tabs) while processing a guest frame.

First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to
VirtualConsole struct then set it only if the VC and its window is
visible.
 
Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is
invisible when the tab is closed or deactivated. This notifies the guest
that the associated guest display is not active anymore.

Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK
window-state-event. The flag, 'visible' is updated based on the
minization status of the window.

Dongwon Kim (3):
  ui/gtk: skip drawing guest scanout when associated VC is invisible
  ui/gtk: set the ui size to 0 when invisible
  ui/gtk: reset visible flag when window is minimized

 include/ui/gtk.h |  1 +
 ui/gtk-egl.c     |  8 +++++++
 ui/gtk-gl-area.c |  8 +++++++
 ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 77 insertions(+), 2 deletions(-)

-- 
2.34.1

Hi Marc-André Lureau,

Just a reminder.. I need your help reviewing this patch series. Please take a look at my messages at
https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html and
https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html

Thanks!!
DW

> -----Original Message-----
> From: qemu-devel-bounces+dongwon.kim=intel.com@nongnu.org <qemu-
> devel-bounces+dongwon.kim=intel.com@nongnu.org> On Behalf Of
> dongwon.kim@intel.com
> Sent: Tuesday, January 30, 2024 3:49 PM
> To: qemu-devel@nongnu.org
> Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> 
> From: Dongwon Kim <dongwon.kim@intel.com>
> 
> Drawing guest display frames can't be completed while the VC is not in visible
> state, which could result in timeout in both the host and the guest especially
> when using blob scanout. Therefore it is needed to update and track the visiblity
> status of the VC and unblock the pipeline in case when VC becomes invisible (e.g.
> windows minimization, switching among tabs) while processing a guest frame.
> 
> First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to VirtualConsole
> struct then set it only if the VC and its window is visible.
> 
> Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is invisible when
> the tab is closed or deactivated. This notifies the guest that the associated guest
> display is not active anymore.
> 
> Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK window-state-
> event. The flag, 'visible' is updated based on the minization status of the window.
> 
> Dongwon Kim (3):
>   ui/gtk: skip drawing guest scanout when associated VC is invisible
>   ui/gtk: set the ui size to 0 when invisible
>   ui/gtk: reset visible flag when window is minimized
> 
>  include/ui/gtk.h |  1 +
>  ui/gtk-egl.c     |  8 +++++++
>  ui/gtk-gl-area.c |  8 +++++++
>  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 77 insertions(+), 2 deletions(-)
> 
> --
> 2.34.1
> 

Hi Kim

I am uncomfortable with the series in general.

Not only we don't have the means to draw dmabuf/scanout "when
required", so resuming drawing won't work until the guest draws (this
is already a problem but you are only making it worse). And I also
think reconfiguring the guest by merely minimizing or switching
window/tabs isn't what most users would expect.

(fwiw, my personal opinion is that QEMU shouldn't provide UIs and
different clients should be able to implement different behaviours,
out of process.. that makes me relatively less motivated to break
things and be responsible)

Daniel, could you have a look too?

thanks

On Fri, Mar 1, 2024 at 4:05 AM Kim, Dongwon <dongwon.kim@intel.com> wrote:
>
> Hi Marc-André Lureau,
>
> Just a reminder.. I need your help reviewing this patch series. Please take a look at my messages at
> https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html and
> https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html
>
> Thanks!!
> DW
>
> > -----Original Message-----
> > From: qemu-devel-bounces+dongwon.kim=intel.com@nongnu.org <qemu-
> > devel-bounces+dongwon.kim=intel.com@nongnu.org> On Behalf Of
> > dongwon.kim@intel.com
> > Sent: Tuesday, January 30, 2024 3:49 PM
> > To: qemu-devel@nongnu.org
> > Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> >
> > From: Dongwon Kim <dongwon.kim@intel.com>
> >
> > Drawing guest display frames can't be completed while the VC is not in visible
> > state, which could result in timeout in both the host and the guest especially
> > when using blob scanout. Therefore it is needed to update and track the visiblity
> > status of the VC and unblock the pipeline in case when VC becomes invisible (e.g.
> > windows minimization, switching among tabs) while processing a guest frame.
> >
> > First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to VirtualConsole
> > struct then set it only if the VC and its window is visible.
> >
> > Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is invisible when
> > the tab is closed or deactivated. This notifies the guest that the associated guest
> > display is not active anymore.
> >
> > Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK window-state-
> > event. The flag, 'visible' is updated based on the minization status of the window.
> >
> > Dongwon Kim (3):
> >   ui/gtk: skip drawing guest scanout when associated VC is invisible
> >   ui/gtk: set the ui size to 0 when invisible
> >   ui/gtk: reset visible flag when window is minimized
> >
> >  include/ui/gtk.h |  1 +
> >  ui/gtk-egl.c     |  8 +++++++
> >  ui/gtk-gl-area.c |  8 +++++++
> >  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
> >  4 files changed, 77 insertions(+), 2 deletions(-)
> >
> > --
> > 2.34.1
> >
>

Hi Marc-André,

> -----Original Message-----
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> Sent: Tuesday, March 5, 2024 4:18 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>; P. Berrange, Daniel
> <berrange@redhat.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH 0/3] ui/gtk: introducing vc->visible
> 
> Hi Kim
> 
> I am uncomfortable with the series in general.
> 
> Not only we don't have the means to draw dmabuf/scanout "when required", so
> resuming drawing won't work until the guest draws (this is already a problem but
[Kim, Dongwon] Yes, this is right. The display won't be updated until there is a new frame submitted.
> you are only making it worse). And I also think reconfiguring the guest by merely
> minimizing or switching window/tabs isn't what most users would expect.
[Kim, Dongwon] I understand your concern. Then what do you suggest I need to do or look into to avoid the situation where the host rendering of the guest frame is scheduled but pending due to tab switching or minimization of window as the guest (virtio-gpu drv) is waiting for the response to move on to the next frame? Do you think the frame update should just continue on the host side regardless of visibility of the window? (If this is the standard expectation, then one of our Linux configuration - Yocto + ICE WM has some bug in it.)

> 
> (fwiw, my personal opinion is that QEMU shouldn't provide UIs and different
> clients should be able to implement different behaviours, out of process.. that
> makes me relatively less motivated to break things and be responsible)
> 
> Daniel, could you have a look too?
> 
> thanks
> 
> On Fri, Mar 1, 2024 at 4:05 AM Kim, Dongwon <dongwon.kim@intel.com>
> wrote:
> >
> > Hi Marc-André Lureau,
> >
> > Just a reminder.. I need your help reviewing this patch series. Please
> > take a look at my messages at
> > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html
> > and
> > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html
> >
> > Thanks!!
> > DW
> >
> > > -----Original Message-----
> > > From: qemu-devel-bounces+dongwon.kim=intel.com@nongnu.org <qemu-
> > > devel-bounces+dongwon.kim=intel.com@nongnu.org> On Behalf Of
> > > dongwon.kim@intel.com
> > > Sent: Tuesday, January 30, 2024 3:49 PM
> > > To: qemu-devel@nongnu.org
> > > Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> > >
> > > From: Dongwon Kim <dongwon.kim@intel.com>
> > >
> > > Drawing guest display frames can't be completed while the VC is not
> > > in visible state, which could result in timeout in both the host and
> > > the guest especially when using blob scanout. Therefore it is needed
> > > to update and track the visiblity status of the VC and unblock the pipeline in
> case when VC becomes invisible (e.g.
> > > windows minimization, switching among tabs) while processing a guest
> frame.
> > >
> > > First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to
> > > VirtualConsole struct then set it only if the VC and its window is visible.
> > >
> > > Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is
> > > invisible when the tab is closed or deactivated. This notifies the
> > > guest that the associated guest display is not active anymore.
> > >
> > > Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK
> > > window-state- event. The flag, 'visible' is updated based on the minization
> status of the window.
> > >
> > > Dongwon Kim (3):
> > >   ui/gtk: skip drawing guest scanout when associated VC is invisible
> > >   ui/gtk: set the ui size to 0 when invisible
> > >   ui/gtk: reset visible flag when window is minimized
> > >
> > >  include/ui/gtk.h |  1 +
> > >  ui/gtk-egl.c     |  8 +++++++
> > >  ui/gtk-gl-area.c |  8 +++++++
> > >  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
> > >  4 files changed, 77 insertions(+), 2 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >
> >

Hi

On Fri, Mar 8, 2024 at 4:59 AM Kim, Dongwon <dongwon.kim@intel.com> wrote:
>
> Hi Marc-André,
>
> > -----Original Message-----
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Sent: Tuesday, March 5, 2024 4:18 AM
> > To: Kim, Dongwon <dongwon.kim@intel.com>; P. Berrange, Daniel
> > <berrange@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > Subject: Re: [PATCH 0/3] ui/gtk: introducing vc->visible
> >
> > Hi Kim
> >
> > I am uncomfortable with the series in general.
> >
> > Not only we don't have the means to draw dmabuf/scanout "when required", so
> > resuming drawing won't work until the guest draws (this is already a problem but
> [Kim, Dongwon] Yes, this is right. The display won't be updated until there is a new frame submitted.
> > you are only making it worse). And I also think reconfiguring the guest by merely
> > minimizing or switching window/tabs isn't what most users would expect.
> [Kim, Dongwon] I understand your concern. Then what do you suggest I need to do or look into to avoid the situation where the host rendering of the guest frame is scheduled but pending due to tab switching or minimization of window as the guest (virtio-gpu drv) is waiting for the response to move on to the next frame? Do you think the frame update should just continue on the host side regardless of visibility of the window? (If this is the standard expectation, then one of our Linux configuration - Yocto + ICE WM has some bug in it.)
>


Given that we can't pause/resume the drawing, I think it's best to
always draw regardless of the visibility. If GTK doesn't draw when
minimized or hidden, we should find a way to "force" that.


> >
> > (fwiw, my personal opinion is that QEMU shouldn't provide UIs and different
> > clients should be able to implement different behaviours, out of process.. that
> > makes me relatively less motivated to break things and be responsible)
> >
> > Daniel, could you have a look too?
> >
> > thanks
> >
> > On Fri, Mar 1, 2024 at 4:05 AM Kim, Dongwon <dongwon.kim@intel.com>
> > wrote:
> > >
> > > Hi Marc-André Lureau,
> > >
> > > Just a reminder.. I need your help reviewing this patch series. Please
> > > take a look at my messages at
> > > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06636.html
> > > and
> > > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06637.html
> > >
> > > Thanks!!
> > > DW
> > >
> > > > -----Original Message-----
> > > > From: qemu-devel-bounces+dongwon.kim=intel.com@nongnu.org <qemu-
> > > > devel-bounces+dongwon.kim=intel.com@nongnu.org> On Behalf Of
> > > > dongwon.kim@intel.com
> > > > Sent: Tuesday, January 30, 2024 3:49 PM
> > > > To: qemu-devel@nongnu.org
> > > > Subject: [PATCH 0/3] ui/gtk: introducing vc->visible
> > > >
> > > > From: Dongwon Kim <dongwon.kim@intel.com>
> > > >
> > > > Drawing guest display frames can't be completed while the VC is not
> > > > in visible state, which could result in timeout in both the host and
> > > > the guest especially when using blob scanout. Therefore it is needed
> > > > to update and track the visiblity status of the VC and unblock the pipeline in
> > case when VC becomes invisible (e.g.
> > > > windows minimization, switching among tabs) while processing a guest
> > frame.
> > > >
> > > > First patch (0001-ui-gtk-skip...) is introducing a flag 'visible' to
> > > > VirtualConsole struct then set it only if the VC and its window is visible.
> > > >
> > > > Second patch (0002-ui-gtk-set-...) sets the ui size to 0 when VC is
> > > > invisible when the tab is closed or deactivated. This notifies the
> > > > guest that the associated guest display is not active anymore.
> > > >
> > > > Third patch (0003-ui-gtk-reset-visible...) adds a callback for GTK
> > > > window-state- event. The flag, 'visible' is updated based on the minization
> > status of the window.
> > > >
> > > > Dongwon Kim (3):
> > > >   ui/gtk: skip drawing guest scanout when associated VC is invisible
> > > >   ui/gtk: set the ui size to 0 when invisible
> > > >   ui/gtk: reset visible flag when window is minimized
> > > >
> > > >  include/ui/gtk.h |  1 +
> > > >  ui/gtk-egl.c     |  8 +++++++
> > > >  ui/gtk-gl-area.c |  8 +++++++
> > > >  ui/gtk.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
> > > >  4 files changed, 77 insertions(+), 2 deletions(-)
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > >
>


-- 
Marc-André Lureau

On Tue, Mar 05, 2024 at 04:18:18PM +0400, Marc-André Lureau wrote:
> Hi Kim
> 
> I am uncomfortable with the series in general.
> 
> Not only we don't have the means to draw dmabuf/scanout "when
> required", so resuming drawing won't work until the guest draws (this
> is already a problem but you are only making it worse). And I also
> think reconfiguring the guest by merely minimizing or switching
> window/tabs isn't what most users would expect.
> 
> Daniel, could you have a look too?

I'm similarly pretty uncomfortable with the proposed behaviour
as it feels like it would be introducing a whole new set of
unfortunate side-effects/problems.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|