[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 #37 from Owen Taylor <otaylor at redhat.com> ---
(In reply to Jonas Ådahl from comment #34)
> (In reply to Owen Taylor from comment #30)
> > Review of attachment 299282 [details] [review] [review]:
> > 
> > If the point of the refactoring is to get to a clean design, there are
> > elements of this patch that feel off to me
> > 
> > ::: src/frontends/meta-cursor.c
> > @@ +156,2 @@
> >  MetaCursorSprite *
> > +meta_cursor_sprite_new (void)
> > 
> > meta_foo_new() should create a MetaFoo, not a MetaFoo or a subclass. This is
> > something like meta_create_cursor_sprite().
> 
> Then meta_create_cursor_sprite() it is then.
> 
> > 
> > @@ +160,3 @@
> > +    return g_object_new (META_TYPE_CURSOR_SPRITE_WAYLAND, NULL);
> > +  else
> > +    return g_object_new (META_TYPE_CURSOR_SPRITE, NULL);
> > 
> > Hmm, I think this reflects confusion whether MetaCursorSprite is a frontend
> > or a backend object. If it's genuinely a "frontend" object, then when
> > creating it we should always know what protocol we are creating it for. And
> > meta_cursor_sprite_from_theme() would be a third subclass independent of the
> > backend.
> 
> MetaCursorSprite is a kind of a frontend/protocol thing. Some confusion is
> that while we always have one backend, we have multiple frontends/protocols
> but one "running mode" (i.e. Wayland compositor or X11 CM), so really
> MetaCursorSprite is always a MetaCursorSpriteWayland when running as a
> Wayland compositor. I've been thinking of organising things in some way that
> reflects that but haven't come up with any good idea yet.

So:

 Wayland running mode:
  - With *both* wayland and X protocols to talk to clients
  - With *either* native or x11 backend

 X11 compositor:
  - With X protocol to talk to client
  - With x11 backend

Code might be X11 code because it's about a window that we are going to act as
a X11 window manager for (X window or Xwayland window), it might be X11 code
because it's handling X input events (X compositor or nested Wayland), it might
be X11 code because it's specific to when we are running as a X compositor.
This is obviously pretty hard to represent either with a directory hierarchy or
with a class hierarchy.

I think trying to be too strict with how we organize things is going to be
confusing, which could encourage something like a winsys/ directory (I guess
this is the same as our current x11/ and wayland/ directories in the toplevel.)
I'm not sure I believe in a strict "frontend protocol" directory, because it
seems like the first thing that you wanted to put in there doesn't really match
that directory!

That all being said I have some specific concerns about MetaCursorSprite vs.
MetaCursorSpriteWayland.

MetaCursorSprite, as I understand it, is basically similar to 
MetaCursorReference - it's basically a union of different ways to specify the
appearance of a cursor. A wayland-specific subclass of this might make sense so
that it can store along with the appearance a pointer back to a Wayland object.
But a Wayland specific subclass shouldn't gain entirely new capabilities like
looking going out and looking at where it is relative to the monitors and
reloading the theme image at a different size.

I feel like the parent class is fur, and the subclass is a cat!

Things to think about for improving the design:

 - Do some pieces (like loading from the theme at a different size) work better
in the generic code, even if they are only ever used in the Wayland case?
 - Can meta_cursor_sprite_wayland_update_position() be modified to use only
public functionality and then live somewhere else in the code, perhaps as a
procedural method?

As I said before, if you think this is the best way to organize it, I'm willing
to just review it as is - sometimes there's no clean design that actually works
out.

-- 
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/20150701/f484a00a/attachment.html>