[Spice-devel] [PATCH spice-gtk 0/2] Make sure to set initial cursor

[Marc-André Lureau]

Hi

----- Original Message -----
> Hi Marc-André,
> 
> On Fri, 2017-04-28 at 04:54 -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> > > Hi,
> > > 
> > > It can happen that the initial cursor is not set for some monitors
> > > (SpiceDisplays)
> > > in a multimonitor linux guest env (there is a single cursor
> > > channel). To fix
> > > the problem a new class is created to handle gtk/gdk properties of
> > > the remote
> > > cursor. There is a 1:1 relation between SpiceChannelCursorGtk and
> > > SpiceChannelCursor
> > > (similar as SpiceSession and SpiceGtkSession).
> > > 
> > > Maybe it is a too heavy approach. The other approach I though
> > > about is
> > > setting
> > > the cursor GdkPixbuf as data to the cursor channel. In the end I
> > > decided to
> > > go
> > > for the class, imho it is more clean.
> > 
> > Why not just save the last display_cursor in the cursor channel and
> > add a "cursor" property?
> 
> I don't want to add the gtk/gdk dependency to spice-glib. Or all the
> display widgets would have to create their own GdkPixbufs.

No, no gtk dependency in spice-glib. I think you should expose some/all of the channel-cursor.c struct display_cursor.

> >  It would need to be a public cursor struct, introspectable etc.
> Do we need a new api? The struct would have to contain all the info
> provided by the "cursor-set" signal. I try to not add a new api when
> it is not needed.

Well it is needed, you just add the facility at the gtk level where it could be at a lower level.

The cursor channel API isn't that great, it reflects only the protocol events, it could be made a bit higher level and expose the state instead.

> 
> > 
> > Tbh, I think the cursor channel should have been designed that way
> > (and not just reflect the protocol events). We can then later think
> > about deprecating SET/RESET/HIDE events.
> That is an advantage of the new api. In my opinion the private gtk
> layer on the cursor channel can do the same better (allows to create
> the graphical representation).
> > 
> > Advantage is that this could also be useful to other UI frontends
> > (qt/e/android/etc)
> > 
> Maybe... qt uses the signal, with that changed it'd have to listen to
> the notify (signal) and read the property. This problem exists only in
> multimonitor env, currently gtk frontend specific.

Yes, a property is more appropriate. We can keep the signals for compatibility for a (long) while.

> > 
> > > 
> > > Thanks for comments,
> > > Pavel
> > > 
> > > Resolves:
> > >   https://bugzilla.redhat.com/show_bug.cgi?id=1411380
> > > 
> > > git branch cursor_gtk:
> > >   https://github.com/xerus/spice-gtk/tree/cursor_gtk
> > > 
> > > initial approach:
> > >   https://lists.freedesktop.org/archives/spice-devel/2017-January/
> > > 034972.html
> > > 
> > > Pavel Grunt (2):
> > >   Add helper class to handle cursor image
> > >   widget: Set cursor during construction
> > > 
> > >  src/Makefile.am          |   2 +
> > >  src/channel-cursor-gtk.c | 269
> > >  +++++++++++++++++++++++++++++++++++++++++++++++
> > >  src/channel-cursor-gtk.h |  52 +++++++++
> > >  src/spice-widget.c       |  32 +++---
> > >  4 files changed, 340 insertions(+), 15 deletions(-)
> > >  create mode 100644 src/channel-cursor-gtk.c
> > >  create mode 100644 src/channel-cursor-gtk.h
> > > 
> > > --
> > > 2.12.2
> > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
>