Holy shit :) Thanks, will look into this ASAP.

Welcome, the only caveat is that you need to restart the app for the changes to take effect. I haven't figured out how to force a reset for the whole app so it can redraw all the icons.

That patch looks very good. I have to look into how we set the icon theme,
but that's about the only thing I have to look into :)

Could you turn the patches in the zip into one patch and attach it
separately please? The patches contain .orig and .rej files, and
some parts of the various patches are redundant.

I better let you clean up that before I mess with something...

Sorry for that i thought i'd taken all those out. Then again i was getting tired at the end of yesterday lol. Looks like i missed a few, I'll see about cleaning them out. 

Also not sure if you want to keep the icon i made, if you have anyone that can do better best to replace it in that case :) It was something i threw together real quick.

Just to confirm one patch for each branch right?

We don't need a patch for gtk3-port because I will rebase it on top
of master after applying your patch to master.

there is a minor difference between each, that's why i was asking.  I'll resubmit only the master branch then.

Created attachment 316878 [details] [review]
Patch for master to add icon theme support

Thanks! I guess I will see the conflict when rebasing gtk3-port and hopefully
resolve it :)

Ok it seems I have some open questions.

Why create an own icon theme at all and not simply set the search
pathcof the default icon theme.

Apparently I haven't looked into how switching icon themes works,
particularly how to get a consistent look across all (default *and*
GIMP icons). Have you looked into this and can help me out?

Shouldn't we move the "hicolor" folder into a "Default" subdirectory
and simply switch between these subdirectories when we change themes?
It's all falling back to "hicolor" and al icon themes should inherit from
it.

Also, where does that "greycolor" icon theme come from? It doesn't seem
to inherit from hicolor, so how are we going to find all the icons
that are not provided by it?

Oh no disregard that i was testing just to make sure i would notice the change, so yeah you're right it sucks for that. 

Mind you i did try that earlier today on my linux box by setting inheritance to either hicolor, Hicolor, gnome, and default. Sadly that didn't change much but i think it is because i am setting a new icon theme and not using the default one and losing those paths it contain. I believe adding them in will resolve that but i still need to fix the code for. 

Spent to much time this PM trying to figure out how to force a redraw of the icon/widgets when doing the change. 

Will look at this path thing now instead.

As to why use them at? Not sure why you guys switched to that, guess it should have made things easier at some future date.

As to why i did, well i spent a week trying to use the official methods in Linux to override it and none of them were working to my satisfaction hence why came up with the patch.

As to the why change them for a user, that's easy to change the look to something more compatible with whatever the user is comfortable with, for example i have a set that i am working on a set that replaces the gimp ones with ones with some that are similar to Photoshop. Actually i think that's the one i used in the screenshot actually instead of what i submitted with the patch. 

Not sure for other OS's but under Linux i would have had to set it globally and then have had to inherit whatever default one i preferred. For some reason on my setup (currently Ubuntu 15.10) that still didn't work out at all. 

Yes if you are going to override the default theme then you're going to have to deal with overrides for the gtk stock ones as well to stay consistent but that would be up to the theme maintainer to fix.

Right, as to the paths i did look into a bit of this earlier today because right now the patch only looks for two paths either the default gimp or the user one and may me sacrificing whatever was default. I am looking to figure out how to get the default one and add them back in appended after either of those two cases get used (gimp and user directory). When i submitted it for some reason it was working fine for both gimp and stock gtk icons (which was a fluke i think) using my current theme for the default gtk stuff. Started showing the basic default ones. So as i say i think something is getting lost and i think find the original defaults and adding them in should fix that. 

When you say move into a default subdirectory not sure what you mean, but for the paths its like i said in the paragraph above. 

Feeling rusty with this gtk stuff lol, last time i did any serious work with it gtk 1.2 was still it 'ish :)

(hmm for some reason this didn't get posted before that last one, trying again).

What I meant was not "why use a custom icon theme at all" but why not
stick with gtk_icon_theme_get_default() and configure its path to
include the GIMP icon theme. That's what we do now and it works fine,
so simply switching between GIMP icon theme folders, each containing
a default "hicolor" hierarchy and index file should work fine, an would
make the patch much smaller and less intrusive.

BTW, to properly notify all widgets of icon changes, it's probably
enough to emit "notify::icon-theme" on the GtkSettings object, but I
haven't verified that.

Because with get default you can't use a custom theme. 

It gets flagged as private and you cannot do anything else with it. I went and looked at how gtk were doing it in code to figure it out. If you use get default for icon theme and then try to set a custom one mostly only the gtk icons get overriden but all the stock ones that get registered in gimpstock.c get stuck with the originally defined theme. 

When i tested with the grey color theme i posted here i would get some grey ones in prefs and everywhere else was the default colored ones when doing it the original way it was. Took me 3 days pouring over gtk lib code and testing it to even get this much :) 

Thanks for the heads up on that gtksettings will look at that again.

from https://developer.gnome.org/gtk2/stable/GtkIconTheme.html#gtk-icon-theme-set-custom-theme

gtk_icon_theme_set_custom_theme ()

Sets the name of the icon theme that the GtkIconTheme object uses overriding system configuration. This function cannot be called on the icon theme objects returned from gtk_icon_theme_get_default() and gtk_icon_theme_get_for_screen().

So far this is the only way to use your theme and still be able to set custom ones. If someone else can show me another easier way i'll be glad to switch to it :)

I've fixed the path issue inheritance now works again, and when using the default theme it no longer tries to load Hicolor as a custom theme. Almost have it working when reloading the new icon theme. Hoping to have a new patch tomorrow.

That sounds like me digging in GTK+ when first porting to icon themes...
Good luck, looking forward to the patch :)

Just a note. If this is fixed we can have a look at this bugs:

Add HiDPI icon theme for gimp-2.9 (gtk2)
https://bugzilla.gnome.org/show_bug.cgi?id=725464

Provide a dark theme for GIMP
https://bugzilla.gnome.org/show_bug.cgi?id=757716

Migrating 2.8 theme to 2.9/2.10
https://bugzilla.gnome.org/show_bug.cgi?id=750105

Created attachment 316978 [details] [review]
Patch for master to add custom icon theme support

Haven't added the code to redraw the window at this point because it only manages to redraw the app and not the prefs dialog window. No matter how i try to force it to redraw the icons stay with the original theme in prefs while they change everywhere else in the app. I may revisit this for the weekend or early next week. So for now i am only submitting the code to handle selection. Users will be greeted with a dialog to restart the app upon selection change. 

The patch now works as it originally did for the default theme, but for custom themes maintainers will either have to manually inherit or support all of icon items. As far i as i can tell its a limitation in gtk2 (i could be wrong lol but the documentation while concise for each function is rather obtuse on the whole). 

I have the base of the redraw patch (without multitudinous revs i've tried for for the prefs dialog) handy as a separate file should want that as well.

Thanks a lot! Will look into this right away :)

Welcome. I figure its safer not to have the redraw feature on selection change (for now) to avoid any unnecessary support questions at the onset. I may be slow to answer for a few days later this week, personal stuff to attend to (fyi) :)

One question comes to mind if you are planning to support more than one official default theme? If so the code may require further changes at that time.

We do plan to ship a flat theme with 2.10.

Created attachment 316989 [details] [review]
Unfinished changed patch

I asked around on #gtk+ what would be the best way of changing icon
themes, and changed your patch to use the default theme and modify that.
All GTK+ widgets are listening to that theme, so live theme changing
works now. It however requires the themes to live in subdirectories
so they can all be called hicolor. It also simplifies the patch.

I had something similar originally and it didn't work on my ubuntu box but i will try this and see what happens. As to the subdirectory where specifically would my theme reside? for example with my patch set it was checking the data dir for gimp and the user home data dir.

Created attachment 316991 [details]
screenshot ubuntu unfinished patch

Created attachment 316992 [details]
screenshot prefs ubuntu unfinished patch

I ok it still finds my home folder, but this is what i get in ubuntu with your patch. Attaching screenshot-1.png

Also note that the prefs icons haven't changed automatically. But if close it and reopen prefs i now get screenshot-2.png.

I tried hicolor and Hicolor as the theme name, i get the same result no matter the name i give it.

I did a fresh build from a fresh clone from the git repo i pulled from two hours ago, same thing.

The only one working fine is the Default one.

Just checked and am running with libgtk 2.24.28 for v2 and 3.16.7 for v3.

You need to put your theme in an extra subfolder, so the
paths would be e.g.

~/.config/GIMP/2.9/icons/MyTheme/hicolor/16,32,...

Then you can choose between "Default" and "MyTheme". You never see
"hicolor" in prefs.

Prefs being broken in a problem in prefs, not with theme changing,
that will need some refactoring. All icons that use e.g. GtkImage
or GtkImageMenuItem do change live and work fine.

It's 4am here time for coffee before my eyes roll in backwards and i shut down, lol, i spent 24 hrs trying to get that to work and it explains that (the prefs not changing), so much to learn about the codebase still i guess.

Just did a clean rebuild again to make sure. Sadly i still get the same issue with your suggested folder structure but it works with my patch. 

I looked at the patches from ubuntu for the lib they include and can't see any reason why it would be an issue, using my patch works with that folder structure so at this i would have to guess its maybe an issue with get default on linux.

Hmm, that's odd. The word from the #gtk+ IRC channel was to definitely
use and modify the default icon theme. I suggest I push my modified patch,
fix prefs and then let's see what happens :)

Maybe I messed somewhere... will take care of this after work (in around
8 hours).

Indeed, but was that for gtk2 or 3? For that matter are they still working or fixing gtk2?

I doubt that you messed up, because like i said i took a similar route originally and only after i got the same results did i research to tweak the code to override get default. 

Well have a good one then :)

Just to be sure: where did you put your greycolor icon theme for
use with "unfinished patch"?

/home/myuser/.config/GIMP/2.9/greycolor/hicolor/ and in there i have the various sized directories (12,16,22,etc...), the index.theme was in hicolor and i copied it over from the Default directory to make sure i hadn't messed it up previously.

Try /home/myuser/.config/GIMP/2.9/icons/greycolor/hicolor/ before you
drop sleeping :) Should be 4:30 by now...

sorry i mistyped i did have icons in there jut double checked it

still feels like its 4:30 lol, but its two hours ahead now :D

Argh, put it in $prefix/share/gimp/2.0/icons

I think I did mess with using the config folder, but that can be
fixed.

No wait, it does work perfectly for me with the folder in
~/.config/GIMP/2.9/icons

And the index.theme must still be in the hicolor folder...

Pushed it during lunch break instead so others can try. Let's continue
from here.

commit 6762bf5b9be9eeb3a8f214647f1460152e227af1
Author: Benoit Touchette <draekko.sofware@gmail.com>
Date:   Tue Dec 8 14:52:12 2015 -0500

    Bug 759105 - Patch to add icon theme selection
    
    Add support for custom icon themes much like custom theme
    support. There is still work to be done but this is the
    basic functionality.

 app/config/gimpguiconfig.c         |  30 ++++++
 app/config/gimpguiconfig.h         |   2 +
 app/config/gimprc-blurbs.h         |   6 ++
 app/core/gimp-gui.c                |  12 +++
 app/core/gimp-gui.h                |   2 +
 app/core/gimp-user-install.c       |   1 +
 app/dialogs/preferences-dialog.c   | 112 ++++++++++++++++++++-
 app/gui/Makefile.am                |   2 +
 app/gui/gui-vtable.c               |   9 ++
 app/gui/gui.c                      |   3 +
 app/gui/icon-themes.c              | 261 ++++++++++++++++++++++++++++++++++++++++++++++++
 app/gui/icon-themes.h              |  34 +++++++
 app/widgets/gimphelp-ids.h         |   2 +
 icons/16/gimp-prefs-icon-theme.png | Bin 0 -> 492 bytes
 icons/16/gimp-prefs-icon-theme.svg | 196 ++++++++++++++++++++++++++++++++++++
 icons/22/gimp-prefs-icon-theme.png | Bin 0 -> 653 bytes
 icons/22/gimp-prefs-icon-theme.svg | 202 +++++++++++++++++++++++++++++++++++++
 icons/Makefile.am                  |  30 +++---
 libgimpwidgets/gimpstock.c         |   7 +-
 19 files changed, 895 insertions(+), 16 deletions(-)

It works, i deleted all the original folders i had there and made a new one and it works now. Weird, it still looks like the same set of files but something must have been messed up in there originally. I tried it both ways in config and from share/gimp/2.0/icons with two sets of icon themes (i have another one i was working on) and both now work. Disregard my previous comment. Also i can confirm that you can put any name in the index.theme file, i guess having the folder structure set up that way overrides that or doesn't get used. Note to self start fresh when things get weird lol.

Will try the new commit in a bit.

Created attachment 317014 [details]
Desaturated icons

This is your gray theme that works for me, index.theme from the
default theme just to be sure.

Oh, didn't see you comment. Great :) Now for fixing the things that miss
the theme change...

You'd think with all the tech support i've done over the years i'd have remembered to nuke my config folder and start over before now. *Rolls eyes* idjit :)

Doing make now, will let you know later today how it went, should work now :)

As for that other issue i'll try to find some time later today to start figuring that out. Unless you want some else to give it a go :)

It looks like that the "gimp-prefs-icon-theme.svg" is licens as GPL 2.0:
"<cc:License         rdf:about="http://creativecommons.org/licenses/GPL/2.0/">"

As the SVGs are from Jakub Steiner it I think he should be asked to change the licence to GPL 3.0.

It was from the gimp-prefs-themes.svg as a base for it, i guess there are probably a few more of them there like that. Probably worth double checking :)

That's awesome. I haven't read all in details in this long bug report, but I can see there is work on-going for finally making the long-awaited symbolic (light and dark) themes. Right?

Or is it work only on the platform itself and you are not working on the new symbolic icon theme itself?

> As the SVGs are from Jakub Steiner it I think he should be asked to change the licence to GPL 3.0.

Jakub Steiner and Barbara Muraus already agreed to relicense their work under GPLv3. Unfortunately we only have indirect message through Scl, but since Jakub is a GNOME member and very usual contributor, I have no doubt we can ask him to repeat as a public message himself (on the tracker, or a mailing list)…
See: https://mail.gnome.org/archives/gimp-developer-list/2013-November/msg00011.html

I'm not sure which icon set you are starting from, but another contributor (jEsuSdA) also started to work on some additional icons at some point.
See: 
https://mail.gnome.org/archives/gimp-developer-list/2013-October/msg00020.html

Anyway awesome work.

Nope the icons in this bug report were just placeholders i used to test the patch with :)

@Micheal Took an hour and almost fixed the prefs icon theme issue, just need how to replace the pixbuf in the cell with images, there only seems to be a pixbuf cell render in gtk. The rest is working now. I'll tackle that later.

> Nope the icons in this bug report were just placeholders i used to test the patch with :)

Yep I realized this after I posted by checking the screenshots more carefully. :-)

I have actually started to have fun with your changes, and will push in a feature branch commits with all the icons from Jimmac (Jakub Steiner), Barbara Muraus and jEsuSdA. This should be available in an hour hopefully.

Cool, but please don't replace the Default theme
just yet, please push to a new theme "Symbolic".

Of course, I was never planning to replace the default theme. And just in case we are not on the same page, I think we should keep the default theme as an alternative.

I don't mind if the symbolic theme becomes the new default in GIMP 2.10, but the old default should be kept around and selectable. :-)

I agree with everything you said :)

Ok the symbolic icons are now available in branch "origin/symbolic".

I have only committed the icons from Jakub/Barabara/jEsuSda which were present in our current default icon theme (i.e. I did not include a whole bunch of icons from jEsuSda archive because they were not in our current set).

We are still missing a whole bunch of icons (or sizes). To get the full list of missing icons, run these commands in a terminal:

> cd icons/
> find default/ -name '*.png' |sort |sed s/^default// > default-icons
> find symbolic/ -name '*.png' |sort |sed s/^symbolic// > symbolic-icons
> diff default-icons symbolic-icons |grep '^<' | sed 's/^< /symbolic/'

As of today, this is 49 icons missing (I have not checked if the missing ones are all really used. I can just say they are missing compared to current default icon theme). That's some work for all designers who contribute to GIMP. :-)

Also note that I have not made a dark theme yet. This first commit just adds the icon theme.
I'll see later to add a new theme too. :-)

Created attachment 317064 [details]
Screenshot with Barbara/Jakub/jEsuSda icons.

Oh while I am at it, a small screenshot showing the current state. As we see, some icons are missing, but that's a good start. I have sent an email to gimp-gui mailing list hoping to get icon contributions.

@Jehan if you want to save some time i had started something https://github.com/draekko/gimp-cc-themes everything in images are from someone else's theme and i hadn't replaced them yet, most of the ones in UI are mine but since they aren't needed anymore with the icon themes you could use the core to get you started for a dark theme.

These "missing icon" icons make me think that we should always have
our complete default theme in the search path after the chosen theme,
so we at least have a fallback icon.

Pushed to master:

commit 5beb69d0096c28863fcf12f5e32cbb1ee6042480
Author: Jehan <jehan@girinstud.io>
Date:   Wed Dec 9 19:18:28 2015 +0100

    icons: new symbolic icon theme for GIMP.
    
    Images originally created by Jakub Steiner and Barbara Muraus as the
    "Art Libre" icon set.
    The contributor jEsuSdA later worked on it.
    It will now be available in the new icon theme selection through
    preferences.
    Many icons/size are still missing but this first commit makes a start
    for complementary work.


> @Jehan if you want to save some time i had started something

Thanks but I actually already had a working dark theme somewhere (that's not the hardest thing to do anyway starting from the Default theme!). The icon theme issue was the only one which I did not manage to have working, and I never made the time to dig in the code for this specific issue.

But you did it and you fixed it. Thanks for this, it's awesome. :-)

Welcome, still need to fix the prefs dialog, trying to figure out how the cell render works, looking at adding one for images so that the prefs will also update quasi automagically on theme change.

> These "missing icon" icons make me think that we should always have
our complete default theme in the search path after the chosen theme,
so we at least have a fallback icon.

Fixed with:

commit 6fe8f75f5dc5870741cdcd54f82a242da864c58c
Author: Jehan <jehan@girinstud.io>
Date:   Wed Dec 9 23:46:58 2015 +0100

    app: prepend the selected icon theme path to the search path...
    
    ... instead of replacing the first path.
    This allows to fallback to our complete default theme icons when the
    selected icon theme has missing icons.

draekko > actually thinking about it, I'd be more than happy to delegate the work, and you obviously have more experience than me in theme making.

If you wish to propose us a working dark theme for GIMP master, you can upload a patch for this. :-)

Also do you know if there is a way for a theme to invert monochrome icons?

It would be fairly easy to create a dark version of our new symbolic icon set during compilation by inverting colors, but I wonder if this cannot be done on the theme side, dynamically. Then there would be no need to show 2 symbolic icon theme: the user selects the only Symbolic icon theme, simply when one loads the dark theme, we invert the colors.

@Jehan Is there a time frame for this? As for inverting icon colors no sadly to my knowledge it can't be done without right code for it. Makes me wish gtk had a tint mode like Android does for its widgets since v5. Then you could set it in your properties like anything else. With that you could have a whole rainbow of icons lol.

writing code i meant XD

Jehan: had to revert your last patch, it let the search path grow
infinitely. I have a (more complicated) patch locally, but it's not
quite right yet, will push asap.

Mitch > no problem.

Draekko > there is no time frame. The sooner, the better. I have also asked on the gimp-gui@ mailing list if anyone was willing to contribute a dark theme. I guess the first contributed usable theme will be good (though it is not a race, and everyone should also collaborate to build together the best fit theme).

@Jehan i'll try to find some time to make something :)

Created attachment 317201 [details]
Adds missing icon for size 48

@Jehan i get this when i switch to the symbolic icons "incorrect gamma=(0/100000)" fyi

@Micheal Just added a fix for support of icon themes in the prefs dialog.  https://bugzilla.gnome.org/show_bug.cgi?id=759345

Comment on attachment 317201 [details]
Adds missing icon for size 48

Thanks for the icon, but I already noticed myself and added it :)

All good, at the time i hadn't realized that one was needed for the notebook part of the prefs dialog :)

> i get this when i switch to the symbolic icons "incorrect gamma=(0/100000)"

As an error popup? In terminal output? Does the icon switching work otherwise (other than the missing icons)?
Because I have no such error.

Linux Ubuntu 15.10, shows in terminal. When i run mogrify with the set it also isn't happy with them (was inverting/negating the colors to do tests on darker/lighter set). Don't have the exact output from mogrify right now.

Created attachment 317220 [details]
Alternate (Color) Default Icon Theme icon

You might prefer this colored icon to the original one i made for the 'Default' theme.

Created attachment 317221 [details]
Proposed Icon Theme icon for the 'Symbolic' theme

Proposing this as the icon for the 'icon theme' in the 'Symbolic' theme. Includes dark and light version.

Symbolic icon committed. We only need the light version. We will create the dark ones at build, this is easier and will make sure icons are consistent.

Don't you have a SVG as well for these though?

@Jehan No i handcrafted them while having breakfast, i'll try to cook some up this weekend.

This commit should complete the feature, app and plug-ins now use
the same theme, and the default theme is always used as a fallback
for missing icons:

commit 69f87bcc84304456b5305ff416b8bad5491e550d
Author: Michael Natterer <mitch@gimp.org>
Date:   Sat Dec 12 19:32:11 2015 +0100

    libgimpwidgets: be smarter when changing icon themes, and have fallbacks
    
    Add gimp_stock_set_icon_theme() which can be called at any time, also
    before gimp_stock_init(), in which case we avoid setting the configured
    icon theme twice on startup. Call it from libgimp/gimpui.c and
    from app/gui/icon-themes.c so the app and plug-ins use the same
    icon theme.

 app/gui/gui.c                  |   7 ++-
 app/gui/icon-themes.c          |  21 +-------
 libgimp/gimpui.c               |   5 ++
 libgimpwidgets/gimpstock.c     | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 libgimpwidgets/gimpstock.h     |   4 +-
 libgimpwidgets/gimpwidgets.def |   1 +
 6 files changed, 158 insertions(+), 28 deletions(-)

Hmm, I'm just rebasing gtk3-port and the icon theme doesn't change...

You said there was a small difference in the patches, what exactly
was it?

I think i had to call "gtk_style_context_reset_widgets(gdk_screen_get_default())" instead of calling "gtk_rc_reparse_all()" after setting a new icon theme in icons_theme_change_notify (icon-themes.c) to get the gtk3 version to switch over.

Thanks, will try that after rebasing the next bunch of master changes...

It really looks almsost finished now, thanks again for getting this
started with a good patch! :)

And, you should really come to IRC where all the fun most discussions
happen...

Created attachment 317302 [details]
Symbolic vector icons

SVGs for the symbolic icon set. Both light and dark.

I think you uploaded the wrong archive. It contains only the png images.

Also I already said it, but there is no need to duplicate your work twice. I think it really makes more sense to generate the inverted icons at build time (see bug 759384 where I already have a patch for this).

@Jehan yeah re-uploading once more. I know but I like to be thorough, since i was in it and done i figured may as well get them out there (both light and dark), it'll be there for those times where you might need them in some distant future. ;)

@Micheal Glad i could help. I haven't used IRC since the very late 90s lol. Just had a flashback of getting usenet feed msg's from a fidonet bbs gateway from some guy running a unix box as a bbs locally. XD 

Where and what channel, i'll try to set some time aside for this. So much to do, not enough time :/

Created attachment 317303 [details]
the missing svg icons for symbolic theme

Vectorial icons added:

commit 46b9c6927bbf68af3785910e48f365c5c32e4cb8
Author: Jehan <jehan@girinstud.io>
Date:   Sun Dec 13 15:44:03 2015 +0100

    icons: add vectorial icons by Benoit Touchette for prefs-icon-theme.


---------

As for the IRC channel, it's #gimp on irc.gimp.org.

@Jehan thanks, time to find a decent chat program for linux.

@Jehan you'd ask about controling icon colors, not sure you can from the theme side, not directly. I was in the gtk3 docs looking some info over and noticed they added gtk_icon_info_load_symbolic* functions which if i understand correctly does let you set the color for symbolic icons. 

https://developer.gnome.org/gtk3/stable/GtkIconTheme.html#gtk-icon-info-load-symbolic 

I'll try to take a look at this some time in the coming weeks to see what it actually does.

Yes for GIMP 2.10, I guess we will simply provide 2 versions of the icon theme (one being simply generated from the original one). For GIMP 3 though, we should definitely have proper symbolic icons changing colors automatically.

Since you are obviously planning to look into this, when you make a patch, could you please open a new ticket? The current one has been close and has already so many comments.
Thanks! :-)

@Jehan Re: the patch that would have been my intent lol.

*** Bug 743269 has been marked as a duplicate of this bug. ***