Bug 254784

Thanks for your report. This is an artefact of the way we currently compose the displayed image when we enable/disable channels. It's not easy to fix, but I agree that the current view is not useful, so we need to think about it.

I thought this was fixed, I think I even mentioned it at the sprint?

Basically there's an option in configure Krita->Display that allows you to set whether you prefer colour channels, if there's only one of them, to be greyscale or in color.

Unless this person is reffering to the active channels(in properties), instead of enabled channels(in channel docker).

I think the bug is still valid, right now, when I enable 'show channels in color', I will get a blue image for the L channel, not a gray one, and when I disable that option, I will get the orange/blue result Pat describes.

But I still don't know how to fix it...

Renamed the bug into something more understandable

Ah, I just looked at this a little, and understand half of the problem now:

In Lab, the assumption is that when a and b are inactive, they aren't black, but greyscale. (because they go from -0.5 to +0.5, so to speak.)

If you put a full mid-tone layer behind the image layer, and then disable the channels, everything looks correct.

(Except for the part where the 'show single channels as color' doesn't seem to make a difference...)

Ok, I am enclosing a pseudo-patch here, because I am not sure how to go about the stuff in the comments:


diff --git a/libs/ui/opengl/kis_texture_tile_update_info.h b/libs/ui/opengl/kis_texture_tile_update_info.h
index 60900c9d58..42cd777e13 100644
--- a/libs/ui/opengl/kis_texture_tile_update_info.h
+++ b/libs/ui/opengl/kis_texture_tile_update_info.h
@@ -161,6 +161,8 @@ public:
     void retrieveData(KisPaintDeviceSP projectionDevice, const QBitArray &channelFlags, bool onlyOneChannelSelected, int selectedChannelIndex)
     {
         m_patchColorSpace = projectionDevice->colorSpace();
+        //This seems like a dangerous thing to do at this place...?
+        bool isLab = m_patchColorSpace->colorModelId() == "LABA";
         m_patchPixels.allocate(m_patchColorSpace->pixelSize());
 
         projectionDevice->readBytes(m_patchPixels.data(),
@@ -185,6 +187,21 @@ public:
                     for (uint channelIndex = 0; channelIndex < m_patchColorSpace->channelCount(); ++channelIndex) {
 
                         if (channelInfo[channelIndex]->channelType() == KoChannelInfo::COLOR) {
+                            if (isLab) {
+                                //if we're in lab, only copy the data into the first color channel(l), and the others filled with gray.
+                                if (channelIndex==0) {
+                                    memcpy(conversionCache.data() + (pixelIndex * pixelSize) + (channelIndex * channelSize),
+                                           m_patchPixels.data() + (pixelIndex * pixelSize) + selectedChannelPos,
+                                           channelSize);
+                                } else {
+                                    //something that fills the section with lab (0.5, 0.5, 0.5)
+                                }
+
+                            } else {
+                                memcpy(conversionCache.data() + (pixelIndex * pixelSize) + (channelIndex * channelSize),
+                                       m_patchPixels.data() + (pixelIndex * pixelSize) + selectedChannelPos,
+                                       channelSize);
+                            }
                             memcpy(conversionCache.data() + (pixelIndex * pixelSize) + (channelIndex * channelSize),
                                    m_patchPixels.data() + (pixelIndex * pixelSize) + selectedChannelPos,
                                    channelSize);
@@ -204,6 +221,8 @@ public:
                             memcpy(conversionCache.data() + (pixelIndex * pixelSize) + (channelIndex * channelSize),
                                    m_patchPixels.data() + (pixelIndex * pixelSize) + (channelIndex * channelSize),
                                    channelSize);
+                        } else if (isLab) {
+                            // fill the section with lab (0.5, 0.5, 0.5)
                         }
                         else {
                             memset(conversionCache.data() + (pixelIndex * pixelSize) + (channelIndex * channelSize), 0, channelSize);

ok, my previous comment is incorrect: this code is only relevant for the channels docker. -_-

I don't know where the paintdevices' channel compositing is happening, but what does happen there is that it should have the deactivated channels treated as lab(128, 128, 128) for 8bit and lab(50, 0, 0) for the rest... Or single channels get depicted as greyscale, always, lik dmitry suggested.

(In reply to wolthera from comment #7)
> ok, my previous comment is incorrect: this code is only relevant for the
> channels docker. -_-
> 
> I don't know where the paintdevices' channel compositing is happening, but
> what does happen there is that it should have the deactivated channels
> treated as lab(128, 128, 128) for 8bit and lab(50, 0, 0) for the rest... Or
> single channels get depicted as greyscale, always, lik dmitry suggested.

Actually, testing now, this section should probably be edited for the same reasons as well, I just can't figure out how to do it without crashing everything.

Assigning to myself (https://invent.kde.org/kde/krita/merge_requests/251).

Git commit 0a8a1c69059a0977cad5610d0b791165231a6cda by L. E. Segovia.
Committed on 03/03/2020 at 14:30.
Pushed by lsegovia into branch 'amyspark/254784-fix-lab-channel-canvas'.

Fix display of Lab channels

This is a horrible hack to manually set channel values when they are
disabled. Usually, they are left to 0, but Lab requires them to be set
to the corresponding halfValue.

Thanks to Wolthera for the initial pseudopatch -- the missing part was
the canvas code.

M  +136  -5    libs/ui/canvas/kis_image_pyramid.cpp
M  +147  -8    libs/ui/opengl/kis_texture_tile_update_info.h

https://invent.kde.org/kde/krita/commit/0a8a1c69059a0977cad5610d0b791165231a6cda

Pushed to the wrong repository, the MR still needs to be merged.

Git commit 7261d7bd192bcd0b555a0f5d2db1e10c760cc9a9 by L. E. Segovia.
Committed on 05/03/2020 at 01:30.
Pushed by lsegovia into branch 'master'.

Fix Lab channel render in canvas and Channels docker

This moves the channel rendering code to a KoColorSpace function
named convertChannelToVisualRepresentation, implemented in
KoColorSpaceAbstract and overriden in Lab colorspaces.

This function now handles copying channel values and, where necessary,
renormalizes it so as to fit the valid channel range (as with Lab).

As a bonus, I also fixed scaleToU8 in Lab because it needed
the same renormalization procedure.

Thanks to Wolthera for the initial pseudopatch -- the missing part was
the canvas code.

M  +28   -0    libs/pigment/KoColorSpace.h
M  +31   -0    libs/pigment/KoColorSpaceAbstract.h
M  +3    -0    libs/pigment/KoColorSpaceTraits.h
M  +88   -0    libs/pigment/colorspaces/KoLabColorSpace.cpp
M  +3    -0    libs/pigment/colorspaces/KoLabColorSpace.h
M  +2    -32   libs/ui/canvas/kis_image_pyramid.cpp
M  +8    -38   libs/ui/opengl/kis_texture_tile_update_info.h
M  +88   -0    plugins/color/lcms2engine/colorspaces/lab_f32/LabF32ColorSpace.cpp
M  +3    -0    plugins/color/lcms2engine/colorspaces/lab_f32/LabF32ColorSpace.h
M  +88   -0    plugins/color/lcms2engine/colorspaces/lab_u16/LabColorSpace.cpp
M  +3    -0    plugins/color/lcms2engine/colorspaces/lab_u16/LabColorSpace.h
M  +88   -0    plugins/color/lcms2engine/colorspaces/lab_u8/LabU8ColorSpace.cpp
M  +3    -0    plugins/color/lcms2engine/colorspaces/lab_u8/LabU8ColorSpace.h

https://invent.kde.org/kde/krita/commit/7261d7bd192bcd0b555a0f5d2db1e10c760cc9a9

Git commit effe8431cdb0c08618ddce879740f7f6abd1e9e3 by L. E. Segovia.
Committed on 05/03/2020 at 13:37.
Pushed by lsegovia into branch 'krita/4.2'.

Fix Lab channel render in canvas and Channels docker

This moves the channel rendering code to a KoColorSpace function
named convertChannelToVisualRepresentation, implemented in
KoColorSpaceAbstract and overriden in Lab colorspaces.

This function now handles copying channel values and, where necessary,
renormalizes it so as to fit the valid channel range (as with Lab).

As a bonus, I also fixed scaleToU8 in Lab because it needed
the same renormalization procedure.

Thanks to Wolthera for the initial pseudopatch -- the missing part was
the canvas code.

M  +28   -0    libs/pigment/KoColorSpace.h
M  +31   -0    libs/pigment/KoColorSpaceAbstract.h
M  +3    -0    libs/pigment/KoColorSpaceTraits.h
M  +88   -0    libs/pigment/colorspaces/KoLabColorSpace.cpp
M  +3    -0    libs/pigment/colorspaces/KoLabColorSpace.h
M  +2    -32   libs/ui/canvas/kis_image_pyramid.cpp
M  +8    -38   libs/ui/opengl/kis_texture_tile_update_info.h
M  +88   -0    plugins/color/lcms2engine/colorspaces/lab_f32/LabF32ColorSpace.cpp
M  +3    -0    plugins/color/lcms2engine/colorspaces/lab_f32/LabF32ColorSpace.h
M  +88   -0    plugins/color/lcms2engine/colorspaces/lab_u16/LabColorSpace.cpp
M  +3    -0    plugins/color/lcms2engine/colorspaces/lab_u16/LabColorSpace.h
M  +88   -0    plugins/color/lcms2engine/colorspaces/lab_u8/LabU8ColorSpace.cpp
M  +3    -0    plugins/color/lcms2engine/colorspaces/lab_u8/LabU8ColorSpace.h

https://invent.kde.org/kde/krita/commit/effe8431cdb0c08618ddce879740f7f6abd1e9e3