Zoom preview when using circle color sampler (Pixel-perfect color sampler)

Dat Lerequested to merge
dootdoot/krita:dootdoot/percisecolorsampler into master

This patch add the feature to zoom on canvas / reference image when using the circle color sampler and also the ability to change its position. Because the center of the circle color sampler is now filled and not hollow, it's now possible to change the position of the circle color sampler. This is helpful when using touch gesture to sample color so you can actually see what you're pointing at.

Feature enable/disable, zoom scale, circle color sampler position can be configured through Configure Krita->General->Cursor. I added a ScrollArea because it's VERY cramped in there.

image.png

Disabled Enabled, 5x zoom, position center Enabled, 10x zoom, position center
image.png image.png image.png
Position top Position top left Position top right
image.png image.png image.png

Because there's no cursor when the circle color sampler is offsetted, I draw a simple vector crosshair in the center. Its color can change from black to white on darker color .It's calculated by the fomula: luminance = 0.299 * redF + 0.587 * greenF + 0.114 * blueF, which is allegedly a bit inaccurate, but the more accurate fomula requires calculating power, so pass. It's probably would be overkill anyways. using KisPaintingTweaks::luminosityCoarse()

Dark Light
image.png image.png

This patch also fix a minor inaccuracy when sampling color because of back and forth coordinate converting from docPoint->imagePoint->docPoint, see fix commit dootdoot/krita@4188390d

Before After
image.png image.png

A note on memory usage:

Sampling from reference image is really fast because I can get the QImage directly. Sampling from canvas though, requires using KisImage::convertToQImage(), which makes copies. I try to avoid calling this rapidly when user move around by caching an area that's 4x of the sampled area if the sampled area is under 250x250, 2x if under 500x500 pixel, and 1x (no cache) if larger (Maybe this should change, but memory usage seems sensitive so I choose to sacrifice CPU time?). Due to the usage of KisDisplayColorConverter::convertImageToDisplayColorSpace() for correct color profile, performance has tanked quite a lot, I can no longer afford caching 😥

Sampled area is calculated by (circle sampler size in pixel) / (zoom scale factor). In worst case, if the cache (or sampled) area is bigger than the canvas, the whole canvas is sampled and cached until color sampling stop.

So, the bigger the scale factor is, the more performance and less RAM this feature will use. The default is 5x 10x to improve performance. Maybe I really should put a note for user in the settings?

Test Plan

  1. Enable the feature
  2. Sample color from canvas, check if the preview is shown and zoomed to the configured scale, the preview center is same as the sampled color
  3. Rotate the canvas, check if the preview rotate
  4. Add reference images, check if preview shown for reference images
  5. Change reference image saturation setting, check if preview is same as the reference image
  6. Layer two reference images, check if preview show the top reference image
  7. Change opacity of the top reference image to 0, check if preview show the reference image underneath
  8. Rotate reference image, check if preview rotate and still accurate
  9. Change the circle color sampler position, check if the position is correct.
  10. Rotate canvas and check if the position and crosshair of preview is not rotated
  11. Disable the feature, check that preview is not shown

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.
  • Does the patch add a user-visible feature? If yes, is there a documentation MR ready for it at Krita Documentation Repository?

Reminder: the reviewer is responsible for merging the patch, this is to ensure at the least two people can build the patch. In case a patch breaks the build, both the author and the reviewer should be contacted to fix the build. If this is not possible, the commits shall be reverted, and a notification with the reasoning and any relevant logs shall be sent to the mailing list, kimageshop@kde.org.

Edited by Dat Le

Merge request reports