Skip to content

Wide Gamut Color Selector

Mathias Wein requested to merge mwein/krita:wide-gamut-selector-docker-master into master

See also: https://phabricator.kde.org/T13509 and !1355 (closed)

Let's see how I can recap the code (especially the newer changes) to help review...

Unhappy with the lack of a well defined and somehow encapsulated state of Advanced Color Selector, and with KisVisualColorSelector living in the main widget lib, there needed to be a proper solution to add more UI components in a plugin.

Small Disclaimer:

So far I kept core lib changes to a minimum, for one it helped avoiding conflicts on code rebase, but I also figured deeper integration must wait until we agreed that this design is any good. Also, outside of this new plugin KisVisualColorSelector will still adapt to Advanced Color Selector configuration.

Notable changes in the core libs

I decided to go with a central state object that (after some iterations) is UI agnostic and gets shared by widgets. For this, I split part of KisVisualColorSelector off into KisVisualColorModel (name is debatable, it's neither a Qt item model nor does it refer to color model like HSV, it models the color state and translates its own channel representation to/from KoColor).

All components communicate via this class (signals/slots), preferably by manipulating the float channels of the internal color representation (like HSV channels for an RGB color). This restricts all components to use the same representation, but in turn avoids issues with reconstructing values when UI parts only communicate via KoColor (like poor precision from 8-bit/channels, or hue being undefined for colors with zero saturation). And you can just keep more than one state independent of widgets.

Outside the KisVisualColorModel split-off, there are some recent additions to KisDisplayColorConverter.
Advanced Color Selector does the proofing to painting color space at the earliest possible point, thus each generated KoColor has to be converted separately. I though it's more efficient (and logical) to have the conversion functions to QColor/QImage of KisDisplayColorConverter handle the proofing of the input, so they gained an additional parameter to enable this.
There's also an overload that takes a raw memory block, because copying it all into a KisPaintDevice seemed a lot of overhead too, but admittedly there are more important places to optimize still.

There is one more class I put in the core, KisUniqueColorSet. This is to hold the color history and "colors from image" data, because using a plain QVector, do linear searches and copy the whole thing from docker popup widgets isn't exactly an elegant solution.
It has similarities to KoColorSet, and I would've preferred a common class but the needs just seemed a little to specific to find a nice unified solution.

Plugin code notes

There are basically two main parts that live side by side, the actual WGColorSelectorDock that defines the dock widget, and WGActionManager.

As the name implies, the action manager does everything related to shortcut actions, like the popup widgets, and hotkeys to directly manipulate color. It only fetches the editing state of foreground color on demand and feeds the changes back to the docker, otherwise it's not involved when using the docker.

The rest should look familiar compared to Advanced Color Selector, maybe with the exception of WGConfig that is basically a KisConfig derivative to not have repeating strings, default values and bound checks all over the place.

Notes for Testing

  • To configure the keyboard shortcuts, look for the Krita -> WG Color Selector Actions category.
  • The preexisting "Color Selector Uses Different Color Space than Image" setting also affects this docker now, but only if "Proof Colors to Painting Color Space" is enabled.
    It's a problem these settings are so disconnected, and how the configuration should be integrated in the main "Configure Krita..." dialog needs some thoughts.
  • If you want to select in RGB for CMYK documents, prefer 16-bit RGB color spaces, LCMS produces visible artifacts when coming from 8-bit RGB unless you disable optimizations. You may want to test specifically how Adobe RGB ClayRGB color profile performs (if your display exceeds sRGB gamut, mine doesn't really).

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.

Merge request reports