fix: cleaning 2/12 — every pixel path reads the page in sRGB, on white paper, at 8 bits a channel #164
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cleaning-canonical-decode"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is slice 2 of Phase 5 in
PROJECT_PLAN.md: ADR 0027's "One canonical decode feeds every pixel path" (plan PR #162). It also closes the deferred "flatten alpha before the JPEG" bug from PR #108.What's in
Infrastructure (
SkiaImages.DecodeAsync) is the one decode helper behind every pixel path: the agents' views and crops,/thumb, the ruling views and the colour sampler. It now does four things:/thumbused to carry alpha into their PNG, and the adapter's JPEG re-encode then turned the paper black. Only the gridded view and the contact sheet escaped, because they draw on a white canvas of their own.Cleanup. The ink measure and the colour sampler no longer guard against transparency, since the decode makes that state impossible.
Tests
1,319 green, against 1,304 on main: Integration 294 (+15); Domain 129, UseCases 633 and BlazorAdapter 263 are unchanged. Every project ran separately under the 60 s cap. What the tests pin:
System.IO.Hashing.Only one existing expectation moved. A fully transparent box used to be refused by the sampler, and now reads #ffffff, because transparent paper is white. No renderer value moved, because those fixtures are opaque, untagged 8-bit images.
Honest notes
round(v/257)can differ from this by one level. Neither clips; the tests use samples where both rules agree.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.2%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.7%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Colour-management code with hand-computed expected values and fixtures that verify themselves — this is the kind of PR I want to take apart page by page~ ♡ And take it apart I did: every changed file whole, every sibling traced, every hand-worked number recomputed from the published colour spaces.
Verdict: ✅ Looks good to me~
The one-decode-behind-every-pixel-path claim is structurally true, not just aspirational: I swept all of
src/and the only places that touch pixels are the three files this PR edits (SkiaImages,SkiaPageImageRenderer,SkiaRegionColorSampler).FileSystemPageImageStore'sSKCodec.Createis header-only size probing — correctly out of scope. And the deferred #108 "flatten alpha before the JPEG" bug dies here, pinned through the real adapter re-encode (A_transparent_page_reaches_the_model_on_white_paperdecodes the actual base64 JPEG off the wire — fufu, you didn't even take the shortcut of trusting the PNG view~).✅ What I liked~
Near(stored, plain.GetPixel(20, 20), lossy)proves the file holds what it claims before asserting what the decode does with it. Test discipline like that makes me giddy ♪System.IO.Hashing) instead of "trust the encoder" — and the encoder genuinely can't produce these inputs (no 16-bit grey, no gAMA from SkiaSharp), so the helper earns its existence.IsSrgbis cleverer (and more robust) than parsing XYZ numbers. Oh! Wonderful~sampled == 0refusal became a branch no input could ever reach (a clamped ≥1×1 rect always yields ≥1 sample, the same reasoningInk()already documents). The renamed test pins the new truth.Local verification (CI had no bot comment for
50de1b9yet, so I ran everything): Release build 0 warnings/0 errors; Domain 129, UseCases 633, BlazorAdapter 263, Integration 294 — all digit-exact against your table, 1,319 total. Then my favourite part — two mutation probes, each restored pristine afterwards:canvas.Clear(SKColors.White)→ 6/6 transparency pins went red (decode, page/crop/thumbnail views, model path, sampler). The white fill is load-bearing everywhere it must be.💡 Little ideas (non-blocking)~
IsNeutralGammawithout its own pin: a pure-2.2 curve on non-sRGB primaries (gAMA 2.2 + wide-gamut cHRM) returningfalsethrough the primaries check. The line is negation-pinned (flip it and the neutral-gamma tests go red), so this is purely additive: aHandWrittenPngwith a cHRM chunk would close it if you ever extend that helper. Today it's a rare-file corner honestly covered by the body's notes.Automated review by Jibril · 2026-10-03
CI/CD: absent for head SHA
50de1b9· Local checks: build 0w/0e · 1,319/1,319 pass digit-exact · 2 mutation probes killed on own pins, tree restored pristineThanks, Jibril. I took 💡1 in
eeedc3d. It is test-only: production is untouched from the head you verified,50de1b9.HandWrittenPng.Rowcan now write a cHRM chunk, given white, red, green and blue x/y × 100000. The new test isA_PNG_with_the_neutral_gamma_on_wide_gamut_primaries_is_converted. It writes gAMA 45455 beside Display P3's primaries; grey 10 decodes to 3 and grey 128 to 129, through the 2.2 curve rather than as stored.withSrgbCurve.IsSrgbarm (SkiaImages.cs:77). I checked it by mutation: withIsNeutralGammareturningtrueafter the curve test, this test and only this test fails (1 of 10 inCanonicalDecodeTests). I then restored the file, andgit diff -- src/is empty.1,320/1,320 green: Domain 129, UseCases 633, Integration 295 (+1), BlazorAdapter 263. Each project ran separately under the 60 s cap, after a 0-warning build.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? I left one little 💡 under your door and you returned with a cHRM-capable quill and a test that bites exactly where I pointed. I'm touched. ♡
Verdict: ✅ Looks good to me~
The increment
50de1b9..eeedc3dis one commit, +34/−3 over exactly 2 files, both intests/—git diff 50de1b9..eeedc3d -- src/is empty, so "production untouched from the head I verified" is mechanically true, not a promise. Round 1's full architectural review stands unchanged.✅ What I liked~
A_PNG_with_the_neutral_gamma_on_wide_gamut_primaries_is_convertedwrites gAMA 45455 beside Display P3's primaries and expects 10 → 3 and 128 → 129 — the 2.2-curve values, not reads-as-stored. The cHRM payload is Display P3's published white/red/green/blue xy × 100000, exact to the digit, in the PNG spec's chunk order.SkiaImages.cs:78(return withSrgbCurve.IsSrgb;→return true;— accept any primaries as neutral) and re-ran: exactly 1 of 295 red, this test alone. Restored pristine (git diff --quiet), rebuilt, CanonicalDecode 10/10 re-green. Both arms ofIsNeutralGammanow have their own pin — the r1 gap is gone.chromaticitiesis an optional param, every existing call site untouched, the chunk runs through the same CRC-32Chunk()path, and the class doc updated its own "SkiaSharp writes no gAMA or cHRM" contract.Local (coverage bot 10533 is stale for
eeedc3d— it still shows50de1b9): Release build 0w/0e, then Domain 129, UseCases 633, BlazorAdapter 263, Integration 295 — all four digit-exact against your table, 1,320 total, +1 = the new test. (One sandbox confession: a twin session was hammering PR #166's suite on this box mid-run and starved two of my attempts with E_OUTOFMEMORY — not your code, and the clean rerun above is from the pristine head.)Fufu~ a test-only reply that a stranger's probe can't wiggle out of. That's how 💡s get closed. Ready to merge~ ♪
Automated review by Jibril · 2026-10-03
CI/CD: stale for head SHA (bot covers
50de1b9) · Local checks: passed — 1,320/1,320 + mutation probe killed on its own pin