GNOME Bugzilla – Bug 759904
Some icons can't be set from the icon theme
Last modified: 2016-09-27 00:30:00 UTC
Created attachment 317929 [details] Various icons missing. There are a bunch of places where GIMP have no icons, in both the color and symbolic themes. It seems it uses the desktop-set icons instead (I made some tests, switching the GNOME default icon set, it indeed changes these icons). See the screenshot. Shouldn't we want to have icons for all these in order to have a consistent GIMP experience? Alexia_Death says on KDE, she doesn't even see the text justification icons, probably because the theme she is using there does not have default for this?
Note: the attached screenshot are just an excerpt of icons I just saw from a first glimpse. Pretty sure there are a lot of places with missing icons in our color and symbolic icon sets.
That is probably the case yes for completely missing icons. I didnt even notice the few system default's among the symbolic ones at first, but yes, several common things are missing.
Most noticeable are Edit and Delete. These are present a lot in the UI.
For info, I tested and saw the text justification icons under a KDE environment and I had the text justification icons. Alexia says she uses oxygen GTK theme for KDE. I'm wondering if this isn't even a separate issue since desktops usually have a fallback theme with all common icons (and I imagine that text justifications are common). Could it be that the search path gets broken in some cases and the icons loses the fallback?
These icons show up because we don't override them. I think the proper way to override desktop theme is adding the replacements as a resource and then use gtk_icon_theme_add_resource_path().
Just started to paint the required ressources as replacements (both symbolic and standard). see bug 759673 At the moment replacements for the text-tool and for the tool-box are available. Hopefully I catch them all, someday ... but the year just started.
Created attachment 318135 [details] icon set for texttool, default & symbolic 1. Sorry for posting falsly in bug 759673 I've completed the text-tool icon set, both for symbolic and default in svg and png two of the icons can be used directly (gimp-letter-spacing and gimp-line-spacing) the other one's (gimp-align-left, -right, -center, -block and gimp-increment-intend) require some code change, because they are hardwired to use the system theme. Symbolic icons are directly derived from libre-office sifr-theme (for what reason ever gimp-symblic and libre-office-sifr use slightly different grey). The colored one's are derived from the symbolic ones. and are quite similar to gnome, adwaita, ubuntu-humanity icon themes, so they should fit there.
Created attachment 318136 [details] gimp-toolbox-icons default & symbolic Yet another icon-set. gimp-delete-settings-tool gimp-save-settings-tool gimp-restore-settings-tool gimp-settings-template (for future additions) gimp-reset is already done and themeable there It is intended to replace the external wired icons for the toolbox. Both svg and png and default and symbolic. Usage requires gimp code change The icons are derived from gimp own icons.
Created attachment 318157 [details] gimp-toolbx-icons default&symbolic corrected gimp-toolbox-icons default & symbolic As I've found a new template for symbolic (it was all the time on my pc it is delivered with inkscape in the icons folder, but I wasn't aware of it's existance at all, but you can get it on the web too https://github.com/gnome-design-team/gnome-icons/blob/master/inkscape-symbolic/inkscape-icons.svg). I've remastered some of the icons to please inkscape users too. gimp-delete-settings-tool gimp-save-settings-tool gimp-restore-settings-tool gimp-settings-template (for future additions) gimp-reset is already done and themeable there It is intended to replace the external wired icons for the toolbox. Both svg and png and default and symbolic. Usage requires gimp code change The icons are derived from gimp own icons and inkscape icons.
Created attachment 318158 [details] Missing icons for window-snapping/presentation preferences I've added two icons (svg only so far) both default&symbolic. the symblic icons are glued together from gimp-symbolic and inkscape symbolic (e.g. the snappping magnet is from inkscape), so inkscape users will feel familiar. Usage needs gimp code change they can be used in settings where 2 icons (gimp-prefs-image-windows & gimp-prefs-tool-options) are in double-usage. gimp-prefs-window-snapping is intended for window-snapping instead of gimp-prefs-tool-options gimp-prefs-window-presentation is intended instead of gimp-prefs-image-windows.
Created attachment 318161 [details] missing gimp-layerbox-icons Some icons for the layerbox (both default and symbolic, svg and png) gimp-layer-delete gimp-layer-new gimp-layer-down gimp-layer-up gimp-layer-newgroup Icons are again a mashup of gimp and inkscape resources, cross-checked against libreoffice sifr
I have now ALL required icons (think so) as symbolic svg's, they are in my experimental folders, yet unpublished. Is there any real interest ? If yes I gonna do the colored ones too (which is more time consuming). My holidays (and my recent illness which resulted in some immobility, even laying in bed and sleeping is more pain than relief) will end quite soon. Afterwards additions will need longer (as real life and real job want their contributes).
Thanks, yes there is real interest :) I'm in holiday mode and didn't get around to looking into how to use them properly (code-wise).
Created attachment 318210 [details] single-svg external replacements for symbolic The svg contains all necessary icons for replacing system icons within gimp for the symbolic icon theme. The color can be changed globally for all by a swatch box, rceent color is 'bebebeff', exactly as the art-libre icons. All icons are 16x16. The icons have the follwing id's id MISCELLANEOUS gimp-add gimp-clean gimp-configure gimp-delete gimp-down gimp-edit gimp-file-open gimp-folder-open gimp-folder-closed gimp-image-open gimp-okay gimp-refresh gimp-remove gimp-restore gimp-revoke gimp-save gimp-shred gimp-up CHANNEL gimp-channel-delete gimp-channel-down gimp-channel-edit gimp-channel-new gimp-channel-up LAYER gimp-layer-delete gimp-layer-down gimp-layer-new gimp-layer-newgroup gimp-layer-up PATH gimp-path-delete gimp-path-down gimp-path-new gimp-path-up TEXT gimp-align-block gimp-align-center gimp-align-left gimp-align-right gimp-increment-intend gimp-letter-spacing gimp-line-spacing TOOL gimp-delete-settings-tool gimp-restore-settings-tool gimp-save-settings-tool FIBONACCI gimp-golden-ratio the golden ratio icon has currently no usage, but who knows ... the shredder icon is modeled after OS/2 2.1 shredder (they had perfectly symbolic icons in those stone-aged gui-days... ;-) I intended it as replacement for cleaning journal & list of used documents. Next step will be to do the same for colored icons.
Created attachment 318230 [details] single-svg external replacements for default As before but this time colored. All in 16x16, with named id's. No swatch-box (makes no sense here), but with an easter-egg :-) As demonstration how powerful vectorgraphics can be when getting scaled, this time in minimal direction.... Aah yes, the shredder is now on warp3 (OS/2 evolved a lot...) id MISCELLANEOUS gimp-add gimp-clean gimp-configure gimp-delete gimp-down gimp-edit gimp-file-open gimp-folder-open gimp-folder-closed gimp-image-open gimp-okay gimp-refresh gimp-remove gimp-restore gimp-revoke gimp-save gimp-shred gimp-up CHANNEL gimp-channel-delete gimp-channel-down gimp-channel-edit gimp-channel-new gimp-channel-up LAYER gimp-layer-delete gimp-layer-down gimp-layer-new gimp-layer-newgroup gimp-layer-up PATH gimp-path-delete gimp-path-down gimp-path-new gimp-path-up TEXT gimp-align-block gimp-align-center gimp-align-left gimp-align-right gimp-increment-intend gimp-letter-spacing gimp-line-spacing TOOL gimp-delete-settings-tool gimp-restore-settings-tool gimp-save-settings-tool
Created attachment 318270 [details] Updated single-svg coloered&symbolic for external icons in gimp I'vbe updated both sheets, cause I was unsatiesfied.... In symbolic only the gimp-clean icon was replaced by a better one. In colored, most icons have changed partly (the ideas itself remained), so they harmonize now better with each other (and hopefully gimp default). Creating icons for two controverse icon-themes at the same time is not the best idea, it produced some confusion. BTW. changed the color parts in the according text to pantone 2016 spring-colors (rose-quartz and serenity) so everybody ;-) knows when the icons are created.
Created attachment 318825 [details] symbolic 'complete' colored unchanged Seems like a hydra, whenever I paint an icon two new ones pop-up ... Hopefully I have now all for symbolic theme (+ some more). As the colored ones need more time and I need a break until next week ... the missing ones will be done later. Please verify if all are catched if, not call for missing ones Currently we have in symbolical these icons. id MISCELLANEOUS gimp-add gimp-clean gimp-configure gimp-delete gimp-down gimp-edit gimp-file-open gimp-folder-open gimp-folder-closed gimp-image-open gimp-okay gimp-refresh gimp-remove gimp-restore gimp-revoke gimp-save gimp-shred gimp-up gimp-image-property gimp-set-scale gimp-document-index gimp-curt gimp-search gimp-left gimp-right gimp-bottom gimp-information CHANNEL gimp-channel-delete gimp-channel-down gimp-channel-edit gimp-channel-new gimp-channel-up LAYER gimp-layer-delete gimp-layer-down gimp-layer-new gimp-layer-newgroup gimp-layer-up PATH gimp-path-delete gimp-path-down gimp-path-new gimp-path-up TEXT gimp-align-block gimp-align-center gimp-align-left gimp-align-right gimp-increment-intend gimp-letter-spacing gimp-line-spacing TOOL gimp-delete-settings-tool gimp-restore-settings-tool gimp-save-settings-tool COLOR gimp-cmyk gimp-color-palette gimp-watercolor (currently doubles gimp-tool-paintbrush) TEMPLATES gimp-template-a3 (currently doubles gimp-template) gimp-template-a4 (") gimp-template-a5 (") gimp-template-a6 (") gimp-template-b4 (") gimp-template-us (") gimp-template-cd-label gimp-template-floppy-label FIBONACCI gimp-golden-ratio
oops a typo 'gimp-curt' gimp-cut is correct. In the svg itself all is correct
Created attachment 319585 [details] complete color&symbolic substitute icons we now have icons with the following id's in both symbolic and color. Question is now: What to happen with these ? How do they get used by gimp ? If an external used icon is missing please tell. id MISCELLANEOUS gimp-add gimp-clean gimp-configure gimp-delete gimp-down gimp-edit gimp-file-open gimp-folder-open gimp-image-open gimp-okay gimp-fonts gimp-refresh gimp-remove gimp-restore gimp-revoke gimp-save gimp-shred gimp-up gimp-image-property gimp-set-scale gimp-document-index gimp-cut gimp-search gimp-left gimp-right gimp-bottom gimp-information gimp-new-unit gimp-script gimp-image-fit-to-window gimp-window-fit-to-image gimp-window-fit-to-size gimp-one-to-one CHANNEL gimp-channel-delete gimp-channel-down gimp-channel-edit gimp-channel-new gimp-channel-up LAYER gimp-layer-delete gimp-layer-down gimp-layer-new gimp-layer-newgroup gimp-layer-up PATH gimp-path-delete gimp-path-down gimp-path-new gimp-path-up TEXT gimp-align-block gimp-align-center gimp-align-left gimp-align-right gimp-increment-intend gimp-letter-spacing gimp-line-spacing TOOL gimp-delete-settings-tool gimp-restore-settings-tool gimp-save-settings-tool COLOR gimp-cmyk gimp-color-palette gimp-watercolor (currently doubles gimp-tool-paintbrush) TEMPLATES gimp-template-a3 gimp-template-a4 gimp-template-a5 gimp-template-a6 gimp-template-b4 gimp-template-us gimp-template-cd-label gimp-template-floppy-label
It's difficult to tell if you've missed any of the GTK stock icons as you haven't provided a mapping between the existing GTK ids and your new GIMP id versions.
(In reply to Kevin Payne from comment #20) > It's difficult to tell if you've missed any of the GTK stock icons as you > haven't provided a mapping between the existing GTK ids and your new GIMP id > versions. As nobody gave me any advice or any help to find out which GTK stock icons are used by GIMP I've painted replacement icons for what I saw inside GIMP. Maybe thats the wrong way to do such, by thats the way I did it (and if I had known how much GTK icons are used in GIMP I would never started to do so, I have enough to do I don't need anything to fill my time). No I won't check the 6.626 icons in gnome, or the 7287 icons inside humanity, to find out the mappings. Either someone tells me which id's are used in GIMP so I can map them based on that list, or things are as they are.
Here's all the ids I've found by hours of looking through the GIMP source: gtk-about gtk-add gtk-clear gtk-close gtk-convert gtk-copy gtk-cut gtk-delete gtk-directory gtk-edit gtk-execute gtk-help gtk-info gtk-go-down gtk-go-up gtk-goto-bottom gtk-goto-top gtk-indent gtk-justify-center gtk-justify-fill gtk-justify-left gtk-justify-right gtk-new gtk-new gtk-open gtk-open-recent gtk-page-setup gtk-paste gtk-preferences gtk-print gtk-quit gtk-redo gtk-refresh gtk-remove gtk-revert-to-saved gtk-save gtk-save-as gtk-select-all gtk-select-color gtk-select-font gtk-undo gtk-zoom-100 gtk-zoom-fit gtk-zoom-in gtk-zoom-out
Klaus > No problem for not getting the exact GTK+ IDs. You already did a great job! Kevin > Thanks for getting some mapping to GTK+ IDs too. It may be useful. :-) In any case, from what Mitch said on IRC, we would not be able to just name the icons the same as the GTK+ common themes and see them in GIMP. So even naming them as in GTK+ won't help much to detect visually what's missing. Our icon themes are used as "fallbacks" of desktop themes. It means that if your desktop had an icon theme with gimp-* icons, these would be used in priority. And oppositely if we name our icons with common gtk-* names, they would be used only if the desktop themes didn't have these. Hopefully I would prefer the opposite obviously: using our icons first would allow us to use common GTK+ names and the desktop icons can therefore be used as fallback only. Anyway we'll just have to wait for Mitch input now. He seemed to have an idea already, so I won't chime in anymore on this.
Klaus > I wasn't making a criticism, I had just assumed that you'd found which ones to make the same way I did when making my themes, so you would already have the list available. Jehan > That sounds like a mess. It seems strange that you can replace the GTK icons using the "standard" GTK theme mechanism, but that the icon theme doesn't work sensibly.
Well as far as I understood, what GIMP does *is* the "standard" GTK+ way. If you know better, I suggest you speak with Mitch on IRC.
I'm currently working on this bug (have a look at the icons-wip branch). Unfortunately I don't know how to replace the gtk-justify icons used in 'gimp/app/tools/gimptextoptions.c: box = gimp_prop_enum_icon_box_new (config, "justify", "gtk-justify", 0, 0); by gimp-justify-center gimp-justify-block gimp-justify-left gimp-justify-right
I should really look into this.
Mitch > Klaus already does a very good job there for a whole bunch of the missing icons, in the branch origin/icons-wip, finding where they are set. There are still bugs here and there (which is also why we made a branch, in order not to break master), but I'm fixing them regularly. And I actually just merged the current state of the branch into master.
The recent commits on this bug in git are wrong, we must not replace the standard icon names by gimp icon names just because we didn't get around yet to fix this properly. Those commits should be reverted.
Mitch > Oh ok. All of the commits? Since I am not sure what is the proper fix, I'm not sure if you mean all of the commits or if some of them are still ok.
Also maybe we should not revert the full commits, only the code parts to not delete the icons.
Yes, and please don't revert anything just yet. And when we do let's keep the icons. It's easier to rename them in git to the standard names than to add-remove-add-foo them... I've been too deep in color management to look at much else for too long now :/
Ok. I just won't touch anything for now. Klaus > just in case you did not get it all, basically you can continue to do the missing icons, but don't bother with code anymore. Mitch seems to have a much better way to deal with them (maybe even without code change?). But when it happens, we will have all the icons just waiting to get in! :-)
The resent changes has broken the build: Making all in Color make[3]: Entering directory '/home/somnium/git/gimp/icons/Color' make[3]: *** No rule to make target '22/gimp-indent.svg', needed by 'all-am'. Stop. make[3]: Leaving directory '/home/somnium/git/gimp/icons/Color' make[2]: Leaving directory '/home/somnium/git/gimp/icons' make[2]: *** [Makefile:546: all-recursive] Error 1 make[1]: Leaving directory '/home/somnium/git/gimp' make[1]: *** [Makefile:716: all-recursive] Error 1 make: *** [Makefile:616: all] Error 2
Argh sorry, my bad! I build only with --enable-vector-icons lately. I should not have merged without testing both vector and bitmap icons. I'm fixing the bugs.
Fixed.
Unfortunately the list of 'alien' icons in use by gimp is rather long (even after my patches ...) Currently (IMHO) these icons are missing and need to be added/replaced "document-open" "document-new" "window-close" "edit-undo" "edit-redo" "application-exit" dialog-warning "gtk-edit" "gtk-directory" "edit-delete" "edit-clear" "edit-cut" "go-up" "go-top" "go-down" "go-bottom" "edit-paste" "edit-paste" "zoom-in" "zoom-out" "zoom-fit-best" "view-refresh" "tool-options" "list-add" "clear-button" "preferences-system" "window-new" "Zoom to _Selection" "gtk-no" "gtk-yes" Even more unfortunately ... (or better this is a great chance) some of them need to be replaced by two different icons (or already existent ones) a 1:1 change is better than nothing, but not in all cases the best way ... When all these icons are in place as 16px, some (better many) of them need 24px twins. (One of the great fortunes of vector icons, you have to paint them for every resolution to be pixel-perfect, this is a great progress compared to bitmap icons ;-) So still a lot of work to do. Thanks to Jehan for mergin icons-wip into master (and fixing my typo's and & other peculiarities) this is of great help (at least for me) because getting a build by https://launchpad.net/~otto-kesselgulasch/+archive/ubuntu/gimp-edge with all icons in it's 'natural habitat' gives me a better visual impression (and a better impression what needs to be corrected) than all those icons out of place. As I'm away for 10 days expect no icon adds in this time
> One of the great fortunes of vector icons, you have to paint them for every resolution to be pixel-perfect, this is a great progress compared to bitmap icons ;-) Really unless you really want to do this, you should not bother. As I think I told you in a previous email, Jimmac (one of historical GIMP and GNOME icon designers) was telling us that even in GNOME, they don't bother anymore. Do the pixel perfect business at the "default" size that we decide the icon would be. And let's accept non-perfect icons for users who go for particular sizes. Of course you will always find someone to complain that the icon is blurry at some weird non-default size, but this is a compromise we have to make. We definitely prefer to have as many icons as possible, pixel-perfect for the chosen default size, rather than fewer icons at every possible sizes.
As nothing other happened in the meanwhile ... I continued with my patches and paintings. All can be found here https://git.gnome.org/browse/gimp/log/?h=icons-wip as soon (after some review) this tree is merged by the devs with master, this bug can be closed.
Hi, Actually Mitch thinks he knows how to better include your new icons while still using GTK+ icon names (cf. comment 29), which will be a much cleaner implementation. This is why I can't merge the branch right now. But your work on icons will definitely be used, no worry there. :-) I will just wait for Mitch magic on our code first, then we will make so all the new icons in icons-wip are used. Just hang in there and let's get the whip for Mitch to work faster! ;-) Thanks for the awesome work.
Please keep in mind that some of the icons aren't 1:1 replacements. E.g for layer, channels and paths I've added additionally (different) icons. Also e.g. gimp-detach is and was never a gtk-icon it was only replaced by gtk-convert (in the gimpicons.h missing list)because nobody painted gimp-detach before, so I only needed to remove it from the missing list.
Created attachment 333000 [details] program to test laoding custom icons using gtk builtin icon names Here the trick is that when changing the theme, it is necessary to set the default GtkSettings "icon-theme-name" property to the theme "name". Setting that value forces Gtk to prefer existing built-in icons in the "name" subdir of the icon_theme_search_path. I attached a simple main program to test the theory. Save it in an empty directory. To rename the 2 used icons like the Gtk builtins, create a subdir tree like this (adjusting PREFIX as necessary): PREFIX=$HOME/prefix mkdir -p icons/Gimp{Legacy,Color,Symbolic}/{16x16,22x22}/apps icons/hicolor for i in {Color,Legacy,Symbolic} ; do cp -v $PREFIX/share/gimp/2.0/icons/$i/hicolor/index.theme icons/Gimp$i/ cp -v $PREFIX/share/gimp/2.0/icons/$i/hicolor/16x16/apps/gimp-open.png icons/Gimp$i/16x16/apps/document-open.png cp -v $PREFIX/share/gimp/2.0/icons/$i/hicolor/22x22/apps/gimp-justify-center.png icons/Gimp$i/22x22/apps/gtk-justify-center.png done Compile and run it. Choosing GimpLegacy uses fallback builtin icons, whereas GimpColor and GimpSymbolic use their version. Adding to icons/GimpLegacy/index.theme a line like: Inherits=GimpColor after [Icon Theme] GimpLegacy uses GimpColor version. Notes: gtk_icon_theme_add_resource_path is: "Since: 3.14"
Created attachment 333049 [details] [review] quick hack to install icons not to be overridden by system icons This is a first attempt to install icons as my understanding of icon themes suggests. It includes code changes that have been necessary to suppress warnings in my little testing (I removed a sanity check for the presence of hicolor dir). The test I made is to rename a copy of the installed "gimp-group-layer.png" to "folder.png" in the same directory and observe that the new icon is used in the layer dialog when the image has a layer group and changing icon theme in preferences changes the icon used and with Legacy icon theme the system "folder" icon is used.
That's cool. I don't have time right now, but I'll try and review this next week during GUADEC. :-)
Created attachment 333110 [details] [review] second attempt to install icons not to be overridden by system icons The problem is simple: icons put in the hicolor subdir of a icon theme search path are considered fallback icons. Gimp is prepending the icon theme search path (to the system one) so icons found in a system icon theme subdir (!= hicolor) are choosen in preference to equally named present in GIMP hicolor (fall back) subdir. Attached a second attempt that let the user use a third party icon theme installed for example in ~/.config/GIMP/2.9/icons
Created attachment 333176 [details] [review] Third attempt to install icons not to be overridden by system icons Fixed a typo in the attached patch. I downloaded a color, bigger icon, theme file from: https://bitbucket.org/paynekj/paynekj-gimp-scripts/raw/2e8e87faf5eaaf6036e0d8b68ff9f6f37a3f0421/themes/Color-48.7z to see if the patch attached here breaks anything in that setup. I observed that Gtk gtkrc parser is brittle. That theme is missing all images that it expects to be in the 12/ subdir (plus few other images in other subdirs). On the console a warning is printed 'Unable to locate image file in pixmap_path: "12/gimp-close.png' when choosing the theme in preferences, but if you make it the default, the next time you start gimp-2.9 it crashes without warnings. Its gimp-imagerc.rc file has also few image names missing the extension which also cause crashes. Its gtkrc file sets gtk_color_scheme. This is incompatible with a system gtkrc that also sets it, Gtk gtkrc parser connects a callback to reparse all gtkrc files when GtkSettings 'color-hash' property is changed and these two assignments in the 2 gtkrc (system theme, custom theme) change that property leading directly to a crash or starting an apparently infinite loop on gimp-2.9 start. That theme has to set GtkMenuItem::horizontal-padding or it conflicts with the mint gtkrc (menubar shows only "File"). Fixing these incompatibilities, a custom and its fallback icon theme are used as expected also with this custom theme.
(In reply to Massimo from comment #46) > I downloaded a color, bigger icon, theme file from: > > https://bitbucket.org/paynekj/paynekj-gimp-scripts/raw/ > 2e8e87faf5eaaf6036e0d8b68ff9f6f37a3f0421/themes/Color-48.7z > > to see if the patch attached here breaks anything in > that setup. > Interesting comments, but I'm not sure how valid a test it is given that I created that theme as a stop-gap for Gimp 2.8 to be used on high dpi screens.
(In reply to Kevin Payne from comment #47) > (In reply to Massimo from comment #46) > > I downloaded a color, bigger icon, theme file from: > > > > https://bitbucket.org/paynekj/paynekj-gimp-scripts/raw/ > > 2e8e87faf5eaaf6036e0d8b68ff9f6f37a3f0421/themes/Color-48.7z > > > > to see if the patch attached here breaks anything in > > that setup. > > > > Interesting comments, but I'm not sure how valid a test it is given that I > created that theme as a stop-gap for Gimp 2.8 to be used on high dpi screens. Yes, I was looking to test an icon theme that would be surely used, possibly installed in a user defined folder. I downloaded your, which is a theme, and since it crashed at the first restart I wanted to be sure it was not something related to the patch.
Created attachment 333206 [details] [review] Fourth attempt to install icons not to be overridden by system icons Forgot to commit hicolor/index.theme This time I've also run make distcheck and tested a first run after installing.
How does this new patch behave when the system theme changes? Do icons not in the GIMP icon set change and all others stay the same?
(In reply to Michael Natterer from comment #50) > How does this new patch behave when the system theme changes? Do icons > not in the GIMP icon set change and all others stay the same? No, nothing changes. The GtkSettings object notifies that the "gtk-icon-theme-name" property has possibly changed, but getting its value returns the value I set at start.
(In reply to Michael Natterer from comment #50) > How does this new patch behave when the system theme changes? Do icons > not in the GIMP icon set change and all others stay the same? I've been able to get that behaviour only with a hack like this: >diff --git a/libgimpwidgets/gimpicons.c b/libgimpwidgets/gimpicons.c >index 91b8572..6b3d064 100644 >--- a/libgimpwidgets/gimpicons.c >+++ b/libgimpwidgets/gimpicons.c >@@ -539,6 +539,43 @@ gimp_stock_init (void) > gimp_icons_init (); > } > >+static void >+notify_me (GObject *settings); >+ >+static void >+reset_signal_handler (GObject *settings) >+{ >+ g_signal_handlers_disconnect_by_func (settings, G_CALLBACK (reset_signal_handler), NULL); >+ g_signal_connect (settings, "notify::gtk-icon-theme-name", G_CALLBACK (notify_me), NULL); >+} >+ >+static void >+notify_me (GObject *settings) >+{ >+ GdkScreen *screen = gdk_screen_get_default (); >+ GValue value = G_VALUE_INIT; >+ >+ g_value_init (&value, G_TYPE_STRING); >+ >+ if (gdk_screen_get_setting (screen, "gtk-icon-theme-name", &value)) >+ { >+ g_object_set (settings, >+ "gtk-fallback-icon-theme", g_value_get_string (&value), >+ NULL); >+ g_value_reset (&value); >+ >+ g_signal_handlers_disconnect_by_func (settings, G_CALLBACK (notify_me), NULL); >+ g_signal_connect (settings, "notify::gtk-icon-theme-name", G_CALLBACK (reset_signal_handler), NULL); >+ >+ g_object_notify (settings, "gtk-icon-theme-name"); >+ } >+ else >+ { >+ g_value_reset (&value); >+ } >+} >+ >+ > /** > * gimp_icons_init: > * >@@ -631,6 +668,7 @@ gimp_icons_init (void) > settings = gtk_settings_get_for_screen (gdk_screen_get_default ()); > > g_object_get (settings, "gtk-icon-theme-name", &system_icon_theme, NULL); >+ > g_object_set (settings, > "gtk-fallback-icon-theme", system_icon_theme, > "gtk-icon-theme-name", gimp_icon_theme, >@@ -639,6 +677,7 @@ gimp_icons_init (void) > g_free (gimp_icon_theme); > g_free (system_icon_theme); > >+ g_signal_connect (settings, "notify::gtk-icon-theme-name", G_CALLBACK (notify_me), NULL); > pixbuf = gdk_pixbuf_new_from_resource ("/org/gimp/icons/64/gimp-wilber-eek.png", > &error); >
Created attachment 333310 [details] [review] Fifth attempt to install icons not to be overridden by system icons well, ignore my previous comment (to avoid infinite recursion is enough to emit the notify signal when the system theme effectively changed) So the attached patch reacts to a system icon theme change updating only the icons that are not included in the GIMP specific icon theme.
*** Bug 770463 has been marked as a duplicate of this bug. ***
Hello Massimo, I tested your patch. It just works. I tested adding some of the standard icon names from the freedesktop specification and also adding some of the GTK+ icons in our icon sets, both of which used to not work (because GIMP would grab the system icons first) now works! Also I confirm that icon them change works well. I have only skimmed through your patch because you say it is a "quick hack", so I can't say in details, but this is already a great "quick hack", as usual coming from you! :-) I'm quite happy to see that this ugly hack of calling each and every icon theme "hicolor" is gone. That was really not what hicolor is supposed to be in the icon theme specification. I see though that you add a hicolor theme locale to GIMP. What are you expecting to be found in there? Finally since your commit messages says it is a quick hack, should I wait for a new version before starting a more thorough review or can I do it with this patch?
(In reply to Jehan from comment #55) > Hello Massimo, > > I tested your patch. It just works. I tested adding some of the standard > icon names from the freedesktop specification and also adding some of the > GTK+ icons in our icon sets, both of which used to not work (because GIMP > would grab the system icons first) now works! > > Also I confirm that icon them change works well. > > I have only skimmed through your patch because you say it is a "quick hack", > so I can't say in details, but this is already a great "quick hack", as > usual coming from you! :-) > > I'm quite happy to see that this ugly hack of calling each and every icon > theme "hicolor" is gone. That was really not what hicolor is supposed to be > in the icon theme specification. > > I see though that you add a hicolor theme locale to GIMP. What are you > expecting to be found in there? > It is there to avoid a warning from gtk, when you set a icon theme search path, IIRC, a sanity check is done and at least a hicolor dir is expected to be found there. In this patch it includes a index.theme file with an empty 'Directories' field. The real fallback theme is in a system icon theme path. > Finally since your commit messages says it is a quick hack, should I wait > for a new version before starting a more thorough review or can I do it with > this patch? It is a quick hack because it must be followed by undoing much of the work done here. Icons added here and those present in the icons-wip branch must be renamed according to the standard spec where possible so they're used also in gtk dialogs (File open chooser for example uses document-open-recent not gimp-document-recent, gtk-cancel not gimp-cancel etc) BTW the patch attached here has 2 memory leaks, I'll attach later a patch without them. OTOH gtk3 ignores the "gtk-fallback-icon-theme" GtkSettings so it is something to be revisited for gimp-3
Created attachment 334279 [details] [review] install icons not to be overridden by system icons Same patch with 2 memory leaks fixed
Review of attachment 334279 [details] [review]: 1) In gimp_icons_sanity_check(): > GFile *index = g_file_get_child (child, "index.theme"); You forgot to unref() index. 2) In gimp_icons_set_icon_theme(), when (! g_file_equal (icon_theme_path, path)), you set icon_theme_path to path. But just above, you dealt with the case that path may be NULL. In such a case, shouldn't you compare and set to `gimp_data_directory_file ("icons", GIMP_DEFAULT_ICON_THEME, NULL)` instead? And same a few lines below. Unless path can never be NULL, but in this case, the case above should be changed. 3) icon_theme_path is a path to the icon theme, whereas default_icon_theme_path is now a search path where the default icon theme can be found. So first of all, I would suggest to rename default_icon_theme_path into for instance default_search_path so that the 2 concepts are not confused. And I think these 2 are confused at least once: in gimp_icons_change_icon_theme(), when you compare `if (! g_file_equal (path, icon_theme_path))`, `path` here is a search path (parent folder of the icon theme we are changing to), whereas icon_theme_path is a finale theme path. So obviously they should likely always be different and this comparison is made useless. I think you wanted to compare path to the `parent` of icon_theme_path. 4) In gimp_icons_notify_system_icon_theme(), why do you g_object_notify (settings, "gtk-icon-theme-name") ? This is already how the function was called and this property has not been changed in the callback. The only reason the callback does not loop indefinitely is because the second call does not notify again thanks to the equality condition. Any reason for this g_object_notify()? Other than this, looks pretty cool to me! To conclude, Mitch was saying on IRC that he was very annoyed that gtk-fallback-icon-theme does not work anymore in GTK+3 because it means your code will break when we start working on GIMP 3. I don't think this will be a blocker because I think your patch is worth pushing, even though we'll have to search for an alternative later. Yet to ease a little Mitch's mind, would you have already an idea on how this could be made to work in GIMP 3? Anyway I'll see if he is OK for you to push this once you have fixed the few details I listed above.
Sorry for the double comment. Also forget the point 4). I tested to remove the notify() and the fallback to the system theme was not working (or rather one step late: if you change the system theme twice, the first change is applied on the second change, and so on). I don't really understand why, but obviously you do.
Created attachment 334461 [details] [review] install icons not to be overridden by system icons 1) 2) 3) You're right, thanks, fixed and renamed to hopefully clarify the code. BTW, the 'default_search_path' loses part of its role because in this scheme all GIMP icon themes have the same search path. To make the 'Legacy' icon theme use missing icons from the 'Color' icon theme it is necessary to add the 'Inherits=Color' field to Legacy/index.theme (done this as well and I've verified looking at the symmetry dockable tab icon). 'default_search_path' is still relevant to find user installed icon themes inheriting from Gimp ones (I've updated comments). Re: Gtk3 gtk3 ignores gtk-fallback-icon-theme, but the whole point of this approach is to depend as little as possible on fallback icons. That is, using the same names gtk uses makes it possible to theme/style icons used in ui elements not directly coded in GIMP. gtk3 has 3 fallback icon themes hardcoded (gnome, Adwaita and hicolor), plus it is possible to add the Inherits field to GIMP index.theme files and using standard names allows to use builtin/resource icons in gtk library, so this solution should anyway work. The only problem should be it does not use the currently system icon theme as the first fallback (I did not test though). Anyway Gtk3 introduced also new things like a flag to force loading symbolic icons, perhaps making it simpler to have paired themes and icon themes
Awesome. I have not rechecked the new patch but I'm sure it's fine. I would give this all a go (and we'll so for GIMP 3 when we will be there), and Mitch agrees but he just wants to have a last look tonight. So let's wait for his green flag. As soon as it is in, I will start merging/modifying Klaus work to get his new icons with GTK+ or standard icon names. :-)
If there are no objections I'm going to commit the patch attached to comment 61 tomorrow.
We should also revert the patches that changed icon names away from the default ones which your patch now makes work.
(In reply to Michael Natterer from comment #64) > We should also revert the patches that changed icon names away from > the default ones which your patch now makes work. As said in comment 62, I will take care of these commits, as well as all the other icons in a feature branch.
I committed the patch: commit 3cc77b03bfc99c15183e867734fb30ea1cecc3d0 Author: Massimo Valentini <mvalentini@src.gnome.org> Date: Fri Sep 2 12:30:49 2016 +0200 Bug 759904: Some icons can't be set from the icon theme make it possible to use themed standard named icons so to have also gtk dialogs show icons in the same style as GIMP ui. (In reply to Michael Natterer from comment #64) > We should also revert the patches that changed icon names away from > the default ones which your patch now makes work. Revert only the rename/registration as stock items, because these patches added themed icons which are going to be used.
I started working on it. This commit hopefully reverts all code parts which were changed (unless when they added new dedicated — i.e. more accurate — icons): commit 6c674e973c0080c41dcd930f8eff559fc62e8ac8 Author: Jehan <jehan@girinstud.io> Date: Sat Sep 3 15:28:22 2016 +0200 app, libgimp*, plug-ins, icons: revert icon names into freedesktop... ... standard icon names and GTK+ icon names as second choice. We should only use GIMP specific icon names as last resort, when there is no standard or GTK+ names dedicated to the function. This is made possible thanks to commit 3cc77b0. s/gimp-document-recent/document-open-recent/ s/gimp-indent/format-indent-more/ s/gimp-next/go-next/ s/gimp-previous/go-previous/ s/gimp-save/document-save/ s/gimp-save-as/document-save-as/ s/gimp-revert/document-revert/ s/gimp-open/document-open/ s/gimp-document-recent/document-open-recent/ s/gimp-quit/window-close/ ou s/gimp-quit/application-exit/ s/gimp-warning/dialog-warning/ s/gimp-edit-clear/edit-clear/ s/gimp-justify-.*/gtk-justify-.*/ s/gimp-font/gtk-select-font/ s/gimp-color-palette/gtk-select-color/ s/gimp-cancel/gtk-cancel/ -------------- That's still WIP and in particular I'll look into the icons-wip branch.
I squashed Klaus' work from the icons-wip branch into 2 commits so far: a first for every modified icons, and a second one adding icons for standard Freedesktop icons or generic GTK+ icons. No code change integrated. It is not finished since the branch also had a lot more icons which were not standards, mostly splitting existing icons in several versions. I'll slowly review these to see if they are really needed. I may create some new bug reports to submit these new icons for review before actually committing. For now, I will focus on any missing icons from Klaus' work.
commit 8bff597c6f3b5e833562f94cba80d2300a8bdaab Author: Jehan <jehan@girinstud.io> Date: Sun Sep 4 04:44:05 2016 +0200 icons: install the new Freedesktop/GTK+ icons. commit 0eeee33c70aa27a25f954228440902639bc036dd Author: Klaus Staedtler <staedtler-przyborski@web.de> Date: Sun Sep 4 04:11:38 2016 +0200 icons: many new icons. These icons were massively renamed by Jehan, from a feature branch, in order to fit either into Freedesktop's "Icon Naming Specification", or as standard GTK+ icons. commit a440f6cbff1e1351d380354a064990f091e26865 Author: Klaus Staedtler <staedtler-przyborski@web.de> Date: Sun Sep 4 04:15:22 2016 +0200 icons: updated icons. As squashed by Jehan from the work of Klaus Staedtler in a feature branch.
Thanks for the updates GIMP looks much better with the new icons, still a few missing, just laying out there as FYI. (i only verified this with symbolic so far) Menus: file -> print file -> send by email file -> copy image location edit -> copy Layer -> delete layer help -> help help -> context help Docks: in every docks -> any button item that deletes Channels dock -> edit channels name button Brush dock -> edit this brush button Gradient dock -> Edit gradient button The last three dock ones seem to be the same icon.
I think I have merged everything which had to from the icons-wip branch (except for this stuff, I believe: bug 772019). Some icons are still missing and some things have to be ironed-out, possibly before release. But now the infrastructure is there and all icons can easily be replaced. So I will just close this bug report which was about not being able to replace some icons.