Skip to content

Validate PyO3 buffer dimensions; fix mono OKLab midpoint comment#49

Open
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/pyo3-validate-dimensions-and-doc
Open

Validate PyO3 buffer dimensions; fix mono OKLab midpoint comment#49
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/pyo3-validate-dimensions-and-doc

Conversation

@balloob

@balloob balloob commented Jul 5, 2026

Copy link
Copy Markdown

Two small, low-risk fixes to the epaper-dithering core and its PyO3 binding.

Finding 1 (behavior/safety): PyO3 entry point ignored the caller's height

packages/python/src/lib.rs's dither_image discarded its height argument (let _ = height;) and let ImageBuffer::new derive height via integer division, with only a debug_assert! guarding consistency. In a release build, a caller passing a width that does not match the byte-buffer length would silently truncate trailing pixels instead of raising.

The entry point now validates that the pixel buffer length equals width × height × 3 (the flat-RGB layout, confirmed 3 channels) using checked multiplication, and raises PyValueError on mismatch (or on overflow). A binding-level pytest (TestBufferValidation) covers both the mismatch-raises and the happy path.

The wasm binding (packages/rust/wasm/src/lib.rs) takes no height argument at all — dimensions are derived from width alone — so it has no equivalent ignored-parameter issue and is left unchanged.

Finding 2 (docs): wrong comment about the mono OKLab midpoint

Two comments in packages/rust/core/src/algorithms.rs claimed the mono decision midpoint "sits at OKLab L=0.5 ≈ sRGB 188". That conflates linear 0.5 (which sRGB-encodes to ~188) with OKLab L=0.5, whose neutral-gray sRGB value is ~100. Both comments now read ≈ sRGB 100. Comments only — no logic changed, and the ordered_dither_activity_is_perceptually_uniform test is unaffected.

Verification

  • cargo build and cargo test in packages/rust: 48 unit + 3 regression tests pass.
  • cargo check on packages/python: clean.
  • maturin develop + pytest: 58 tests pass, including the two new validation tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR

Finding 1 (behavior/safety): the PyO3 `dither_image` entry point ignored
the caller's `height` argument and let `ImageBuffer::new` derive height by
integer division. In a release build a caller passing a `width` that does
not match the byte-buffer length would silently truncate trailing pixels
instead of erroring. The entry point now validates that the pixel buffer
length equals width x height x 3 (flat RGB) and raises `PyValueError` on
mismatch. A binding-level pytest exercises both the mismatch and the
happy path. The wasm binding takes no `height` argument (dimensions are
derived from width alone) and so has no equivalent ignored-parameter
issue; it is left unchanged.

Finding 2 (docs): two comments in algorithms.rs claimed the mono decision
midpoint "sits at OKLab L=0.5 ≈ sRGB 188". That conflates linear 0.5
(which sRGB-encodes to ~188) with OKLab L=0.5, whose neutral-gray sRGB
value is ~100. Corrected both comments to ≈ sRGB 100. Comments only; no
logic changed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
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.

1 participant