ArtColorSel: KisColor refactoring
ClosedPublic

Authored by amedonosova on Sep 16 2018, 3:51 PM.

Details

Summary

KisColor now uses KisDisplayColorConverter and it's functions, like Advanced Color Selector. The color spaces behave like Advanced Selector's. That solves small issues with the selector's original color spaces and also enables the benefits of color management.

The selector accepts and sets fg and bg color as KoColor.

Diff Detail

Repository
R37 Krita
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
amedonosova created this revision.Sep 16 2018, 3:51 PM
Restricted Application added a reviewer: Krita. · View Herald TranscriptSep 16 2018, 3:51 PM
Restricted Application added a project: Krita. · View Herald Transcript
amedonosova requested review of this revision.Sep 16 2018, 3:51 PM
amedonosova edited the summary of this revision. (Show Details)Sep 17 2018, 5:27 AM
dkazakov requested changes to this revision.Sep 17 2018, 11:14 AM

Hi, @amedonosova!

I have a feeling that it works incorrectly. As if it applies a reversed color transformation. When I make exposure higher, the color in the color selector becomes darker... It looks very wrong...

Here is a video:

This revision now requires changes to proceed.Sep 17 2018, 11:14 AM
amedonosova edited the summary of this revision. (Show Details)
  • ArtColorSel: fixes conversion to QColor
  • ArtColorSel: implements proper luma coefficients for HSY

Value scale numbers for HSY are broken.

I have a feeling that it works incorrectly. As if it applies a reversed color transformation. When I make exposure higher, the color in the color selector becomes darker... It looks very wrong...

The problem was in conversion from KisColor to QColor. I used KoColor::toQColor, which gave wrong results. It needs to be done through the display converter instead.

amedonosova edited the summary of this revision. (Show Details)Sep 18 2018, 2:48 PM
amedonosova added a comment.EditedSep 18 2018, 3:10 PM

What remains is to fix the value scale numbering.

The goal is to emulate a "traditional feel" of printed numbered value scales.

normal, usual numbering

modified

dkazakov requested changes to this revision.Sep 19 2018, 10:22 AM

Hi, @amedonosova!

The exposure is half working now :) It renders the color selector correctly, but it looks like color picking from a color corrected selector selects non-color-corrected color. I remember there was a special "convert-color-backwards" system for that. I guess it is relevant. Here s a video:

There is also a small crash when closing the document and then trying to create a new document afterwards :)
https://paste.kde.org/psocywf4l

This revision now requires changes to proceed.Sep 19 2018, 10:22 AM
  • ArtColorSel: fixes value scale numbering
  • ArtColorSel: fixes a crash on canvas set/unset

I have yet to fix the color issues.

amedonosova updated this revision to Diff 41959.EditedSep 20 2018, 6:24 AM
  • ArtColorSel: fix color update on outside change

This change also seems to fix the exposure problem. Is the current behavior correct? Are there other things I should test/implement before it's ready to merge?

This comment was removed by lsegovia.
amedonosova edited the summary of this revision. (Show Details)Sep 22 2018, 4:44 AM
dkazakov accepted this revision.Sep 25 2018, 3:26 PM

Hi, @amedonosova!

Now the patch works perfectly fine, please push! Thank you for all your work you do for the artistic color selector! :)

This revision is now accepted and ready to land.Sep 25 2018, 3:26 PM
amedonosova edited the summary of this revision. (Show Details)Sep 26 2018, 5:28 AM
This revision was automatically updated to reflect the committed changes.