Avoid NaN saturation when converting black from HSL to HSB#8734
Closed
Chessing234 wants to merge 1 commit intoprocessing:mainfrom
Closed
Avoid NaN saturation when converting black from HSL to HSB#8734Chessing234 wants to merge 1 commit intoprocessing:mainfrom
Chessing234 wants to merge 1 commit intoprocessing:mainfrom
Conversation
_hslaToHSBA computes
val = (1 + sat) * li (for li < 0.5)
val = li + sat - li * sat (otherwise)
sat = 2 * (val - li) / val
When the input is pure black the lightness li is 0, so val is 0 as
well, and the saturation step performs 0 / 0 and returns NaN. The
p5.Color instance then carries a NaN saturation, which breaks any
downstream math or serialization that touches it.
Black has no meaningful hue or saturation, so mirror the grayscale
check used by _rgbaToHSBA and keep saturation at 0 whenever val is 0.
Non-black colors are unaffected.
Member
|
This does not link to an open bug report, so I will close it. Please review contribution guidelines before making future PRs. Thank you! |
This file contains hidden or 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
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.
Bug
ColorConversion._hslaToHSBAreturns a NaN saturation when the inputis pure black. Any p5.Color constructed from an HSL black
(e.g.
color('hsl(0, 50%, 0%)')) ends up withsat = NaNonce it'sconverted to the internal HSBA representation, and downstream
getters/serializers propagate that NaN.
Root cause
For black,
li === 0, and the first branch setsval = (1 + sat) * 0 = 0.The subsequent
2 * (val - li) / valbecomes2 * 0 / 0, which isNaN.Why the fix is correct
conventional value is
0- the same choice_rgbaToHSBAalreadymakes for grayscale (
chroma === 0) inputs elsewhere in the file.val === 0with a ternary keeps theexisting formula unchanged for every non-black input (floating
point
valis only ever exactly0whenli === 0, so theexisting
val > 0code path behaviour is identical).NaNoutput becomes0.Change
src/color/color_conversion.js: replacewith a ternary guard returning
0whenval === 0. One-line fix(plus a brief comment).