Fix conversion from RGB to signed CMYK

[harshitpant1]

Description

Conversion of RGB pixels based on unsigned integral channels to CMYK based on signed integral channels was/is broken (issue #479). It happend because relations used for color correction like c' = (c - k) * s etc. didn't work well for signed integral channels.

This correction is attempted by doing all color correction calculations of pixel values based on unsigned integral channels then(after the calculations are finished) converting those values to signed ones.

References

Principles of Digital Image Processing - Fundamental Techniques (Book name provided in color_convert.hpp)

Tasklist

• Add test case(s)
• Ensure all CI builds pass
• Review and approve

[mloskot]

Thank you for the attempt to fix it.

Please, if you could DO ensure any new features or changes to existing behaviours are covered with test cases. IOW, please add a tests for the bug fix (see some samples in #479).

[harshitpant1]

@mloskot can you please have a look at the tests I just added.

[mloskot]

Good work with the tests!

I added two nitpick comments.

We also need to update the build configuration, so we can build and run the test program locally and with the the CI builds, so if you could do this:

For Boost.Build, add run default_color_converter_rgb_to_cmyk.cpp ; below this line

gil/test/core/color/Jamfile

Line 15 in f6a3553

run default_color_converter_impl.cpp ;

For CMake, remove ) from this line and add default_color_converter_rgb_to_cmyk) below this line:

gil/test/core/color/CMakeLists.txt

Line 11 in f6a3553

default_color_converter_impl)

[mloskot]

Nitpick: trailing space after r should be removed

[mloskot]

Nitpick: missing space before k in //k

[mloskot]

The CI are failing because of

libs\gil\test\core\color\default_color_converter_rgb_to_cmyk.cpp(10):
  fatal error C1083: Cannot open include file: 'test_fixture.hpp':
  No such file or directory

I think you can remove include of this header from the test, as you don't seem to be using any fixture.

[harshitpant1]

I guess I should have been more attentive and avoided that issue/fail.
The CI checks are successful now.
Thanks for your help.

[mloskot]

We all learn from mistakes. As you see, submitting a PR is an effort.

I'm on mobile now and can't verify the result, but I'll take it from the CI, and I'll merge.

Thank you!

[harshitpant1]

Thank you as well.