Hi On Wed, Mar 9, 2022 at 2:01 PM Akihiko Odaki <akihiko.od...@gmail.com> wrote:
> On 2022/03/09 18:53, Marc-André Lureau wrote: > > Hi > > > > On Wed, Mar 9, 2022 at 1:32 PM Akihiko Odaki <akihiko.od...@gmail.com > > <mailto:akihiko.od...@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.od...@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. > At this point, your change is much larger than the proposed fixes. I already discussed the rationale for the current design. To summarize: - dispatching DCL in the common code allows for greater reuse if an alternative to dbus emerges, and should help making the code more dynamic - the GL context split also is a separation of concerns and should help for alternatives to EGL - dbus code only handles dbus specifics My understanding of your proposal is that you would rather see all this done within the dbus code. I disagree for the reasons above. I may be proven wrong, but so far, this works as expected minor the left-over and regressions you pointed out that should be fixed. Going back to a different design should be done in a next release if sufficiently motivated. -- Marc-André Lureau
