Hi On Wed, Apr 17, 2024 at 9:06 PM Kim, Dongwon <dongwon....@intel.com> wrote: > > Hi Daniel, > > > -----Original Message----- > > From: Daniel P. Berrangé <berra...@redhat.com> > > Sent: Wednesday, April 17, 2024 4:05 AM > > To: Kim, Dongwon <dongwon....@intel.com> > > Cc: qemu-devel@nongnu.org; marcandre.lur...@redhat.com > > Subject: Re: [PATCH v6 1/3] ui/console: Introduce > > dpy_gl_qemu_dmabuf_get_..() helpers > > > > On Tue, Apr 16, 2024 at 09:09:52PM -0700, dongwon....@intel.com wrote: > > > From: Dongwon Kim <dongwon....@intel.com> > > > > > > This commit introduces dpy_gl_qemu_dmabuf_get_... helpers to extract > > > specific fields from the QemuDmaBuf struct. It also updates all > > > instances where fields within the QemuDmaBuf struct are directly > > > accessed, replacing them with calls to these new helper functions. > > > > > > v6: fix typos in helper names in ui/spice-display.c > > > > > > Suggested-by: Marc-André Lureau <marcandre.lur...@redhat.com> > > > Cc: Philippe Mathieu-Daudé <phi...@linaro.org> > > > Cc: Vivek Kasireddy <vivek.kasire...@intel.com> > > > Signed-off-by: Dongwon Kim <dongwon....@intel.com> > > > --- > > > include/ui/console.h | 17 +++++ > > > hw/display/vhost-user-gpu.c | 6 +- > > > hw/display/virtio-gpu-udmabuf.c | 7 +- > > > hw/vfio/display.c | 15 +++-- > > > ui/console.c | 116 +++++++++++++++++++++++++++++++- > > > ui/dbus-console.c | 9 ++- > > > ui/dbus-listener.c | 43 +++++++----- > > > ui/egl-headless.c | 23 +++++-- > > > ui/egl-helpers.c | 47 +++++++------ > > > ui/gtk-egl.c | 48 ++++++++----- > > > ui/gtk-gl-area.c | 37 ++++++---- > > > ui/gtk.c | 6 +- > > > ui/spice-display.c | 50 ++++++++------ > > > 13 files changed, 316 insertions(+), 108 deletions(-) > > > > > > diff --git a/include/ui/console.h b/include/ui/console.h index > > > 0bc7a00ac0..6292943a82 100644 > > > --- a/include/ui/console.h > > > +++ b/include/ui/console.h > > > @@ -358,6 +358,23 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, > > QemuDmaBuf *dmabuf, > > > bool have_hot, uint32_t hot_x, uint32_t > > > hot_y); void dpy_gl_cursor_position(QemuConsole *con, > > > uint32_t pos_x, uint32_t pos_y); > > > + > > > +int32_t dpy_gl_qemu_dmabuf_get_fd(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_width(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_height(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_stride(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_fourcc(QemuDmaBuf *dmabuf); uint64_t > > > +dpy_gl_qemu_dmabuf_get_modifier(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_texture(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_x(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_y(QemuDmaBuf *dmabuf); uint32_t > > > +dpy_gl_qemu_dmabuf_get_backing_width(QemuDmaBuf *dmabuf); > > uint32_t > > > +dpy_gl_qemu_dmabuf_get_backing_height(QemuDmaBuf *dmabuf); bool > > > +dpy_gl_qemu_dmabuf_get_y0_top(QemuDmaBuf *dmabuf); void > > > +*dpy_gl_qemu_dmabuf_get_sync(QemuDmaBuf *dmabuf); int32_t > > > +dpy_gl_qemu_dmabuf_get_fence_fd(QemuDmaBuf *dmabuf); bool > > > +dpy_gl_qemu_dmabuf_get_allow_fences(QemuDmaBuf *dmabuf); bool > > > +dpy_gl_qemu_dmabuf_get_draw_submitted(QemuDmaBuf *dmabuf); > > > > IMHO these method names don't need a "dpy_gl_" prefix on them. Since > > they're accessors for the "QemuDmaBuf" struct, I think its sufficient to > > just > > have "qemu_dmabuf_" as the name prefix, making names more compact. > > > > The console.{h,c} files are a bit of a dumping ground for UI code. While > > QemuDmaBuf was just a struct with direct field access that's OK. > > > > With turning this into a more of an object with accessors, I think it would > > also > > be desirable to move the struct definition and all its methods into separate > > ui/dmabuf.{c,h} files, so its fully self-contained. > > [Kim, Dongwon] I am ok with changing function names and create > separate c and h for dmabuf helpers as you suggested. But I would > like to hear Marc-André's opinion about this suggestion before I make > such changes. > > Marc-André, do you have any thought on Daniel's suggestion?
Sure, I was about to ask the same. Anything we can do to slim ui/console.c helps :)