"Wide Gamut Color Selector" Docker
Open, Needs TriagePublic

Description

"Wide Gamut Color Selector" Docker

Basic idea: Re-implement Advanced Color Selector (ACS) docker using KisVisualColorSelector.

While it may seem just a matter of replacing the widgets of the central selector and add some "glue" to connect the interfaces, the selector isn't really an encapsulated part just controlling the state of color selection. That every part of ACS basically just minds its own business and manages its own internal state is part of the issues I have with it, not just the lack of non-sRGB color spaces.

The advantage of not using a central internal state is that the main selector and the shade selectors can just work in the color model of choice, but each part needs to reconstruct its state from the new KoColor. That limits precision or even gamut and requires various precautions for edge cases (e.g. undefined hue/saturation of black in RGB representation).
One could try to mitigate those issues by using wide gamut float color spaces everywhere, but that makes the already tricky code even more tricky and possibly slower too. And then there's non-RGB color spaces that KisVisualColorSelector supports natively.

So instead of starting a giant patchwork, the more sane solution seemed to be to just start a fresh docker from scratch, avoiding duplicate state information as much as possible and exchange colors in the model (HSX, CMYK etc.) the main selector stores internally, in float precision.
Not just the selector state but also the preview popup suffers issues with having multiple instances getting out of sync with ACS docker.

That approach of course drastically reduces the amount of code that can be re-used as it is, and requires a bunch of design decisions:

Consequences the new approach has:

  • The minimal shade selector becomes tied to the color model of the main selector (HSV, HSL, HSI, HSY'). With ACS it only supports HSV to begin with, so on one hand it gains new modes, on the other hand the mode is currently linked and not independent. Conversion could be done of course with the mentioned problematic, question is, would it be worth the hassle?
  • How do we want to handle non-RGB color spaces in shade selections anyway? Technically, handling CMYK and Lab gradients can be implemented easily, but then it basically needs to have separate configurations. (Right now it simply does not work at all for non-RGB due to a short-sighted decision by me, but it's no big deal to change that).
  • Color hotkeys also need review. Right now, they have no internal state at all, and there's several bugs about them losing hue/saturation or just drifting off badly. Using the internal state would also either be restricted to modify the channels of the central color model, or keep suffering from at least the edge case issues

Things I personally don't like:

Since the decisions I'm faced with already question the possibility to just make this a drop-in replacement for ACS, I figured this is a good occasion to address a few things I never liked anyway:

  • No explicit foreground/background mode, if either color changes externally, it gets picked up, and using right-clicks to adjust color is extremely pen/stylus unfriendly. I also noticed many of the GUI mock-ups lately feature a FG/BG toggle, so I guess I'm not the only one.
  • Switching color models requires too many steps (open settings, 2 clicks to choose model from the dropdown, another two clicks to select the same selector layout with the new color model in the other popover, and finally "ok"). I'd prefer a dropdown directly in the selector where color model and layout can be changed with one click each.

Bugs/Issues uncovered in ACS

  • Shade sliders have a jump in values left and right of the neutral zone, the shades inbetween can't be selected
  • Every element has its own color preview popup, and most of them aren't properly initialized and/or updated, causing the popup to miss information of the previous color and the last used color

Current progress:

development branch: https://invent.kde.org/mwein/krita/-/tree/wide-gamut-selector-docker-master
(stale 4.x based branch: https://invent.kde.org/mwein/krita/-/tree/wide-gamut-selector-docker-refactor )
(old branch: https://invent.kde.org/mwein/krita/-/tree/wide-gamut-selector-docker )

  • Basic selection through KisVisualColorSelector widget
  • Gamut Masks:
    • Display in KisVisualColorSelector
    • UI element to toggle/rotate gamut masks
  • Minimal shade selector: Sliders
  • Minimal shade selector: Patches
  • Color preview popup
  • MyPaint shade selector
  • Color History
  • Colors from Image (only sRGB though!)
  • Color Hotkeys
  • Popup variants of main selector, shade selectors, history
  • Configuration:
    • Main selector
    • Minimal Shade Selector
    • MyPaint shade selector (no need for extra settings?)
    • Color History
    • Colors from Image
    • Docker Layout, Popup sizes etc. (partially done)
    • Hotkeys
mwein created this task.Aug 17 2020, 7:53 AM
woltherav added a comment.EditedAug 17 2020, 10:07 AM

The minimal shade selector becomes tied to the color model of the main selector (HSV, HSL, HSI, HSY'). With ACS it only supports HSV to begin with, so on one hand it gains new modes, on the other hand the mode is currently linked and not independent. Conversion could be done of course with the mentioned problematic, question is, would it be worth the hassle?

I personally do not think this is worth it. We've seen too many conversion rounding error bugs in the past, and we'll end up seeing them again. For our sanity's sake, this is not worth it.

How do we want to handle non-RGB color spaces in shade selections anyway? Technically, handling CMYK and Lab gradients can be implemented easily, but then it basically needs to have separate configurations. (Right now it simply does not work at all for non-RGB due to a short-sighted decision by me, but it's no big deal to change that).

Previously we just went through sRGB, but that might not be wise. For LAB, we could use LCH (which is what is used for the color hotkeys already, though the algorithm might need adjustment). CMYK has always been a huge hassle to figure out, and even for the hotkeys, we're currently going through sRGB.

Color hotkeys also need review. Right now, they have no internal state at all, and there's several bugs about them losing hue/saturation or just drifting off badly. Using the internal state would also either be restricted to modify the channels of the central color model, or keep suffering from at least the edge case issues

Yes, that is correct, and right now we're doing this in... I think kiscontrolframe and then going directly via the colorspaces. Putting these hotkeys as associated actions to the new acs-docker is indeed wisest, especially if people think the point of said hotkeys is to connect them to physical audio-equipment-dials or whatever.

No explicit foreground/background mode, if either color changes externally, it gets picked up, and using right-clicks to adjust color is extremely pen/stylus unfriendly. I also noticed many of the GUI mock-ups lately feature a FG/BG toggle, so I guess I'm not the only one.

I am ok with you doing this.

Switching color models requires too many steps (open settings, 2 clicks to choose model from the dropdown, another two clicks to select the same selector layout with the new color model in the other popover, and finally "ok"). I'd prefer a dropdown directly in the selector where color model and layout can be changed with one click each.

I am unsure about this, part of the current complexity of the config is because there's too many things to configure to begin with, and I feel this just shifts the complexity. I think this would require a bit more user-feedback, because... well, how often do you change these shapes?

We might also, for the future need to consider that there might be a nits slider in this docker at some point, though of course HDR is it's own beast here which I do not hold you responsible for.

EDIT: I am missing the mention of the gamut masks in your whole plan?

EDIT2: It would be great if we could have a generalized place outside the docker that 'colors from image' and 'color history' are drawn from, because then we can reuse that in the pop-up palette, but also reuse that in the palette docker, like 'generate palette from current color history' and the like. No idea where we'd start there.

I am unsure about this, part of the current complexity of the config is because there's too many things to configure to begin with, and I feel this just shifts the complexity. I think this would require a bit more user-feedback, because... well, how often do you change these shapes?

Okay, I guess it wouldn't be too complicated to make such a "quick access" menu optional, when disabled the config button just opens the full blown dialog as it does now, otherwise you get some quick options first. Or a small number of easily accessible presets could work too, so yes let's find out what users think.

I personally do switch a fair bit depending on what I do, but that may be just me still learning how to choose colors more confidently.

We might also, for the future need to consider that there might be a nits slider in this docker at some point, though of course HDR is it's own beast here which I do not hold you responsible for.

Ah yes HDR displays...I really wish Qt could handle that natively. I've wondered how hard it would be to base KisVisualColorSelector on QOpenGLWidget and how badly that would break UI compositing, but haven't dared yet to put in all the work to find out.
I only tried to avoid decisions that could make such a refactoring harder.

EDIT: I am missing the mention of the gamut masks in your whole plan?

Oh yes I knew I forgot something, KisVisualColorSelector itself needs to support it first though (just like LCH model which I'd like to have too), but it definitely should not be left out from the docker plans...

EDIT2: It would be great if we could have a generalized place outside the docker that 'colors from image' and 'color history' are drawn from, because then we can reuse that in the pop-up palette, but also reuse that in the palette docker, like 'generate palette from current color history' and the like. No idea where we'd start there.

Yes good point, the popup palette keeps its own history in KisFavoriteResourceManager, would be nicer to have one central list that is easily accessible, as you say there have been wishes to allow saving those colors to a palette.

mwein updated the task description. (Show Details)Aug 17 2020, 12:00 PM

Ah yes HDR displays...

For adding HDR support you should use this widget for the final rendering KisGLImageWidget. It supports HDR perfectly fine.

That every part of ACS basically just minds its own business and manages its own internal state is part of the issues I have with it, not just the lack of non-sRGB color spaces

I would try to consider some mode-view architecture, when the "view" classes would just show the colors in the color space specified by the "mode" in some way. But I don't know if it is possble

Just an FYI regarding opengl widget, it has unique handling of transparency: https://doc.qt.io/qt-5/qopenglwidget.html#limitations And the visualcolorselectorshape uses regions a lot to deal with transparency, so that might need some effort to get right.

I have a played a bit with KisGLImageWidget (messing with Small Color Selector basically) and Qt::WA_AlwaysStackOnTop attribute, but not extensively.
Stacking multiple partially transparent OpenGL widgets is probably just asking for trouble.

For a docker it should be more feasible to not have the selector parts as individual widgets but render the images on top of a solid background with OpenGL.
But I'm not sure what this would mean for the popup palette, I guess we'd have to turn it inside out so to say, render the UI on top of the selector and mask out the center so you can interact with it.

I would try to consider some mode-view architecture, when the "view" classes would just show the colors in the color space specified by the "mode" in some way. But I don't know if it is possble

I guess it's going in that direction bit by bit. I think I have eliminated all unnecessary "bookkeeping" outside of KisVisualColorSelector. All controls basically just give it new channel values for whatever color model it currently uses, and query the resulting KoColor. I've thought about splitting that "model" part off from the KisVisualColorSelector class because it's a bit awkward to have it in a QWidget based class.

I decided to try the split between color model handling and widget classes, but it was quite a bit more work that I thought. Main reason was that I needed to untangle the dependency on the ACS configuration, but that was on the todo list anyway.

Lacking a better idea, I just called it KisVisualColorModel for now. It's not a Qt item model class though, it "just" takes care of all the KoColor <=> normalized channel values conversions, with all the settings like HSV/HSI/HSL color model, and also provides the necessary information for display conversion.

To keep code changes small, KisVisualColorSelector currently creates a KisVisualColorModel instance on creation (sort of like a QComboBox creates a QStandardItemModel automatically) and forwards a few functions inherited from KisColorSelectorInterface to it, but otherwise only deals with the selector layout.
The numeric HSX controls in the color dialogs and the shade sliders now also just exchange values with this model and don't depend on the presence of a KisVisualColorSelector widget.

The whole configuration part will be a lot of work though, but to test if this all actually works I created a small "quick configuration" menu widget that allows switching between HSV, HSL, HSI and HSY'.

Since I figured it's easier (or less messy at least) to refactor the selector first and then rebase the docker code on top of it, I made a new branch:
https://invent.kde.org/mwein/krita/-/commits/wide-gamut-selector-docker-refactor

mwein updated the task description. (Show Details)Sep 15 2020, 7:06 PM
mwein added a comment.Sep 15 2020, 8:06 PM

Some of the settings are now actually saved to kritarc, the selector can finally be configured independently of ACS (though it still can load the ACS settings, so outside this new docker it's still all the same)

This is how the "quick settings" popup menu idea looks right now:

As you can see, it allows switching between the 4 HSx color models and 3 exemplary selector layouts, and at the bottom gives access to the full configuration dialog. I want to make the offered selector layouts configurable, so you could have only your favorite ones, or all 7 possible ones. The menu itself also should be optional, if you prefer the button to just directly open the configuration dialog.

Also, that selector layout you see here substitutes the triangle + ring from HSV, since I prefer to have color model and selector shape as independent options. HSV can be modeled as a cone while the others become a bicone, hence there's effectively two triangles for the latter, that I turned into this "diamond" shape. The bicone cross-section could be shown as a triangle too, but didn't quite like it. For one, it makes HSL identical to HSV and thus a somewhat redundant option (only that the axes are mapped differently to channels). Plus, my impression is that this variant just makes better use of the available area.

And one thing I'm not sure about yet is if the "quick settings" should affect other selectors outside of the docker, and if this should only be a temporary change or saved to kritarc.

woltherav added a comment.EditedSep 16 2020, 12:17 PM

substitutes the triangle + ring from HSV

Like, it removes the ability to choose the triangle? Or is it rather, only HSV mode has the triangle?

For one, it makes HSL identical to HSV and thus a somewhat redundant option (only that the axes are mapped differently to channels).

Yes, that is correct. The 'actual' way the triangle is supossed to work is that it aligns with the saturation that it has within the model, but we never got that to work right. So the even-sided triangle is actually a HSL triangle, HSV is supossed to have a straight-sided triangle, and HSI/HSY have differently shaped triangles depending on the hue. This is also how the munsell system works:

However, there's no way to draw such a triangle and not drive the user insane (and programmer) insane, so let's not do that. The square is a good alternative :<

And one thing I'm not sure about yet is if the "quick settings" should affect other selectors outside of the docker, and if this should only be a temporary change or saved to kritarc.

Personally, I feel like it should affect all the other selectors, if only to have less ambiguity. But that might just be me...

mwein added a comment.Oct 26 2020, 2:36 PM

Finally, all settings except the colorspace option are hooked up in the settings dialog, now the shade selector can finally be configured.

Shade Selector:

There's still a lot to improve, right now it always shows shade lines in HSV even though the shade selector will switch color model alongside the main selector, but I'm not sure what the best way to present this in the settings is, since setting the default color model is in another tab. And it's all limited to RGB right now, I'm still not sure what to do with other color spaces. Technically it can generate CMYK shades just fine, but that's only useful with a separate setup.

Also missing is the options for patches instead of sliders, but I wonder if it'd be useful to have this option per line. Question is, are people more likely to want sliders+patches, or rather switch between those modes for the whole shade selector as needed, which would be more work if you need to change each line individually.

Since to me the old shade line editor was really confusing, I decided to turn it basically upside down, and try to label the elements more clearly. I still need to add some presets below too, but right now it looks like this:

Other Additions:

I've also finally made the hue ring configurable, it can now be static like in Advanced Color Selector, plus a composite mode I implemented a while back that looks like this:

And I'm experimenting with how to make the selected layout stand out more from the other choices, like desaturating them, or rendering a checkbox or radio button indicator on top, but so far it doesn't look very convincing, needs more work before I can consider adding that:

mwein updated the task description. (Show Details)Jan 2 2021, 11:56 PM

Time for some new pictures...
So for the selector choice, I settled with a slim bar with the theme's highlight color to indicate the currently selected one. With the default dark theme it is rather subtle though, not sure what you think:

After some back and forth I also decided to make the slider vs. patches choice aswell as the patch count configurable per shade line in the "minimal" shade selector. I figured you don't reconfigure that all that often, so more flexibility can't hurt.
On top of that, I started with the popup variants, and one complaint about the existing popups is that there is no border/frame, which makes it hard to use as they disappear when the cursor leaves. So I added one.
Together this currently looks like this:

And one more little change, I never know which of the colors in the tooltip is the last selected and the last used one, and usually guess wrong. So I figured I swap them and add an indicator using the freehand brush icon:

Oh and one more thing I hadn't thought of before:
The current design kind of forces the popup selectors (and also keyboard shortcuts) to comply with the docker choice to modify the foreground or background color. This might be a bit confusing or even inconvenient, but I don't have a clever solution to that right now.

That looks good :-). I'm not sure I get the last point though.

The last point is about the toggle in the docker to choose between foreground or background modification (the Advanced Color Selector has no explicit setting, it is right mouse button for that).

As it is implemented now, the popup selectors (like the equivalent of Shift+i) will also select based on the docker setting, so if you had set the docker to background and forgot about it (maybe even closed the docker), it might be confusing that the popup selector doesn't seem to work until you realize it's changing the wrong color.

rempt added a comment.Jan 11 2021, 1:20 PM

Ah, I get it now. Yes, that's quite confusing.

mwein updated the task description. (Show Details)Feb 5 2021, 12:54 AM
mwein added a comment.Feb 5 2021, 1:20 AM

I think I caught all regressions from porting to master (Krita 5.0), there had been one issue with Gamut Masks I didn't catch initially because it would just fail to connect signals/slots until the code was ported to shared pointers.

I've also found a solution to decouple popup selectors from the foreground/background switch in the docker. My decision to separate the color state from the selector widget paid off here. It just keeps separate states for foreground and background and swaps/clones as necessary.
Right now popup selectors always manipulate the foreground color, but it could be made configurable with a bit of work, if desired.

And the popups now also show the color tooltip that compares current color with previously selected and last used color.

Oh and the keybindings to manipulate Hue, Saturation and Value/Lightness/Intensity/Luma are implemented too, though the increment/decrement is not configurable yet.

mwein updated the task description. (Show Details)Dec 20 2021, 3:21 AM
mwein updated the task description. (Show Details)Jan 4 2022, 1:40 PM
mwein added a comment.Jan 4 2022, 1:49 PM

A summary of what was done since last update:

I moved the actual color set for the color history out of the widget class and made it a bit more efficient to lookup and move entries.
This way the docker and its popup sibling don't have to do some syncing behind the scenes.
I considered to use Model/View classes, but it seemed a bit overkill for the time being. It would be somewhat straight forward by extending it with all the "begin<foo>" type signals and just wrapping the QAbstractItemModel interface around KisUniqueColorSet.

I think we talked about moving the color history to Ko~/KisCanvasResourceProvider, but I haven't done that yet.

Apart from that, I restructured the configuration handling to use static objects in a namespace instead of lots of get/set functions, like the idea Dmitry described here: https://phabricator.kde.org/T14815#262375
It's very basic but helped a lot extending the settings with little code.

With that done, I ported the "Colors from Image" feature of Advanced Color Selector over, but I somewhat doubt the usesfulness of the algorithm as it is, mostly because it averages clusters into a final color. But picking N representative colors from an image seems to be one of those things you can do in many ways with various tradeoffs. So for now it's just a copy&paste and thus limited to sRGB.

Anyway, from the technical side, most functionality of Advanced Color Selector is now ported, the missing things are basically waiting for UI design decisions. Right now the MyPaint shade selector is only available as popup, because I couldn't decide if I want to keep the restriction to have either minimal or MyPaint shade selector or allow both. Also the gamut mask controls are still missing, I don't really like the current way. There should be some way to choose the mask from the docker (has been requested a few times), I'm thinking of a popup/dropdown, and the angle control could be better integrated IMO.

Also the gamut mask controls are still missing, I don't really like the current way. There should be some way to choose the mask from the docker (has been requested a few times), I'm thinking of a popup/dropdown, and the angle control could be better integrated IMO.

I figured this would go into the quick settings you'd made? Either how, please make a MR as soon as possible, I think that'll make it easier for folks to discuss the remaining items.