[Wayland-bugs] [Bug 744932] Wrong (ultra tiny/small) cursor size on HiDPI screen

[mutter (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=744932

--- Comment #116 from Owen Taylor <otaylor at redhat.com> ---
(In reply to Jonas Ådahl from comment #114)
> (In reply to Owen Taylor from comment #111)
> > Review of attachment 309538 [details] [review] [review]:
> > 
> > I have this feeling that it somehow could be simpler (like why does
> > realization have to be triggered throughout the code - couldn't it just be
> > done by the backend when needed?) but as far as I can follow the code it
> > seems like it should work. Some more style comments follow:
> 
> This is because the data backing the cursor may not part of the actual
> cursor (the wl_surface) so if we'd want to realize it just before drawing,
> we wouldn't know how to realize it, since the cursor doesn't know about the
> wl_surface. We could probably add a signal instead that the creator needs to
> set, or a "backing object" so that the renderer can call
> "cursor->backing->realize()" or something. With the current approach the
> renderer specific realization (which is what realize does) is done at
> creation when the source of the data is known.

That's not really what "realize" means in a GTK+/Clutter context - realize
means "create the resources now that everything is set up". It would seem to me
there would be more flexibility from cursor_sprite_set_wl_buffer/xcursor that
stored them, and let the cursor renderer pick up the data and create further
backend resources at need. But anyways, let's get this landed.

> > ::: src/backends/meta-cursor.c
> > @@ +350,3 @@
> >    object_class->finalize = meta_cursor_sprite_finalize;
> > +
> > +  signals[PRE_PAINT_AT] = g_signal_new ("pre-paint-at",
> > 
> > I dislike this naming - the other places where we use "pre-paint", we use it
> > specifically for things done in the clutter PRE_PAINT phase.
> > "position-updated" would be better.
> 
> "position-updated" is not really correct though (I realize the calling
> function name makes it look like it), but it will be signalled even if the
> position didn't change, but the cursor changed. Maybe just "cursor-updated"
> or something.

What about "prepare-at" ?

> > ::: src/wayland/meta-wayland-pointer.c
> > @@ +809,3 @@
> > +  prev_cursor_surface = pointer->cursor_surface;
> > +  if (prev_cursor_surface)
> > +    g_signal_handlers_disconnect_by_data (cursor_tracker,
> > prev_cursor_surface);
> > 
> > Neither cursor_tracker nor prev_cursor_surface is private to this part of
> > the code, so you can't be *sure* that nobody else has made a connection.
> > g_signal_handlers_disconnect_by_func() should always be used unless the data
> > is truly private.
> 
> Well, we can't disconnect by_func because that would make life harder if we
> one day want to support multiple seats.

The naming doesn't make it obvious, but disconnect_by_func takes the function
*and* data.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-bugs/attachments/20150904/8ecd17b6/attachment.html>