Theme: better max chroma detection for taperChroma #73625
Merged
+217
−244
Conversation
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
jsnajdr
added
[Type] Performance
|
Size Change: -85 B (0%) Total Size: 2.54 MB
ℹ️ View Unchanged
|
jsnajdr
commented
Nov 27, 2025
| cPlanned = getCachedMaxChromaAtLH( lOut, hSeed, gamutSpace, cap ); | ||
| } | ||
|
|
||
| cPlanned = Math.min( cPlanned, cSeed ); |
Member
Author
There was a problem hiding this comment.
Here I'm removing code that's effectively toGamut( sRGB ) with the lch.c method. We don't need to do the inGamut check, because a call to clampToGamut always follows anyway, and it will do the job.
| const cq = quantize( cap, 1e-3 ); | ||
| const lq = quantize( l, 0.05 ); | ||
| const hq = quantize( normalizeHue( h ), 10 ); | ||
| const cq = quantize( cap, 0.05 ); |
Member
Author
There was a problem hiding this comment.
These quantize params will lead to a 20x36 grid for cached values. The previous step for hue was probably an oversight. The 1e-1 value leads to 0.1 step. That's 3600 steps from 0.0, 0.1, ... to 359.9, 360. Instead, we want 36 steps, 10 degrees each.
If you ever played with the oklch.com color picker, there is a 3D surface plotted:
That's exactly the surface we're calculating here. For each (L,H) point, the maximum chroma value C.
| // Let `toGamut` reduce the chroma to the gamut maximum. | ||
| const clamped = toGamut( probe, { space: gamutSpace, method: 'css' } ); | ||
|
|
||
| return get( clamped, [ OKLCH, 'c' ] ); |
Member
Author
There was a problem hiding this comment.
Much better method to find the max C value that's in gamut. Create a "probe" color with maximum C, and let toGamut to map it into gamut.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
aduth
approved these changes
Dec 1, 2025
Member
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
I appreciate the inline comments to help explain the changes 👍 Some of it is still over my head, but the comments help clarify the intent. A visual audit of both the code changes and visual result LGTM!
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.
This PR is an optimization and cleanup of the
taperChromafunction. It speeds up the color ramp calculation again: my runs of the "buildRamps" unit test show an improvement from 1400ms to 800ms, almost 2x improvement.The
taperChromafunction makes some colors in the color ramp more gray than they would normally be. It's very visible especially with very vivid colors. The following image shows a ramp for pure blue (#00f). The top ramp is withtaperChroma, the lower ramp is without taper. There is big difference especially forfgSurfaceXcolors, and also forstrokeX.And this PR does several changes to
taperChroma:taperChromauses the sRGB gamut instead of P3. sRGB is where the final colors always are, so there's no point using P3.taperChromano longer does any gamut mapping (toGamut). That's a separate step happening aftertaperChroma. Previously we were doing the gamut mapping twice.toGamutCSSto clamp a color with too high chroma to an in-gamut value. You can read more details in this colorjs.io doc: https://colorjs.io/docs/gamut-mapping. The doc describes a "bad"lch.calgorithm, and that's exactly whattaperChromawas using previously. And it describes the "good" CSS algorithm, and that's what we're using now 🙂PlainColorObjecttype internally, instead of the more genericColorTypes. That's mainly becausecloneaccepts onlyPlainColorObjects.The net result is a much faster and cleaner
taperChroma, and imperceptible differences in generated colors.