Fix invalid conversion from RGB8 to CMYK32 due to overflow #470
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mloskot
added
cat/bug
mloskot
force-pushed
the
ml/fix-rgb-to-cmyk32-fp-overflow
branch
from
f8ded25
to
0e0e5b1
Compare
mloskot
changed the title
mloskot
force-pushed
the
ml/fix-rgb-to-cmyk32-fp-overflow
branch
4 times, most recently
from
4390df4
to
bc52e32
Compare
mloskot
added
status/work-in-progress
|
The fix is still not correct as it does not convert well for negative values of signed pixel types like Basically, RGB color value |
Correct calculation to correctly map CMYK 0 to minimum and 1 to maximum of input and output channel types. If float-point division is necessary, use double instead of float which may be too narrow for large operands. cmyk32_pixel_t is based on uint32_t which multiplication by its maximum may yield result too large to fit 32-bit float. For example, (uint32_t(c) - uint32_t(k)) * float(s) for c = 4294967295, k = 0, s = 1.0 results in 4294967300 value which does not fit uint32_t. Fixes boostorg#406
mloskot
force-pushed
the
ml/fix-rgb-to-cmyk32-fp-overflow
branch
from
bc52e32
to
e545e38
Compare
mloskot
removed
the
status/work-in-progress
mloskot
added a commit
that referenced
this pull request
Jul 11, 2020
* develop: Collect release notes for Boost 1.74 Fix missing header in numeric/kernel.hpp to make it self-contained (#502) Use perfect forwading from apply_operation to visit (#491) Fix typos and replace mpl with mp11 in tutorial (#494) Implemented mechanism to reverse kernel_2d (#489) added missing const& in extend_boundary parameters (#490) Add initializing image constructor (#486) Fix interleaved_view factory using point<std::ptrdiff_t> for dimension (#487) RELEASES.md: Remove beta tag [ci skip] Replace Boost.Variant with Boost.Variant2 (#474) Fix error plane_view_t is not a class or namespace name (#481) Fix image constructor from other image (#477) Fix overflow in RGB to CMYK32 conversion (#470)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Correct calculation to correctly map CMYK 0 to minimum and 1 to maximum of input and output channel types.
If float-point division is necessary, use double instead of float which may be too narrow for large operands.
cmyk32_pixel_tis based onuint32_twhich multiplication by its maximum may yield result too large to fit 32-bit float.For example,
(uint32_t(c) - uint32_t(k)) * float(s)forc = 4294967295, k = 0, s = 1.0results in
4294967300value which does not fituint32_t.FIXME: The original calculation was broken for signed CMYK target types. This has not been corrected yet, see #479
References
Tasklist
currently there are no related tests, to be added