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

[Akihiko Odaki]

On 2022/03/09 17:54, Marc-André Lureau wrote:
> Hi
>
> On Wed, Mar 9, 2022 at 12:49 PM Akihiko Odaki <akihiko.odaki@gmail.com 
> <mailto:akihiko.odaki@gmail.com>> wrote:
>
>     On 2022/03/09 17:40, Marc-André Lureau wrote:
>      > Hi
>      >
>      > On Wed, Mar 9, 2022 at 12:34 PM Akihiko Odaki
>     <akihiko.odaki@gmail.com <mailto:akihiko.odaki@gmail.com>
>      > <mailto:akihiko.odaki@gmail.com
>     <mailto:akihiko.odaki@gmail.com>>> wrote:
>      >
>      >     On 2022/03/09 17:33, Marc-André Lureau wrote:
>      >      > Hi
>      >      >
>      >      > On Wed, Mar 9, 2022 at 12:21 PM Akihiko Odaki
>      >     <akihiko.odaki@gmail.com <mailto:akihiko.odaki@gmail.com>
>     <mailto:akihiko.odaki@gmail.com <mailto:akihiko.odaki@gmail.com>>
>      >      > <mailto:akihiko.odaki@gmail.com
>     <mailto:akihiko.odaki@gmail.com>
>      >     <mailto:akihiko.odaki@gmail.com
>     <mailto:akihiko.odaki@gmail.com>>>> wrote:
>      >      >
>      >      >
>      >      >     If it is expected that dpy_gfx_update is required, it
>     should call
>      >      >     dpy_gfx_update. I agree it is not a right timing to
>     fix vnc
>      >     to remove
>      >      >     the implicit update as it is pre-existing.
>      >      >     However the lack of dpy_gfx_update call is a
>     regression and
>      >     should
>      >      >     be fixed.
>      >      >
>      >      >
>      >      > Calling dpy_gfx_update is done when the scanount.kind is
>     SURFACE.
>      >      >
>      >      > dpy_gfx_update is specific to SURFACE, GL uses dpy_gl_update.
>      >      >
>      >      > --
>      >      > Marc-André Lureau
>      >
>      >     egl-headless requires non-OpenGL to display the surface
>     content even if
>      >     scanout.kind is not SURFACE. Calling dpy_gfx_update is done
>     when the
>      >     scanount.kind is SURFACE is not enough.
>      >
>      >
>      > We are going in circles... egl-headless call dpy_gfx_update on
>      > dpy_gl_update.
>      > --
>      > Marc-André Lureau
>
>     Ok, let me summarize the situation.
>
>     The problem occurs in the following condition.
>     1. register_displaychangelistener of console_select is called for a
>     non-OpenGL display.
>     2. scanout.kind is SURFACE_TEXTURE or SURFACE_DMABUF.
>
>
> You mean SCANOUT_TEXTURE or SCANOUT_DMABUF.

Ah, yes, of course. My bad.

>     3. egl-headless is employed.
>
>     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.
>
>
> dpy_gfx_update  is called for non-opengl (SCANOUT_SURFACE)

I meant non-OpenGL "display" (e.g. vnc). scanout.kind is SCANOUT_TEXTURE 
or SCANOUT_SURFACE as I described.

>     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.
>
>
> Sorry, I don't see a regression. Can you translate that to an actually 
> user visible regression?

It shouldn't be visible to user because vnc is the only display which 
uses the code path and vnc does the implicit update. However it still 
violates the model common for all displays and it would break once 
egl-headless gets working on macOS. It is a regression if something is 
getting worse whether it is immediately visible for user or not.

Regards,
Akihiko Odaki

> --
> Marc-André Lureau