HSL lightness is not luminance — the dark floor was too low
The unit test I added with the generated fills failed on its first run, on exactly the claim it was written to check, so it earned its keep immediately. The floor was 0.090 — `neutral-900`'s own HSL lightness — reasoning that a ramp starting at the card surface and climbing could not end up below it. That confuses HSL lightness with luminance. At one fixed lightness the eye sees very different brightnesses by hue, because green carries 71% of the luminance formula and blue only 7%: at L=0.090 a yellow measures 0.0118 and a blue 0.0061. Every blue-ish untagged note was 1.41x DARKER than the card it was supposed to match, which on the board reads as a hole rather than as variety — the opposite of what the whole change is for. Solved rather than nudged: 0.113 is the lowest floor at which EVERY hue clears the card surface. The range now measures 1.11-1.71 against the board against the old 1.06-1.54, so the floor is back where the shipped ramp had it and the ceiling is higher. Body text 7.8 against the 4.5 it needs, meta 4.6 against 3.0. 338 distinct dark fills. Two things about the test are worth keeping. It asserts on LUMINANCE rather than on the lightness that was put in — a test of the input would have agreed with the bug and passed. And it now sweeps 40,000 ids rather than 500. The worst case is a HUE, not an id, and 500 ids reach only 459 of the 2160 hue/level combinations — it caught this one by luck. 40,000 covers all 2160. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -158,7 +158,7 @@ fun noteColorIsChosen(
|
||||
// at most a 1.03 contrast ratio, which is to say not at all. Nine tints that look
|
||||
// like three is exactly the wall the tint was added to break up.
|
||||
//
|
||||
// So this hashes to a colour directly rather than to a key. 324 distinct fills in
|
||||
// So this hashes to a colour directly rather than to a key. 338 distinct fills in
|
||||
// dark, 193 in light, against nine.
|
||||
//
|
||||
// TWO AXES, AND THE SECOND ONE IS THE FIX. The old ramp varied hue while pinning
|
||||
@@ -185,12 +185,23 @@ private const val DARK_SATURATION = 0.25
|
||||
private const val LIGHT_SATURATION = 0.60
|
||||
|
||||
/**
|
||||
* Dark starts at 0.090 — a hair under `neutral-900`, the plain card surface — and
|
||||
* climbs. Nothing is ever darker than an untinted card, so no note recedes into the
|
||||
* board; they only ever rise off it. Top of the range measures 1.54 against the
|
||||
* board where the old single level managed 1.14.
|
||||
* HSL LIGHTNESS IS NOT LUMINANCE, and the floor here is set by the difference.
|
||||
*
|
||||
* The obvious floor is `neutral-900`'s own lightness, 0.090 — "start at the plain card
|
||||
* surface and climb, so no note ever recedes into the board". That was the first
|
||||
* attempt and it was wrong, because at a FIXED HSL lightness the eye sees wildly
|
||||
* different brightnesses depending on hue: green carries 71% of the luminance formula
|
||||
* and blue only 7%, so at L=0.090 a yellow measures 0.0118 and a blue 0.0061 — the
|
||||
* blue landing 1.41x DARKER than the card it was supposed to match. A sixth of the
|
||||
* board would have been holes rather than variety.
|
||||
*
|
||||
* 0.113 is the lowest floor at which EVERY hue clears the card surface, solved for
|
||||
* rather than guessed. The range then measures 1.11–1.71 against the board where the
|
||||
* old single level managed 1.14, so the floor is unchanged and the ceiling is much
|
||||
* higher. `no generated fill sinks below the card surface` in DerivedTintTest is what
|
||||
* caught the original mistake and what holds this.
|
||||
*/
|
||||
private val DARK_LIGHTNESS = doubleArrayOf(0.090, 0.104, 0.118, 0.132, 0.146, 0.160)
|
||||
private val DARK_LIGHTNESS = doubleArrayOf(0.113, 0.127, 0.141, 0.155, 0.169, 0.183)
|
||||
|
||||
/**
|
||||
* Light runs the other way, from white down toward the `neutral-50` board and just
|
||||
|
||||
@@ -196,10 +196,10 @@ class DerivedTintTest {
|
||||
* actually compared. The same four ids and hexes are a comment in colors.ts. */
|
||||
@Test
|
||||
fun `generated fills match the fixture shared with the web`() {
|
||||
assertEquals("#192a29", hex(derivedFillArgb("00000000-0000-0000-0000-000000000000", dark = true)))
|
||||
assertEquals("#152114", hex(derivedFillArgb("11111111-1111-1111-1111-111111111111", dark = true)))
|
||||
assertEquals("#111d14", hex(derivedFillArgb("6ba7b810-9dad-11d1-80b4-00c04fd430c8", dark = true)))
|
||||
assertEquals("#2a191b", hex(derivedFillArgb("f47ac10b-58cc-4372-a567-0e02b2c3d479", dark = true)))
|
||||
assertEquals("#1e3130", hex(derivedFillArgb("00000000-0000-0000-0000-000000000000", dark = true)))
|
||||
assertEquals("#1a2818", hex(derivedFillArgb("11111111-1111-1111-1111-111111111111", dark = true)))
|
||||
assertEquals("#162419", hex(derivedFillArgb("6ba7b810-9dad-11d1-80b4-00c04fd430c8", dark = true)))
|
||||
assertEquals("#311e20", hex(derivedFillArgb("f47ac10b-58cc-4372-a567-0e02b2c3d479", dark = true)))
|
||||
|
||||
assertEquals("#f3fcfb", hex(derivedFillArgb("00000000-0000-0000-0000-000000000000", dark = false)))
|
||||
assertEquals("#fbfefb", hex(derivedFillArgb("11111111-1111-1111-1111-111111111111", dark = false)))
|
||||
@@ -234,14 +234,35 @@ class DerivedTintTest {
|
||||
assertEquals(true, levels.max() / levels.min() > 2.0)
|
||||
}
|
||||
|
||||
/** Nothing may be darker than the plain card surface (`neutral-900`, #171717) in
|
||||
* dark, or the note recedes into the near-black board instead of sitting on it. */
|
||||
/**
|
||||
* Nothing may be darker than the plain card surface (`neutral-900`, #171717) in
|
||||
* dark, or the note reads as a hole in the board rather than a card on it.
|
||||
*
|
||||
* THIS TEST HAS ALREADY EARNED ITS KEEP. The first floor was 0.090 — `neutral-900`'s
|
||||
* own HSL lightness — on the reasoning that starting at the card surface and
|
||||
* climbing could not possibly go below it. That reasoning confuses HSL lightness
|
||||
* with luminance. At one fixed lightness the eye sees very different brightnesses
|
||||
* by hue, because green carries 71% of the luminance formula and blue 7%: at 0.090
|
||||
* a yellow measures 0.0118 and a blue 0.0061, so every blue-ish untagged note was
|
||||
* 1.41x darker than the card it was supposed to match. Solved for, the floor is
|
||||
* 0.113.
|
||||
*
|
||||
* Which is why this asserts on LUMINANCE and not on the input lightness — testing
|
||||
* the number that was put in would have agreed with the bug.
|
||||
*/
|
||||
@Test
|
||||
fun `no generated fill sinks below the card surface`() {
|
||||
val surface = luminance(0xFF171717.toInt())
|
||||
for (n in 0 until 500) {
|
||||
val l = luminance(derivedFillArgb("note-$n", dark = true))
|
||||
assertEquals(true, l >= surface * 0.98)
|
||||
assertEquals(true, luminance(derivedFillArgb("note-$n", dark = true)) >= surface)
|
||||
}
|
||||
// The worst case is a HUE — blue, around 240 — and 500 ids are not enough to
|
||||
// be sure of having visited it at the darkest level. There are 360 x 6 = 2160
|
||||
// reachable combinations; by the coupon-collector bound ~16,600 draws covers
|
||||
// them, so 40,000 makes a miss vanishingly unlikely. Cheap on the JVM, and the
|
||||
// alternative is a test that would have PASSED against the original bug.
|
||||
for (n in 0 until 40_000) {
|
||||
assertEquals(true, luminance(derivedFillArgb("hue-sweep-$n", dark = true)) >= surface)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user