Merge lp:~3v1n0/libindicator/reduce-image-serialization into lp:libindicator/14.04

[Marco Trevisan (Treviño)]

IndicatorImageHelper: always try to use a GIcon or the filename as source of the GdkImage

We don't need to fallback to pure pixbuf, unless an indicator provided a custom icon that is not in the current theme path.
This helps a lot in reducing the Unity7 workload as this decreases the cases where
we need to encode the pixbuf contents, send them via dbus to unity, encode them
back, reload to a new pixbuf...

Also thanks to this, the library clients can load the actual icon, scaled at the value they want.

Try to avoid image loading to a pixbuf, unless we can't export a GImage or a
filename

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:527
http://jenkins.qa.ubuntu.com/job/libindicator-ci/51/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/4
        deb: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/4/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/libindicator-ci/51/rebuild

review: Approve (continuous-integration)

[Lars Karlitski (larsu)]

Yeah, I'd love to have this as well, but some of the icons aren't square and need different rescaling.

For example, this patch breaks indicator-power's icon. It's much smaller and blurry.

[Marco Trevisan (Treviño) (3v1n0)]

> Yeah, I'd love to have this as well, but some of the icons aren't square and
> need different rescaling.
>
> For example, this patch breaks indicator-power's icon. It's much smaller and
> blurry.

Well, not in unity [with lp:~3v1n0/unity/hidpi-better-scaling]... As we handle this thing at UI level (where it should be done imho), btw I've pushed also a change to the indicator-loader to show how to deal with this case.

I know it's something it would be nice to have at library level, but the gain in handling just image references is too high for us...
We could probably do the same thing I did here at unity-panel-service level (reading the "private" INDICATOR_NAMES_DATA, but I'd prefer to depend on an unstable data element like that).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:528
http://jenkins.qa.ubuntu.com/job/libindicator-ci/52/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-amd64-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/5
        deb: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/5/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/libindicator-ci/52/rebuild

review: Approve (continuous-integration)

[Lars Karlitski (larsu)]

> Well, not in unity [with lp:~3v1n0/unity/hidpi-better-scaling]... As we handle
> this thing at UI level (where it should be done imho), btw I've pushed also a
> change to the indicator-loader to show how to deal with this case.

I totally agree that this is where it should be handled. I didn't know you already fixed it. Nice work!

There's a bigger margin around the menu item in the loader now, but the size of the pixbuf seems to be correct. (Doesn't need to be fixed, I'm just mentioning it.)

I'm fine with landing this after the hidpi unity branch landed. Thanks!

review: Approve

[lp:~3v1n0/libindicator/reduce-image-serialization]

528. By Marco Trevisan (Treviño) on 2014-03-03

IndicatorLoader: Make sure that we load the icon at its original size, if not higher than IMAGE_SIZE

In this way icons such as the battery one won't be shrunk.

[Marco Trevisan (Treviño) (3v1n0)]

So, I've pushed a new revision that handles the things in a different way: basically setting the pixel size to 0 makes gtk to load the icon at its full size; so we can safely do that if the actual icon size isn't bigger than the ICON_SIZE value.

In this way also gtk apps (such as the greeter) won't need any change to support these kinds of icons.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:528
http://jenkins.qa.ubuntu.com/job/libindicator-ci/53/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-amd64-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/6
        deb: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/6/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/libindicator-ci/53/rebuild

review: Approve (continuous-integration)

[lp:~3v1n0/libindicator/reduce-image-serialization]

529. By Marco Trevisan (Treviño) on 2014-03-03

IndicatorImageHelper: let's use the actual icon file if its height is less than ICON_SIZE

[Lars Karlitski (larsu)]

Works fine again after the latest changes. Thanks!

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:529
http://jenkins.qa.ubuntu.com/job/libindicator-ci/54/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-amd64-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/7
        deb: http://jenkins.qa.ubuntu.com/job/libindicator-trusty-armhf-ci/7/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/libindicator-ci/54/rebuild

review: Approve (continuous-integration)

=== modified file 'libindicator/indicator-image-helper.c'
--- libindicator/indicator-image-helper.c	2013-12-20 11:30:44 +0000
+++ libindicator/indicator-image-helper.c	2014-03-03 10:25:52 +0000
@@ -25,6 +25,7 @@
 #include "indicator-image-helper.h"
 
 const gchar * INDICATOR_NAMES_DATA = "indicator-names-data";
+const gint ICON_SIZE = 22;
 
 static void
 refresh_image (GtkImage * image)
@@ -32,21 +33,20 @@
 	g_return_if_fail(GTK_IS_IMAGE(image));
 	const gchar * icon_filename = NULL;
 	GtkIconInfo * icon_info = NULL;
-	gint icon_size = 22;
 
 	GIcon * icon_names = (GIcon *)g_object_get_data(G_OBJECT(image), INDICATOR_NAMES_DATA);
-	g_return_if_fail(icon_names != NULL);
+	g_return_if_fail(G_IS_ICON (icon_names));
 
 	/* Get the default theme */
 	GtkIconTheme * default_theme = gtk_icon_theme_get_default();
 	g_return_if_fail(default_theme != NULL);
 
 	/* Look through the themes for that icon */
-	icon_info = gtk_icon_theme_lookup_by_gicon(default_theme, icon_names, icon_size, 0);
+	icon_info = gtk_icon_theme_lookup_by_gicon(default_theme, icon_names, ICON_SIZE, 0);
 	if (icon_info == NULL) {
 		/* Maybe the icon was just added to the theme, see if a rescan helps */
 		gtk_icon_theme_rescan_if_needed(default_theme);
-		icon_info = gtk_icon_theme_lookup_by_gicon(default_theme, icon_names, icon_size, 0);
+		icon_info = gtk_icon_theme_lookup_by_gicon(default_theme, icon_names, ICON_SIZE, 0);
 	}
 	if (icon_info == NULL) {
 		/* Try using the second item in the names, which should be the original filename supplied */
@@ -63,25 +63,56 @@
 		icon_filename = gtk_icon_info_get_filename(icon_info);
 	}
 
-	if (icon_filename == NULL && !G_IS_BYTES_ICON (icon_names)) {
+	if (icon_filename == NULL && !G_IS_BYTES_ICON(icon_names)) {
 		/* show a broken image if we don't have a filename or image data */
-		gtk_image_set_from_icon_name (image, "image-missing", GTK_ICON_SIZE_LARGE_TOOLBAR);
+		gtk_image_set_from_icon_name(image, "image-missing", GTK_ICON_SIZE_LARGE_TOOLBAR);
 		return;
 	}
 
-	/* Build a pixbuf */
-	GdkPixbuf * pixbuf = NULL;
-
-	if (icon_filename == NULL) {
+	if (icon_info != NULL) {
+		GdkPixbuf *pixbuf = gtk_icon_info_load_icon(icon_info, NULL);
+
+		if (gdk_pixbuf_get_height(pixbuf) < ICON_SIZE) {
+			gtk_image_set_from_file(image, icon_filename);
+		} else {
+			gtk_image_set_from_gicon(image, icon_names, GTK_ICON_SIZE_LARGE_TOOLBAR);
+		}
+		g_object_unref (pixbuf);
+	} else if (icon_filename != NULL) {
+		gtk_image_set_from_file(image, icon_filename);
+
+		gint height;
+		gdk_pixbuf_get_file_info(icon_filename, NULL, &height);
+
+		if (height > ICON_SIZE) {
+			gtk_image_set_pixel_size(image, ICON_SIZE);
+		}
+	} else if (G_IS_LOADABLE_ICON(icon_names)) {
+		/* Build a pixbuf if needed */
+		GdkPixbuf * pixbuf = NULL;
 		GError * error = NULL;
-		GInputStream * stream = g_loadable_icon_load (G_LOADABLE_ICON (icon_names), icon_size, NULL, NULL, &error);
+		GInputStream * stream = g_loadable_icon_load(G_LOADABLE_ICON(icon_names), ICON_SIZE, NULL, NULL, &error);
 
 		if (stream != NULL) {
-			pixbuf = gdk_pixbuf_new_from_stream (stream, NULL, &error);
+			pixbuf = gdk_pixbuf_new_from_stream(stream, NULL, &error);
 			g_input_stream_close (stream, NULL, NULL);
 			g_object_unref (stream);
 
-			if (pixbuf == NULL) {
+			if (pixbuf != NULL) {
+				/* Scale icon if all we get is something too big. */
+				if (gdk_pixbuf_get_height(pixbuf) > ICON_SIZE) {
+					gfloat scale = (gfloat)ICON_SIZE / (gfloat)gdk_pixbuf_get_height(pixbuf);
+					gint width = round(gdk_pixbuf_get_width(pixbuf) * scale);
+
+					GdkPixbuf * scaled = gdk_pixbuf_scale_simple(pixbuf, width, ICON_SIZE, GDK_INTERP_BILINEAR);
+					g_object_unref(G_OBJECT(pixbuf));
+					pixbuf = scaled;
+				}
+
+				/* Put the pixbuf on the image */
+				gtk_image_set_from_pixbuf(image, pixbuf);
+				g_object_unref(G_OBJECT(pixbuf));
+			} else {
 				g_warning ("Unable to load icon from data: %s", error->message);
 				g_error_free (error);
 			}
@@ -89,33 +120,6 @@
 			g_warning ("Unable to load icon from data: %s", error->message);
 			g_error_free (error);
 		}
-	} else {
-		GError * error = NULL;
-
-		pixbuf = gdk_pixbuf_new_from_file (icon_filename, &error);
-
-		if (pixbuf == NULL) {
-			g_warning ("Unable to load icon from file '%s': %s", icon_filename, error->message);
-			g_error_free (error);
-		}
-	}
-
-	if (pixbuf != NULL) {
-		/* Scale icon if all we get is something too big. */
-		if (gdk_pixbuf_get_height(pixbuf) > icon_size) {
-			gfloat scale = (gfloat)icon_size / (gfloat)gdk_pixbuf_get_height(pixbuf);
-			gint width = round(gdk_pixbuf_get_width(pixbuf) * scale);
-
-			GdkPixbuf * scaled = gdk_pixbuf_scale_simple(pixbuf, width, icon_size, GDK_INTERP_BILINEAR);
-			g_object_unref(G_OBJECT(pixbuf));
-			pixbuf = scaled;
-		}
-
-		/* Put the pixbuf on the image */
-		gtk_image_set_from_pixbuf(image, pixbuf);
-		g_object_unref(G_OBJECT(pixbuf));
-	} else {
-		gtk_image_set_from_icon_name (image, "image-missing", GTK_ICON_SIZE_LARGE_TOOLBAR);
 	}
 
 	if (icon_info != NULL) {