Re: wip/hires-icons

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
   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
   /* 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

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.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]