Skip to content

Usability improvements for the KisVisualColorSelector

Mathias Wein requested to merge mwein/krita:lynx-color-sel into master

This branch mainly tries to fix some shortcomings of the "internal" color selector used by the dialogs to select foreground and background colors, and optionally in the popup palette.

Overview of things it addresses:

  • Color handles not following the cursor smoothly for a number of reasons.
  • Not reacting at all when the mouse gets dragged outside the selector area, which makes it hard to select colors on the edges/corners.
  • In linear RGB color spaces, the colors were distributed quite differently than in sRGB, giving the darker shades much less area than the light ones, due to the non-linear nature of our eyes.
  • Inability to deal with RGB values outside the [0, 1] range.
  • A number of bugs like dialog layout issues, incorrect coordinate conversions an possible crashes (the appimages regularly crash on me from color selector dialog, but unfortunately I could never reproduce it with my builds).
  • Rectangular selection areas were upside down compared to the Advanced Color Selector, this was changed to match the two.

Overview of how things were addressed:

  • the KisVisualColorShape class was essentially made as dumb as possible, only following the given channel values by its parent, the actual selector widget. This resolves issues with situations like undefined hue when saturation is zero, the shape controlling saturation/value simply does not need to touch hue nor need to care if hue has an effect.
  • A number of performance improving changes:
    • Since the shapes' backgrounds are always determined by the "slave" channels it does not control, there is no need to re-render its background on manipulations, only the background of its sibling.
    • Especially the ring selector shapes were rendering a lot of pixels that are going to be masked anyway, so there's now more specialized rendering for the background.
    • Only re-apply widget mask when widget size changes, not on every paint event.
    • Avoid heap allocated structures like QVector and use QVector4D where possible.
  • KoColorProfile was enhanced with a function to query if it has linear transfer response curves or not. When linear, the selector applies a gamma of 2.2, to keep the familiar shade distribution.
  • The HDR handling (OCIO engine with exposure control in the LUT docker) was changed, the selector now auto-scales colors to match the display range, that means the selector keeps its visual brightness on exposure changes and instead moves the handles accordingly, allowing you to always access the currently visible range rather than being stuck in [0, 1] RGB range.
  • The attempts of signal compressing in the selector were removed, instead the selector dialog's compressor is now used properly to postpone excess outward signalling, but some more tweaking may be desired here. (The popup palette already had a working compressor)

Remaining issues:

  • The L*a*b* color space currently is a mess, the range of "normalized" channel values is not a [0, 1] range, and to make it worse it differs depending on the image's bit depth, and there doesn't seem to be a way to query this range properly either. This needs some clarification what normalized L*a*b* is supposed to be, I can't make sense of it to be honest.
  • The display conversion of float-32 L*a*b* seems broken too, I was only able to add a fallback and show a black selector instead of having the widget painting bail out with error messages by trying to paint on a Null QImage.

Test Plan

  • Set "popuppalette/usevisualcolorselector=true" in kritarc to test the selector in the popup plette aswell.
  • Test picking colors from the foreground/background color selecting dialog in as many color models and bit depths as possible.
  • Test the various selector option in the advanced color selector, because they affect this selector too.
  • Pay attention if the selector properly updates to changes from other color pickers.
  • Make sure the color picked always matched the shade you clicked in the selector.

IMPORTANT: L*a*b* color space will still have issues, but there should be no regressions over the updstream code. Also I couldn't test YCbCr at all due to the lack of profiles.

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.
Edited by Mathias Wein

Merge request reports