GNOME Bugzilla – Bug 781020
GIMP UI vector icons are drawn way too small
Last modified: 2017-12-15 18:56:01 UTC
Created attachment 349437 [details] Patch for GtkIconTheme: correctly identify SVG icons on various platforms Since SVG theme icons were included by default in commit ab1bc49, many of GIMP's UI icons end up extremely small on Windows. This is due to what I believe is a bug in GTK: when determining if an icon file is an SVG, icon_info_ensure_scale_and_pixbuf() in gtkicontheme.c assumes the file's GContentType is always a mime-type, contrary to the GLib documentation. https://developer.gnome.org/gio/stable/gio-GContentType.html#gio-GContentType.description After re-building GTK 2.24.31 with this patch on mingw32, all the icons are pretty again :) I've taken the three major platforms into account, but I may have overlooked something (especially on OSX). Also, I'm not sure if the strcmp() tests should instead be #ifdef-ed for each platform. From glancing at git master, I suspect this issue is moot in GTK 3.
Thanks, that's awesome. I'll make some tests too with your patch and will make the finale review ASAP for inclusion. :-)
We should reassign this to GTK+ once the reviews are done.
My impression is that this should make further use of the gio content type API: Maybe g_content_type_get_mime_type (content_type) and then doing the strcmp, or g_content_type_is_mime_type (content_type, "image/svg+xml") (for glib >= 2.52)
Ok. I finally came around to re-cross-compile GTK+ and GIMP. Took me some time this afternoon. I tested and it indeed fixes the problem! Comments on the code: proper solution would normally to use macros around every test so that each platform would only compile its native test. Now GTK+ can build on other platform than these 3, and leaving boolean OR tests would allow to work on any platform that would use one of these content type logics. And since I doubt these tests would provide any kind of interference (i.e. detecting SVG by mistake if you really do absurd stuff, maybe), I guess it could be fine as-is. The proposition from Michael looks like a good idea but I don't have time to test now. I have to get out. I'll test this later.
Created attachment 349500 [details] [review] Use the actual mime type to determine if an icon is a SVG file. This approach uses g_content_type_get_mime_type to get the actual mime type from the platform-specific content type. It is untested in this context - i.e. I didn't build GTK+ with this change - but I verified that gio.content_type_get_mime_type works as expected in Python.
Schumaml's patch is tested and working as well in a Windows VM. Same as the first patch, it has not been tested on OSX/MacOS. I guess it indeed looks nicer. If the second patch is chosen, could the initial author (Edward E.) be cited in the log message please? Moving now the bug report to GTK+. Would it be possible to have one of these patches (or any updated version) before the next release so that we can use a patched version when we will release GIMP 2.10 please? For this reason, I am upping the priority to Major. The fact is that without this commit, we will have to disable vector icons on GIMP 2.10 and consequently the (basic) HiPPI support we added. Thanks!
Attachment 349500 [details] pushed as bb39a12 - Use the actual mime type to determine if an icon is a SVG file.
We should keep in mind that this may still fail - but if it does, then the place to either add a crazy hack or proper fix will at least be in the gio content type API ;)
Created attachment 363990 [details] [review] (example) fallback .svg check for XP Michael's fix looks like the right way to me, as far as Windows is concerned. However, from the systems that I've looked at, it appears XP forgot to register a Content Type for .svg files, so this is still broken in that case. Someone came to the same conclusion at https://morkrispil.wordpress.com/2012/11/26/svg-support-for-app-engine-running-python-on-windows-xp/ This patch is one way to work around that in GTK+ 2 without breaking anything else. I would have no idea how to deal with this in gio. In case this is actually a good approach, I think it would be nice to get it in before 2.24.32.
Created attachment 363991 [details] Registry script for XP to identify svg type Alternatively, individual users can register the .svg type manually.
Reopening to have a closer look.
I don't have a XP system available, and don't really want to setup a VM to test this. Edward E. has been known to provide very good patches for GIMP on Windows. So if he says that this small tweak fixes the issue on XP, I suggest we just push it. Can I push?
Created attachment 364778 [details] [review] fallback .svg check for XP
I'd rather not add backend-specific ifdefs for a platform we don't support anymore.
I guess that makes sense. GIMP also doesn't have official support of Windows XP anymore, if not mistaken. I still committed the patch inside our build/windows/patches/ directory in GIMP repo so that we can apply it individually in our own builds of GTK+ because a patch has still been contributed and it is harmless to other platforms. So there is no reason to let it rot in the bugtracker. But I guess we don't have to push this in GTK+ which has a valid reason not to. Reclosing as FIXED (on supported platforms). commit 3023227ffe93af64afc90c4116c2f430938fac85 (HEAD -> master, origin/master, origin/HEAD) Author: Jehan <jehan@girinstud.io> Date: Fri Dec 8 21:11:56 2017 +0100 Bug 781020 - GIMP UI vector icons are drawn way too small. Though the bug was mostly fixed, it seems to still happen on Windows XP, where apparently no content type had been registered for SVG. GTK+ developers don't seem too keen to patch GTK+ 2.24 for a platform which they don't support anymore. Also if not mistaken, GIMP does not officially support Windows XP anymore either. A patch though has already been provided by Edward E. and it really doesn't look like it could break anything since it just adds a last "else if" when everything else failed (and inside a #ifdef affecting only WIN32 builds). So let's just add it in our builds at least. We still don't have support for it, but I see no reason to just refuse a minor patch which won't break anything else. build/windows/patches/gtk+-2.24-bug781020.patch | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
Comment on attachment 364778 [details] [review] fallback .svg check for XP The patch has been applied in the Windows build of GIMP 2.9.8.