Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversion of any RGB pixel to signed CMYK conversion is broken #479

Closed
mloskot opened this issue Apr 10, 2020 · 3 comments
Closed

Conversion of any RGB pixel to signed CMYK conversion is broken #479

mloskot opened this issue Apr 10, 2020 · 3 comments
Labels
cat/bug But reports and bug fixes good-first-issue Opportunity for new contributors to help improving GIL
Milestone

Comments

@mloskot
Copy link
Member

mloskot commented Apr 10, 2020

RGB pixels based on unsigned integral channels are not correctly translated to CMYK space with full range of signed integral channels.

The original implementation did not handle properly signed CMYK pixels, so this issue is independent from #406.

C++ Minimal Working Example

#include <boost/gil.hpp>
namespace gil = boost::gil;
int main()
{
    // black
    {
        gil::rgb8_pixel_t src{0, 0, 0};
        gil::cmyk8s_pixel_t dst;
        gil::color_convert(src, dst); // dst={0, 0, 0, 0}
                             // expected dst={-128, -128, -128, 127}
    }
    // white
    {
        gil::rgb8_pixel_t src{255, 255, 255};
        gil::cmyk8s_pixel_t dst;
        gil::color_convert(src, dst); // dst={0, 0, 0, -128}
                             // expected dst={-128, -128, -128, -128}
    }
    // blue
    {
        gil::rgb8_pixel_t src{0, 0, 255};
        gil::cmyk8s_pixel_t dst;
        gil::color_convert(src, dst); // dst={127, 127, 0, -128}
                             // expected dst={127, 127, -128, -128}
        print(1, src, dst);
    }
    // yellow
    {
        gil::rgb8_pixel_t src{255, 255, 0};
        gil::cmyk8s_pixel_t dst;
        gil::color_convert(src, dst); // dst={0, 0, 127, -128}
                             // expected dst={-128, -128, 127, -128}
    }
}

References

Environment

  • Version (Git ref or <boost/version.hpp>): <=1.72, current develop
@mloskot mloskot added the cat/bug But reports and bug fixes label Apr 10, 2020
@mloskot mloskot reopened this Apr 10, 2020
@mloskot mloskot added the good-first-issue Opportunity for new contributors to help improving GIL label Apr 10, 2020
@Vivekyadavgithub
Copy link

this is my first time . Can u give me some pointers to fix this bug

@mloskot
Copy link
Member Author

mloskot commented Aug 14, 2020

Can u give me some pointers to fix this bug

I'm not sure what kind of pointers you may need.
The bug is described quite clearly.
The bug description is accompanied with minimal, reproducible example which clearly points where is the problem (actual output vs expected output).

The task list is quite obvious, for example, it could be:

  1. Compile the program
  2. Run it, observe the actual vs expected output.
  3. Add test case(s) confirm the bug (it can be based on the minimal example provided).
  4. Start debugging it and reading the source code of the implementation
  5. Find out why the conversion does not output expected results.
  6. Fix it.
  7. Confirm the tests pass, add more test cases...
  8. Submit PR and request review
  9. Address reviewers' comments and requests
  10. Get the PR merged
  11. Go for a coffee or a pint of ale and celebrate the great achievement!

mloskot pushed a commit that referenced this issue Oct 10, 2020
* Fix conversion of rgb to signed cmyk(#479)

* changed naming of dst_us_t to uint_t and undid the formatting change

* test for conversion of rgb to cmyk, PR #522.

* small formatting changes/fixes

* Build configuration update for PR #522

* removed unused header file
@mloskot
Copy link
Member Author

mloskot commented Oct 10, 2020

Fixed by #522

@mloskot mloskot added this to the Boost 1.75+ milestone Oct 10, 2020
@mloskot mloskot closed this as completed Oct 10, 2020
@mloskot mloskot modified the milestones: Boost 1.75, Boost 1.76+ Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat/bug But reports and bug fixes good-first-issue Opportunity for new contributors to help improving GIL
Projects
None yet
Development

No branches or pull requests

2 participants