[Wayland-bugs] [Bug 746141] wayland: Support HiDPI pointer cursors

[gtk+ (GNOME Bugzilla)]

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

--- Comment #10 from Jonas Ådahl <jadahl at gmail.com> ---
(In reply to Matthias Clasen from comment #5)
> Review of attachment 299291 [details] [review]:
> 
> ::: gdk/wayland/gdkdisplay-wayland.c
> @@ +623,3 @@
> +  g_assert (scale >= 1);
> +
> +  theme = wayland_display->scaled_cursor_themes[scale - 2];
> 
> scale - 2 ?!
> That seems to rely on the struct ordering to access the cursor_theme member
> _before_ the array if scale is 1 ?
> 
> Can we just put it all in the array, and use scale - 1 here ?


As you mentioned, the "scaled_cursor_themes" are only for the scaled themes,
i.e. scale 1 is stored separately. I suppose we could just put it together to
avoid the confusion.

> 
> @@ +634,3 @@
> +                     wayland_display->cursor_theme_name, scale);
> +          return NULL;
> +        }
> 
> I guess this is where we would expect the theme to be stuffed in that array ?

Oh, right, how did I miss that.

> 
> ::: gdk/wayland/gdkdisplay-wayland.h
> @@ +44,3 @@
>  
> +#define GDK_WAYLAND_MAX_SCALED_THEMES 5
> +#define GDK_WAYLAND_MAX_THEME_SCALE (GDK_WAYLAND_MAX_SCALED_THEMES + 1)
> 
> 5 seems excessive. In practice, we're dealing with scales of 1 or 2 only.

I suppose so. But then again, maybe accessibility potentially want to make
everything extra large?

> 
> @@ +75,2 @@
>    struct wl_cursor_theme *cursor_theme;
> +  struct wl_cursor_theme
> *scaled_cursor_themes[GDK_WAYLAND_MAX_SCALED_THEMES];
> 
> Nothing ever sets the scaled_cursor_themes, afaics.

Yes, as you pointed out, it should have been set when getting it.

> 
> ::: gdk/wayland/gdkwindow-wayland.c
> @@ -38,3 @@
>  #include <errno.h>
>  
> -#define WL_SURFACE_HAS_BUFFER_SCALE 3
> 
> Moving these 'capability' defines to a single shared place is a good idea.
> Might be nice to do as a cleanup patch beforehand.

Sure.

-- 
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/20150313/c14d62fe/attachment.html>