Created attachment 281200 [details] [review]
image: support scale factor when loading from GResource

Currently, when loading an image from a GResource we don't take the
scale factor of the display into consideration, and let GtkIconHelper
scale it accordingly.
While this in general works for non-scalable images, we can take
advantage of the native loader's scaling for e.g. SVG images, and load
them at the right scale factor automatically.
This is achieved by switching to a pixbuf loader instead of using the
native function.

See the blocked g-c-c bug for why I wrote this.

I decided not to touch the code path g-c-c is currently using to load those images (https://git.gnome.org/browse/gtk+/tree/gtk/gtkbuilder.c#n2026) because at the time that property is loaded, there's no knowledge in the builder about how the pixbuf will be used.

Finally, a similar change could be written for gtk_image_new_from_file, but I first want to validate this approach.

Review of attachment 281200 [details] [review]:

::: gtk/gtkimage.c
@@ +943,3 @@
+  image_set = TRUE;
+  scale_factor = gtk_widget_get_scale_factor (GTK_WIDGET (image));
+  _gtk_icon_helper_set_pixbuf_scale (priv->icon_helper, scale_factor);

One thing I wonder about: You set the pixbuf scale to some value here, but not when calling gtk_icon_helper_set_pixbuf in other places, so the scale will stick around.

Maybe it would be better to have a combined

gtk_icon_helper_set_pixbuf_with_scale ?

@@ +954,3 @@
+  g_object_unref (loader);
+
+ out:

I think out: needs to be before g_object_unref (loader), or you'll leak the loader in the error caess.

@@ +958,3 @@
+    gtk_image_set_from_icon_name (image,
+				  "image-missing",
+				  DEFAULT_ICON_SIZE);

I'm increasingly putting things like this just on one line:

 gtk_image_set_from_icon_name (image, "image-missing", DEFAULT_ICON_SIZE);

The general idea looks right to me, anyway.

Created attachment 281450 [details] [review]
image: support scale factor when loading from GResource and file

Currently, when loading an image from a GResource or file we don't take
the scale factor of the display into consideration, and let
GtkIconHelper scale it accordingly.
While this in general works for non-scalable images, we can take
advantage of the native loader's scaling for e.g. SVG images, and load
them at the right scale factor automatically.
This is achieved by switching to a pixbuf loader instead of using the
native function.

Created attachment 281451 [details] [review]
iconhelper: reset original pixbuf scale on clear

Avoids a previously set value for a different image to accidentally
stick around.

Review of attachment 281450 [details] [review]:

::: gtk/gtkimage.c
@@ +830,3 @@
+  format = gdk_pixbuf_loader_get_format (loader);
+  if (!gdk_pixbuf_format_is_scalable (format))
+{

Hmm on second thought I think this won't do the right thing, because we still set the pixbuf scale factor in this case. I will rework later.

Created attachment 281775 [details] [review]
image: support scale factor when loading from GResource and file

--

This should work in all cases.

Review of attachment 281451 [details] [review]:

sure

Review of attachment 281775 [details] [review]:

looks good

Attachment 281451 [details] pushed as 09a36b1 - iconhelper: reset original pixbuf scale on clear
Attachment 281775 [details] pushed as 7e425f3 - image: support scale factor when loading from GResource and file