Re: [PATCH v3 12/12] ui/console: call gfx_switch() even if the current scanout is GL

[Akihiko Odaki <akihiko.odaki@gmail.com>]

On 2022/03/09 18:53, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Mar 9, 2022 at 1:32 PM Akihiko Odaki <akihiko.odaki@gmail.com 
> <mailto:akihiko.odaki@gmail.com>> wrote:
> 
>     On 2022/03/09 18:26, Gerd Hoffmann wrote:
>      >    Hi,
>      >
>      >> dpy_gfx_switch and dpy_gfx_update need to be called to finish the
>      >> initialization or switching of the non-OpenGL display. However,
>     the proposed
>      >> patch only calls dpy_gfx_switch.
>      >>
>      >> vnc actually does not need dpy_gfx_update because the vnc
>     implementation of
>      >> dpy_gfx_switch implicitly does the work for dpy_gfx_update, but
>     the model of
>      >> ui/console expects the two of dpy_gfx_switch and dpy_gfx_update
>     is separated
>      >> and only calling dpy_gfx_switch violates the model.
>     dpy_gfx_update used to
>      >> be called even in such a case before and it is a regression.
>      >
>      > Well, no, the ->dpy_gfx_switch() callback is supposed to do
>     everything
>      > needed to bring the new surface to the screen.  vnc isn't alone here,
>      > gtk for example does the same (see gd_switch()).
>      >
> 
> 
> If dpy_gfx_switch() implies a full dpy_gfx_update(), then we would need 
> another callback to just set the new surface. This would avoid 
> intermediary and useless switches to 2d/surface when the scanout is GL.
> 
> For consistency, we should also declare that gl_scanout_texture and 
> gl_scanout_dmabuf imply full update as well.
> 
>      > Yes, typically this is roughly the same an explicit
>     dpy_gfx_update call
>      > would do.  So this could be changed if it helps making the opengl
>     code
>      > paths less confusing, but that should be a separate patch series and
>      > separate discussion.
>      >
>      > take care,
>      >    Gerd
>      >
> 
>     Then ui/cocoa is probably wrong. I don't think it does the update when
>     dpy_gfx_switch is called.
> 
>     Please tell me if you think dpy_gfx_switch shouldn't do the implicit
>     update in the future. I'll write a patch to do the update in cocoa's
>     dpy_gfx_switch implementation otherwise.
> 
> 
> Can we ack this series first and iterate on top? It solves a number of 
> issues already and is a better starting point.
> 
> thanks
> 
> -- 
> Marc-André Lureau

The call of dpy_gfx_update in displaychangelistener_display_console 
should be removed. It would simplify the patch.

Also it is still not shown that the series is a better alternative to:
https://patchew.org/QEMU/20220213024222.3548-1-akihiko.odaki@gmail.com/

The series "ui/dbus: Share one listener for a console" has significantly 
less code than this series and therefore needs some reasoning for that.

Regards,
Akihiko Odaki