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

> 
> Obviously every transformation of frontend to backend isn't going to be
> possible - e.g., creating an X cursor for a Wayland surface - but I don't
> think that's a big problem - you just need to have the code somewhere that
> knows what can be transformed to what.
> 
> ::: src/frontends/wayland/meta-cursor-wayland.c
> @@ +66,3 @@
> +  MetaCursorSpriteWayland *self;
> +
> +  self = META_CURSOR_SPRITE_WAYLAND (meta_cursor_sprite_new ());
> 
> This doesn't make sense to me - this *has* to be a MetaCursorSpriteWayland,
> so just create one.

The idea was to centralize creation of a MetaCursorSprite* but I can remove
that centralization if you prefer.(In reply to Owen Taylor from comment #31)

> Review of attachment 301060 [details] [review]:
> 
> Hmm, this deviates a lot from how I'd expect things to be organized. Use
> your judgement whether you agree with me or not :-) If you want me to review
> things as is without any reconceptualization of MetaCursorSprite , let me
> know, and I'll do that.
> 
> ::: src/backends/meta-cursor-renderer.c
> @@ -103,3 @@
> -      priv->current_rect.y = priv->current_y - hot_y;
> -      priv->current_rect.width = cogl_texture_get_width (COGL_TEXTURE
> (texture));
> -      priv->current_rect.height = cogl_texture_get_height (COGL_TEXTURE
> (texture));
> 
> I'd have expected the rectangle computations to stay here and the
> MetaCursorSprite operation to be more like "here's the desired scale, what
> do you have for me?"

The resulting rectangle is not only used by the renderer, it is also used to
detect what outputs the cursor wl_surface is on (see meta-wayland-surface.c in
a later patch). In other words, since MetaCursorSpriteWayland sometimes can be
a wl_surface, we need to treat it more as such.

> 
> ::: src/frontends/wayland/meta-cursor-wayland.c
> @@ +129,3 @@
> +      image_scale = 1;
> +      if (priv->monitor_scale != monitor->scale)
> +        meta_cursor_sprite_load_from_theme (cursor_sprite, monitor->scale);
> 
> This is certainly not where I'd expect to find this code... in the pure
> "frontend" conceptualization of MetaCursorSprite, a MetaCursorSpriteWayland
> has nothing to do with themed cursors - a wayland cursor is one set by a
> Wayland client.

A MetaCursorSpriteWayland is the MetaCursorSprite implementation when running
as a Wayland compositor. Again, this confusion is probably that we don't have a
"running mode" abstraction, only the "meta_is_wayland_compositor ()" thing
which is not very nice.

-- 
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/20150411/807c1661/attachment.html>