Skip to content

feat(sinker): Ship 8 — high-bit 4:2:2 RGBA sinker integration#28

Merged
uqio merged 2 commits intomainfrom
feat/ship8-rgba-high-bit-422-sinker
Apr 26, 2026
Merged

feat(sinker): Ship 8 — high-bit 4:2:2 RGBA sinker integration#28
uqio merged 2 commits intomainfrom
feat/ship8-rgba-high-bit-422-sinker

Conversation

@uqio
Copy link
Copy Markdown
Collaborator

@uqio uqio commented Apr 26, 2026

Summary

Wires with_rgba (u8) + with_rgba_u16 (u16) into the 8 high-bit 4:2:2 sinker formats in src/sinker/mixed/subsampled_4_2_2_high_bit.rs and updates each PixelSink::process to consume them via Strategy A combine. Pure sinker-layer integration — no new SIMD kernels, no new dispatchers, no row-level changes: every format reuses an existing 4:2:0 high-bit RGBA dispatcher already wired in PR #25 (Tranche 5a) + PR #26 (Tranche 5b).

Why no row-level work

4:2:2 has the same per-row chroma shape as 4:2:0 (half-width U/V, one pair per Y pair) — only the vertical subsampling differs. The walker handles row cadence; the row primitive is shared. This kernel-reuse pattern was already established for the existing 4:2:2 high-bit RGB path (each Yuv422p<N>::process calls yuv420p<N>_to_rgb_row).

Changes

Sinker (src/sinker/mixed/subsampled_4_2_2_high_bit.rs)

8 sinker impl blocks gain with_rgba / set_rgba / with_rgba_u16 / set_rgba_u16 — 32 new methods. Each format's process() is restructured to consume the new buffers via Strategy A:

Sinker format Reused RGBA dispatcher Strategy-A expand BITS
Yuv422p9 yuv420p9_to_rgba(_u16)_row 9
Yuv422p10 yuv420p10_to_rgba(_u16)_row 10
Yuv422p12 yuv420p12_to_rgba(_u16)_row 12
Yuv422p14 yuv420p14_to_rgba(_u16)_row 14
Yuv422p16 yuv420p16_to_rgba(_u16)_row 16
P210 p010_to_rgba(_u16)_row 10
P212 p012_to_rgba(_u16)_row 12
P216 p016_to_rgba(_u16)_row 16

Strategy A pattern (mirrors the 4:2:0 PR #26 implementation):

  • u16 path: rgba_u16-only routes directly through *_to_rgba_u16_row; rgb_u16 + rgba_u16 runs the RGB kernel once and fans out via expand_rgb_u16_to_rgba_u16_row::<BITS> (cheap per-pixel pad with depth-aware alpha).
  • u8 path: same shape — rgba-only goes direct; rgb + rgba (or hsv + rgba) uses the existing scratch + expand_rgb_to_rgba_row fan-out.

P210 / P212 / P216 previously had no const BITS: u32 declaration in their process() (luma used a hardcoded >> 8); added one per format so expand_rgb_u16_to_rgba_u16_row::<BITS> can be invoked uniformly.

Doc-fail example (src/sinker/mixed/planar_8bit.rs)

The compile_fail doctest demonstrating "attaching RGBA to a sink that doesn't write it is rejected" had been pointing at Yuv422p10 after PR #26. After this PR, Yuv422p10 does write RGBA — moved the example to Yuv444p10 (4:4:4 high-bit, still genuinely lacks with_rgba until the next PR adds it).

Tests (src/sinker/mixed/tests.rs)

+8 new sinker tests (test count 499 → 507):

  • Yuv422p10 u8 RGBA gray-to-gray with opaque alpha (0xFF)
  • Yuv422p10 u16 RGBA gray-to-gray with alpha = 1023
  • Yuv422p10 Strategy A u8: with_rgb + with_rgba byte-identical RGB
  • Yuv422p10 Strategy A u16: with_rgb_u16 + with_rgba_u16 byte-identical RGB
  • Yuv422p10 RgbaBufferTooShort err
  • Yuv422p10 RgbaU16BufferTooShort err
  • P210 u16 RGBA gray-to-gray (covers BITS-generic Pn semi-planar)
  • Yuv422p16 u16 RGBA gray-to-gray (covers 16-bit dedicated kernel)

Explicitly out of scope

  • Yuv440p10 / Yuv440p12 (also live in this file) reuse 4:4:4 high-bit dispatchers — those don't have RGBA wired yet. Deferred to the next PR alongside the 4:4:4 high-bit RGBA work.

Test plan

  • cargo test --lib: 507 pass, 0 fail (was 499; +8 new tests, all in scope)
  • cargo check --tests --lib clean across host (aarch64-darwin), x86_64-unknown-freebsd, wasm32-unknown-unknown
  • RUSTFLAGS=\"-Dwarnings\" cargo clippy --lib --tests clean
  • cargo test --doc clean (the updated Yuv444p10 compile_fail example correctly fails to compile)

Codex adversarial review

Verdict: approve. No material findings. (review log)

🤖 Generated with Claude Code

@al8n al8n requested a review from Copilot April 26, 2026 23:23
@al8n al8n changed the title update feat(sinker): Ship 8 — high-bit 4:2:2 RGBA sinker integration Apr 26, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds packed RGBA (u8) and native-depth RGBA (u16) output wiring for high-bit-depth 4:2:2 formats in MixedSinker, along with targeted regression tests, and updates a compile-fail doc example to keep it valid after the new wiring.

Changes:

  • Implement with_rgba / set_rgba and with_rgba_u16 / set_rgba_u16 for multiple high-bit 4:2:2 sinker formats, including Strategy-A “RGB then expand to RGBA” behavior.
  • Add tests validating gray conversion, opaque alpha, Strategy-A equivalence (RGB-only vs RGB+RGBA), and short-buffer errors for selected 4:2:2 formats.
  • Update the planar 8-bit sinker doc compile-fail snippet to use a still-not-wired format for RGBA.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/sinker/mixed/tests.rs Adds new tests for high-bit 4:2:2 RGBA/u16-RGBA wiring and error paths.
src/sinker/mixed/subsampled_4_2_2_high_bit.rs Wires RGBA outputs (u8/u16) for high-bit 4:2:2 planar and P21x formats, including Strategy-A expand paths.
src/sinker/mixed/planar_8bit.rs Updates a compile_fail doc example to remain a valid negative case now that Yuv422p10 supports RGBA.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/sinker/mixed/subsampled_4_2_2_high_bit.rs Outdated
Comment thread src/sinker/mixed/subsampled_4_2_2_high_bit.rs Outdated
Comment thread src/sinker/mixed/tests.rs
Comment on lines +6220 to +6226
// ---- Ship 8 PR 5d: high-bit 4:2:2 RGBA wiring -------------------------
//
// Strategy A combine for the eight 4:2:2 high-bit sinker formats wired
// in the 4:2:2 high-bit file. Mirrors the 4:2:0 PR #26 test suite;
// covers Yuv422p10 (planar BITS-generic), Yuv422p16 (planar 16-bit
// dedicated kernel), and P210 (semi-planar BITS-generic) — the row
// layer is exhaustively tested elsewhere.
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new test tranche is described as covering the 4:2:2 high-bit RGBA wiring for eight formats, but it only exercises Yuv422p10/Yuv422p16/P210. Please consider adding at least lightweight smoke tests for the remaining newly-wired formats (e.g., Yuv422p9/12/14 and P212/P216) to validate alpha fill semantics across BITS (4095/16383/0xFFFF) and catch format-specific wiring regressions.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defensible suggestion, but deferring this one. Two reasons:

  1. Precedent from PR Ship 8 Tranche 5b: high-bit 4:2:0 RGBA u16 SIMD + sinker integration #26 (Tranche 5b): the 4:2:0 sinker tests added there only smoke-tested Yuv420p10 / P010 / Yuv420p16Yuv420p9 / Yuv420p12 / Yuv420p14 / P012 weren't covered either. The 4:2:2 PR mirrors that gap exactly, so closing it here would create per-PR-asymmetric coverage.

  2. Row-layer covers per-BITS alpha exhaustively: the per-backend RGBA equivalence tests added in PR Ship 8 Tranche 5a: high-bit 4:2:0 RGBA u8 SIMD #25/Ship 8 Tranche 5b: high-bit 4:2:0 RGBA u16 SIMD + sinker integration #26 already prove alpha = (1 << BITS) - 1 (BITS=9/10/12/14) and 0xFFFF (BITS=16) across all 5 SIMD backends + scalar reference. A sinker-layer smoke test for Yuv422p9 etc. would only re-prove that the sinker calls the dispatcher with the right buffer — which the existing Yuv422p10 / Yuv422p16 / P210 tests already confirm for representative kernel families (BITS-generic planar, 16-bit dedicated, BITS-generic semi-planar).

Happy to add the per-format smoke tests as a follow-up if the symmetric gap on the 4:2:0 side bothers reviewers — would prefer to do both in a single cleanup PR.

…16-bit kernel

The `with_rgba` doc on `MixedSinker<Yuv422p16>` and `MixedSinker<P216>` claimed
the 16-bit YUV → 8-bit RGBA path uses "the `BITS = 16` Q15 kernel family", but
both formats actually call `yuv420p16_to_rgba_row` / `p016_to_rgba_row` — the
**dedicated 16-bit kernel family** (i64 chroma multiply, separate from the
BITS-generic Q15 pipeline used at BITS=10/12/14).

Aligns the wording with the existing `Yuv420p16` / `P016` doc convention in
`subsampled_4_2_0_high_bit.rs` and explicitly contrasts the Q15 pipeline so
future readers don't conflate the two kernel families.

Addresses Copilot review comments on PR #28.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@uqio uqio merged commit c66cbdc into main Apr 26, 2026
43 checks passed
@uqio uqio deleted the feat/ship8-rgba-high-bit-422-sinker branch April 26, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants