Okay, without looking too deeply I have to say it is a brilliant idea to use a color-inverting shader for this. The result looks perfect. This could be an awesome tool to use in a bunch of places.

I don't think we'd want to enable it in themes as shown, although that is a great way to show off the feature here. As a shader it would be nice to reserve its use to the last moment in the chain, based on the background it is printing onto at that time.

So, for example, you could have two 3DViews, one with dark background and the other with very light. If using only theme settings both toolbars would have same background and icon colors. But using the shader programatically based on current background we could always show contrasting icons no matter where used.

@Yevgeny Makarov (jenkm): Hey very cool. You could restrict inversion to just very bright colors based on their luminance value compared to a treshold.

Edit: Sorry obviously not just the bright but also the darker colors should get included. But the middle part in terms of luminance could be kept. That way the icons would keep their original look better. Might be worth a try.

@Harley Acheson (harley) Yes, you're right. The toolbar icons are also shown in the Header and in the Active Tool panel, and in theory their background can be different, one dark and the other light.
Let's assume that this is some help to @Ankit Meel (ankitm) with the D6512 and proof of concept, rather than the final patch.

LGTM overall. This really enables a lot of bright themes.

Mainly I'm not sure if we need to add a toggle, or if we could rather just do it automatically based on the toolbar bg color.

Overall I'd probably prefer it to just be automatic, in order to not make the theme editing even more overwhelming.

@Clément Foucault (fclem) I'll leave it up to you if the shader is acceptable.

@William Reynish (billreynish) - ...not sure if we need to add a toggle, or if we could rather just do it automatically based on the toolbar bg color.

This, and a million times this. If we add this shader as a general tool we can use it to solve this problem automagically, but also similar problems all over the place. I think we first just need the shader reviewed. @Clément Foucault (fclem) might see some ways of combining or removing redundancies between these and what we now have. But this would be an awesome addition to solve contrast problems on light colours.

More optimized shader.
Source: https://gamedev.stackexchange.com/questions/59797/

uniform float factor;
in vec2 texCoord_interp;
out vec4 fragColor;

uniform vec4 color;
uniform sampler2D image;

void main()
{
  vec4 tex = texture(image, texCoord_interp);

  vec3 c;

  // unpremultiply alpha
  if (tex.a == 0.0 || tex.a == 1.0) {
    c = tex.rgb;
  }
  else {
    c = tex.rgb / tex.a;
  }

  // invert
  c = 1.0 - c.rgb;

  // rgb to hsv
  vec4 k = vec4(0.0, -1.0 / 3.0, 2.0 / 3.0, -1.0);
  vec4 p = mix(vec4(c.bg, k.wz), vec4(c.gb, k.xy), step(c.b, c.g));
  vec4 q = mix(vec4(p.xyw, c.r), vec4(c.r, p.yzx), step(p.x, c.r));

  float d = q.x - min(q.w, q.y);
  float e = 1.0e-10;

  vec3 hsv = vec3(abs(q.z + (q.w - q.y) / (6.0 * d + e)), d / (q.x + e), q.x);

  // hue offset
  hsv.x += 0.5;
  if (hsv.x > 1.0f) {
    hsv.x -= 1.0f;
  }
  else if (hsv.x < 0.0f) {
    hsv.x += 1.0f;
  }

  // hsv to rgb
  vec4 r = vec4(1.0, 2.0 / 3.0, 1.0 / 3.0, 3.0);
  vec3 t = abs(fract(hsv.xxx + r.xyz) * 6.0 - r.www);

  c.rgb = hsv.z * mix(r.xxx, clamp(t - r.xxx, 0.0, 1.0), hsv.y);

  // premultiply alpha
  tex = vec4(c.rgb * tex.a, tex.a);

  // desaturate
  tex.rgb = ((0.3333333 * factor) * vec3(tex.r + tex.g + tex.b)) + (tex.rgb * (1.0 - factor));

  fragColor = tex * color;
}

@Yevgeny Makarov (jenkm) @William Reynish (billreynish)
Hi, by restricting the color inversion the whole shader could be much simpler.
I dropped the HSV conversion completely and tested out the following two inversion ideas. This also has the benefit that the inversion keeps the intended look of the icon better as colored parts keep their values unchanged.
The first one is limiting the inversion to bright and dark colors.
The second one is identifying all values that are nearly greyscale and inverts just these.
I put both invert methods next to each other in the same shader, which is meant as a possible replacement for gpu_shader_image_color_reverse_frag.glsl

in vec2 texCoord_interp;
out vec4 fragColor;

uniform vec4 color;
uniform sampler2D image;



void color_alpha_premultiply(vec4 color, out vec4 result)
{
  result = vec4(color.rgb * color.a, color.a);
}

void color_alpha_unpremultiply(vec4 color, out vec4 result)
{
  if (color.a == 0.0 || color.a == 1.0) {
    result = vec4(color.rgb, color.a);
  }
  else {
    result = vec4(color.rgb / color.a, color.a);
  }
}


void color_invert2(vec4 color, out vec4 result)
{
    float diffA= abs(color.r-color.g);
    float diffB= abs(color.r-color.b);
    float treshold = 0.1;

    if (diffA < treshold && diffB < treshold) {
      result = vec4(1.0 - color.rgb, color.a);
    }
	else {
	  result = color.rgba;
    }
  
}

void color_invert(vec4 color, out vec4 result)
{
    float lum = 0.2126 *color.r + 0.7152 *color.g + 0.0722 *color.b;
    float treshold = 0.88;

    if (lum > treshold || lum < (1.0-treshold)) {
      result = vec4(1.0 - color.rgb, color.a);
    }
	else {
	  result = color.rgba;
    }
  
}


void main()
{
  vec4 tex = texture(image, texCoord_interp);
 
  color_alpha_unpremultiply(tex, tex);
  color_invert(tex, tex);
  color_alpha_premultiply(tex, tex);

  fragColor = tex * color;
}

Haven’t looked into these implementations yet, but excited about the thought of using them. A general caution though about trying to limit the color inversion to just light and dark parts. We will eventually have color management on everything, and so the cutoff for what looks light or dark will change as your display devices changes. Best practice is to use as few hard-coded values as possible, none being ideal.

@Debuk (Debuk)

You need to invert all the colors so that all the dark colors becomes light and all the light is dark, but keep their hue/color.

Your current result:

If performance is very important here then, perhaps it can be implemented based on rgb_to_yuv() and yuv_to_rgb() transformations, with some changes.

y = 0.299f * r + 0.587f * g + 0.114f * b;
u = -0.147f * r - 0.289f * g + 0.436f * b;
v = 0.615f * r - 0.515f * g - 0.100f * b;

y = 1.0f - y;

r = y + 1.140f * v;
g = y - 0.394f * u - 0.581f * v;
b = y + 2.032f * u;

Hi @Yevgeny Makarov (jenkm), no I don't think we will have to invert the complete icon content. That might be true for arbitrary image content , images with lossy compressions, or icons with many different shades. But in the given case the we have just handdesigned icons with a heavily reduced color palette. I think we have just greyscales and a single keycolor in an icon. That's the opposite of an impossible case for getting a good color separation working.
Anyhow I just wrote it quick down in a couple minutes , so none of the cases I posted were extensively tested or tweaked yet. I posted it just as a base to discuss this as an option.

And yes YUV would fit much better for breaking down the computational part. And you wouldn't have to compensate for a shift in chrominance part. But the luminance part is still off, so the intended keycolor is gone. Trying to fix that would also come to the same point that we have to be evaluate the image content and just partially invert. But I don't see a big problem here. Personally, judging on the current set of icons, I still think the greyscale version would fit best ( caution I guess currently it has a far too large treshold).

But for sure the chosen algorithm and the icon designrules have to fit to each other.
I say that keeping the keycolors intact is a benefit I would go for. But this all is just proposal for a small change in your patch. Really cool idea. Looking forward seeing your patch embedded. Btw. if there is just a single keycolor as a guideline for icons ( not sure about that) other robust and simple options would become possible too.

Edit: Please test the invert2 method. I couldn't make out any visual problems with that solution here

We will eventually have color management on everything.

@Harley Acheson (harley) Could you explain that a bit more? Or guide me to some info about that. How and at what stage this shall be applied to the ui? Hard for me to think about if that has consequences here as I really read nothing about that yet. Is that a sort of post processing effect, do the icons get changed beforehand,...?

Sorry, but I don’t have any details because there isn’t any real plans yet, just (finally) a general growing interest and some consensus that we’re going to have to color manage everything, including the UI, in order for it to work right. But who knows might take, months, years, or forever. LOL.

But the general concern is about taking action based on something that might be changeable. So calling one shader or another based on brightness that assumes BT.709 primaries, then the shader itself changing only some colours based on cutoffs that assumes those same primaries and transfer characteristics.

So no, I have nothing concrete to say, other than we have to start thinking about this stuff. And there will be less correction and cleanup to do if there are less arbitrary hard coded values. But don’t worry too much about my vague moaning. I’m still very excited about getting some version of this shader regardless.

Ok I see. And when it finally arrives we'll be there to have a look. But especially in the grayscale version the treshold is not a hardcoded value in that sense but rather an epsilon (the invert2 variant). Could you perhaps also have a look if the icons invert well with that version?

I haven’t managed to run any versions of these - I’m away from home for a couple of weeks so can’t compile or commit, etc. But someone with actual smarts, unlike me, will probably evaluate these before I get back and can play with it.

Ok, no problem. Thanks anyway.