Re: wip/hires-icons

[Alexander Larsson <alexl redhat com>]

On tor, 2013-05-16 at 18:08 +0200, Carlos Garnacho wrote:

> First of all, I'm aware this partially collides with Alex's work[1], but
> on this branch the backing surface scale details have just been sorted
> out for the Quartz backend, so the meat in this branch can still greatly
> benefit from his work on X11 and Wayland, plus it'd mean plugging less
> holes to get the right icon scale at render time.

I'm aware of this, and in fact i stole the basic scale_factor commits
for my work on wayland hidpi. I think that part is basically right
as-is, although there were some implementation issues:

gdk_window_get_scale_factor() should report the scale factor of the
"closest" native window, not the toplevel. I don't think this is
necessary any practical difference (certainly not for quartz or wayland
which don't do native subwindows), but it seems more right:

diff --git a/gdk/gdkwindow.c b/gdk/gdkwindow.c
index e4f4e51..f321baa 100644
--- a/gdk/gdkwindow.c
+++ b/gdk/gdkwindow.c
@@ -10604,7 +10615,6 @@ gdk_window_get_scale_factor (GdkWindow *window)
   if (GDK_WINDOW_DESTROYED (window))
     return 1.0;
 
-  window = gdk_window_get_toplevel (window);
   impl_class = GDK_WINDOW_IMPL_GET_CLASS (window->impl);
 
   if (impl_class->get_scale_factor)

gtk_widget_get_scale_factor() Should always report the GdkWindow scale
factor if the widget is realized:

diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c
index 123f396..08d86d7 100644
--- a/gtk/gtkwidget.c
+++ b/gtk/gtkwidget.c
@@ -9932,13 +9932,13 @@ gtk_widget_get_scale_factor (GtkWidget *widget)
 
   g_return_val_if_fail (GTK_IS_WIDGET (widget), 1.0);
 
+  if (gtk_widget_get_realized (widget))
+    return gdk_window_get_scale_factor (gtk_widget_get_window
(widget));
+  
   toplevel = gtk_widget_get_toplevel (widget);
   if (toplevel && toplevel != widget)
     return gtk_widget_get_scale_factor (toplevel);
 
-  if (gtk_widget_get_realized (widget))
-    return gdk_window_get_scale_factor (gtk_widget_get_window
(widget));
-
   /* else fall back to something that is more likely to be right than
    * just returning 1.0:
    */

For me, with my wayland HiDPI work I need this in order to create a
surface of the right scale for scrolling. If this is not done then
when the window moves from a display with no scaling to one that has
it looks really bad. Doesn't Quartz need this too?

diff --git a/gtk/gtkpixelcache.c b/gtk/gtkpixelcache.c
index 9b2f1d8..882dbcb 100644
--- a/gtk/gtkpixelcache.c
+++ b/gtk/gtkpixelcache.c
@@ -38,6 +38,7 @@ struct _GtkPixelCache {
   int surface_y;
   int surface_w;
   int surface_h;
+  double surface_scale;
 
   /* may be null if not dirty */
   cairo_region_t *surface_dirty;
@@ -159,7 +160,8 @@ _gtk_pixel_cache_create_surface_if_needed
(GtkPixelCache         *cache,
        cache->surface_w < view_rect->width ||
        cache->surface_w > surface_w + ALLOW_LARGER_SIZE ||
        cache->surface_h < view_rect->height ||
-       cache->surface_h > surface_h + ALLOW_LARGER_SIZE))
+       cache->surface_h > surface_h + ALLOW_LARGER_SIZE ||
+       cache->surface_scale != gdk_window_get_scale_factor (window)))
     {
       cairo_surface_destroy (cache->surface);
       cache->surface = NULL;
@@ -178,6 +180,7 @@ _gtk_pixel_cache_create_surface_if_needed
(GtkPixelCache         *cache,
       cache->surface_y = -canvas_rect->y;
       cache->surface_w = surface_w;
       cache->surface_h = surface_h;
+      cache->surface_scale = gdk_window_get_scale_factor (window);
 
       cache->surface =
        gdk_window_create_similar_surface (window, content,

And in general, we probably need a signal that you can watch for to
get notified of scale factor changes. I propose that the backends send
a spurious configure event in this case, which GtkWindow can catch and
see if the scale has changed. Then it can emit a signal that child
widgets can listen to. Also, obviously a scale change has to invalidate
the whole window for redrawing. We possibly also want to queue a resize,
because the font metrics may have changed, depending on how you're
rendering.

> This branch doesn't precisely reinvent the wheel, there's just a few API
> additions to current components to have this work as seamlessly as
> possible. As choosing an icon must be postponed till rendering time, the
> preferred way to hold this information is GtkIconSet and GtkIconSource,
> as these already do a few things we want here:
>
>   * It may already hold multiple sources for an image
>   * GtkIconSize works as a scale-independent size abstraction
>
> This has implied making GtkIconSets more prominent, so the matching
> properties have been added to GtkEntry and GtkCellRendererPixbuf.
>
> GtkIconTheme has been regarded as a lower level file->pixbuf abstraction
> and mainly a few *_for_scale() calls have been added there so
> GtkIconSets can resolve rendering to the bes3t pixbuf.

Ugh. I hate GtkIconFactory. Mainly due to the GtkIconSize abstraction.
Any time i use it I have to fight that crap, because what you really
*do* want is a (possibly scaled) pixel size. In fact, GtkIconFactory has
been mostly "deprecated" due to this and all modern code use icon names
and GIcons. 

I can see the advantages of having an icon set thing, like you can feed
it two different GdkPixbufs even and have it pick the right one. But,
i'm not at all certain we want to add more use of GtkIconFactory.

In almost all cases, if you use an icon-name and a pixel size (or a
GIcon which references a icon-name) then we should be able to
automatically handle icon sizing (although the branch doesn't seem to do
this in icon helper atm). Maybe we want to add some new form of GIcon
that lets you specify multiple paths for the scaled case. I don't think
using an IconSet is necessarily wrong, but I don't think we should
consider this API as the "target" API for modern code.

> The icon theme parser has been added a extra "OutputScale" per-directory
> property, so there may be directories containing upscaled variants that
> are preferred over "bigger" icons meant for 1x. This way asking for a
> 16x16 icon on a double-density display could first resort to a 16x16 2x
> version over the traditional 32x32 or upscaled 16x16 versions of the
> icon. This means an addendum to the Icon Theme specification though.

I like this, but it has to be discussed on the XDG list first.