[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 #39 from Jonas Ådahl <jadahl at gmail.com> ---
(In reply to Owen Taylor from comment #37)
> (In reply to Jonas Ådahl from comment #34)
> > (In reply to Owen Taylor from comment #30)
> > > Review of attachment 299282 [details] [review] [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

Note that the x11 backend here behaves differently depending on the running
mode which is why I have been suggesting backends/x11/[cm/nested].

> 
>  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.

We have already started separating backends from the rest, so I think in that
case the abstraction can be pretty straight forward (see my comment above).

> 
> 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!
> 

I don't think "winsys" is good term for it either. We'd have different "winsys"
meaning different things depending on the layer (clutter/cogl vs mutter). The
reason MetaCursorSpriteWayland is an awkward fit is because it's a mix of
Wayland client interaction logic and running mode logic. So maybe a better
design would be to split those into different units.

> 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!

That might be because when an X CM, the cat lives in the X server, but in
Wayland we are the 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?

This is why I put it in the Wayland object; it only ever makes sense do do it
there.

>  - 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.

I'd have to think about this more. I agree its not an optimal design as it
blurs the line between "running mode" and the protocol used.

(In reply to Jasper St. Pierre from comment #38)
> The frontend needs a way of specifying "a cursor" (a wl_surface, an X cursor
> name), and the backend needs a way of accepting that and saying "I prefer a
> wl_surface" or "I prefer an X cursor name", because accepting the other
> might be costly.
> 
> There's conceptually two separate objects, one frontend (the protocol object
> the user manipulates) and one backend (the object that is being displayed on
> the screen), but in practice they're not different enough to be two
> different objects. That's why it's sort of in this weird middle-ground right
> now.

I already separated the backend from the frontend completely (all backend
related code is now in MetaCursorRenderer*).

Its rather "meta_is_wayland_compositor ()" or "running mode" or whatever to
call it.

Maybe we should have such a abstraction, not calling it Wayland but "display
server", and then hook wl_surface etc to it some other way and put it under a
"protocols/" directory. Not sure what to call that though; its neither
"winsys", "protocol" or "frontend".

I.e. something like

backends/
  x11/
    cm/
    nested/
protocols/
  wayland/
  x11/
  xwayland/ (funky mix of the two above)
"running mode"/
  x11_cm/
  display_server/

?

MetaCursorSprite would go under "running mode"/ and under protocols we'd have a
MetaWaylandCursor which interacts with MetaCursorSprite.

-- 
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/20150702/ee955566/attachment.html>