! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
libxfcegui4: Cropped icons - GtkIconTheme icon scaling behaviour
Status:
RESOLVED: FIXED
Severity:
trivial
Product:
Libxfcegui4
Component:
General

Comments

Description Stefan Stuhr 2006-02-27 16:34:39 CET
I have a 25 pixel height panel, and the icons in the showdesktop and clipman
plugins and sometimes launchers too are cropped.

The reason seems to be that gtk_icon_theme_load_icon() (which is used in
xfce_themed_icon_load()) not always scale the returned GdkPixbuf. E.g. it
doesn't seem to always be the case with the Tango icon theme, at small icon sizes.

- From
http://developer.gnome.org/doc/API/2.0/gtk/GtkIconTheme.html#gtk-icon-theme-load-icon:
"size : 	 the desired icon size. The resulting icon may not be exactly this
size; see gtk_icon_info_load_icon()."

- If we look at gtk_icon_info_load_icon():
"Renders an icon previously looked up in an icon theme using
gtk_icon_theme_lookup_icon(); the size will be based on the size passed to
gtk_icon_theme_lookup_icon(). Note that the resulting pixbuf may not be exactly
this size; an icon theme may have icons that differ slightly from their nominal
sizes, and in addition GTK+ will avoid scaling icons that it considers
sufficiently close to the requested size or for which the source image would
have to be scaled up too far. (This maintains sharpness.)"

So the problem is, sometimes the GdkPixbuf returned by
gtk_icon_theme_load_icon() is too big, and it will have to be scaled down before
it gets returned by xfce_themed_icon_load().

Reproducible: Always
Steps to Reproduce:
Comment 1 Jasper Huijsmans editbugs 2006-02-27 19:54:02 CET
Created attachment 464 
patch to scale down themed icon if necessary

Brian, this one is for you, I guess. How about this patch?
Comment 2 Brian J. Tarricone (not reading bugmail) 2006-02-28 00:15:13 CET
That's too slow, actually (yes, I've benchmarked).  In the past I've just
assumed the icon was square and w == h.
Comment 3 Jasper Huijsmans editbugs 2006-02-28 21:13:10 CET
(In reply to comment #2)
> That's too slow, actually (yes, I've benchmarked).  In the past I've just
> assumed the icon was square and w == h.

You mean the scaling shows up in the profile, right?

Any ideas about how this might be solved in a more efficient way?
Comment 4 Brian J. Tarricone (not reading bugmail) 2006-02-28 21:24:53 CET
(In reply to comment #3)
> (In reply to comment #2)
> > That's too slow, actually (yes, I've benchmarked).  In the past I've just
> > assumed the icon was square and w == h.
> 
> You mean the scaling shows up in the profile, right?

Yup.  And execution times with and without the aspect ratio check were quite
different.

> Any ideas about how this might be solved in a more efficient way?

Sure,  Just do what XfceIconTheme does: assume the icon is square, and if width
!= size, rescale it to size x size.
Comment 5 Brian J. Tarricone (not reading bugmail) 2006-02-28 21:28:51 CET
Actually, one obvious thing I didn't try doing was this:

if(w != size || h != size) {
    if(w == h) {
        /* resize to (size)x(size) */
    } else {
        /* calculate aspect ratio and resize */
    }
}

That's pretty simple, and gdk_pixbuf_get_width/height() is probably pretty
cheap.  It's really the floating point divs/mults that kills performance here.
Comment 6 Jasper Huijsmans editbugs 2006-03-13 21:18:10 CET
I'd be happy with just scaling down to size x size if necessary. How about the
patch below? We can always try to optimize this if we get complaints.


Index: libxfcegui4/icons.c
===================================================================
--- libxfcegui4/icons.c (revision 20393)
+++ libxfcegui4/icons.c (working copy)
@@ -154,6 +154,15 @@

     g_free(name_fixed);

+    if (pix && (gdk_pixbuf_get_width (pix) > size
+                || gdk_pixbuf_get_height (pix) > size))
+    {
+        GdkPixbuf *scaled =
+            gdk_pixbuf_scale_simple (pix, size, size, GDK_INTERP_BILINEAR);
+        g_object_unref(pix);
+        pix = scaled;
+    }
+
     return pix;
 }
Comment 7 Brian J. Tarricone (not reading bugmail) 2006-03-14 01:33:27 CET
Yeah, that's fine by me.  It doesn't handle non-square icons (obviously), but do
we care?  Maybe if we need it I can add a
xfce_themed_icon_load_maintain_aspect() function.  Though that's probably
overkill.  Go ahead and commit it if you want.
Comment 8 Benedikt Meurer editbugs 2006-03-14 01:36:57 CET
Ahem. We care indeed. Most icon themes (and even more app-specific icon sets)
today contain icons at 50x48, 20x16, etc.

Jasper, just copy the exo_gdk_pixbuf_scale_down() function, which handles the
required scaling properly. It's the one used by Thunar.

http://svn.xfce.org/svn/xfce/libexo/trunk/exo/exo-gdk-pixbuf-extensions.c
Comment 9 Brian J. Tarricone (not reading bugmail) 2006-03-14 01:43:05 CET
Ok, then, fine by me.  As long as the aspect ratio calculation doesn't happen
when width == height.  When loading lots of icons in quick succession (i.e.,
desktop menu), it really does make a difference, at least when they're PNG icons.
Comment 10 Benedikt Meurer editbugs 2006-03-14 01:49:26 CET
You have an amazing harddisk and even more amazing memory chips then, if the
time required for 3 fdiv ops makes a difference here. On common systems -
without these amazing components :-) - the vast majority of time will be spend
loading the image from disk (-> disk I/O) and performing the actual scaling (->
memory I/O).
Comment 11 Brian J. Tarricone (not reading bugmail) 2006-03-14 02:12:31 CET
Hey, I know it sounds weird, but I did some menu generation benchmarks sometime
last year, both with and without the aspect ratio scaling code, and there was a
not-insignificant difference.
Comment 12 Benedikt Meurer editbugs 2006-03-14 02:13:59 CET
Whatever you did then, it was definitely more than 3 fdiv's. ;-)
Comment 13 Jasper Huijsmans editbugs 2006-03-17 19:49:20 CET
Ok, I have committed some code that does scaling with a check for w == h.
Everybody happy? ;-) 

Thanks for the input, guys.

Bug #1516

Reported by:
Stefan Stuhr
Reported on: 2006-02-27
Last modified on: 2009-07-14

People

Assignee:
Xfce Bug Triage
CC List:
0 users

Version

Version:
unspecified

Attachments

Additional information