Skip to content

✨ feat(renderer): add gamma and image auto contrast - #145

Merged
cfviotti merged 4 commits into
mainfrom
caio/dataman-677-media-adjustments
Oct 14, 2026
Merged

cfviotti merged 4 commits into
mainfrom
caio/dataman-677-media-adjustments

Conversation

@cfviotti

@cfviotti cfviotti commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

Add Gamma and automatic local contrast for inspecting difficult lighting. Adjustments change displayed media pixels while source pixels and annotation colors stay intact.

  • Optional Gamma and Auto through the existing display-adjustment API.
  • Image Auto applies CLAHE and replaces manual contrast. Brightness and Gamma remain active; Gamma also supports video.
  • Empty adjustments ({}) restore the unadjusted display.

Type of Change

  • New feature

Validation

  • Full SDK tests: 266 files / 3,427 tests, plus boundary, documentation, runtime and release-contract checks.
  • Scene reset proof: {} after nondefault Gamma and Auto repaints and leaves only the identity ColorMatrix filter; manual contrast restoration remains covered.
  • Actual Pixi uploader regression: tiny and extreme-aspect image LUTs upload complete RGBA rows; delivered Gamma/Auto uniforms checked.
  • Real local WebGL pixels: Gamma, Auto, source-anchored zoom/pan, Gamma after Auto, Reset/source unchanged.
  • Test-inclusive typecheck, lint, formatting and portable package build passed.
  • Public API documentation updated.
  • Candidate 0.2.9 portable tarball, clean consumer smoke and npm publish dry-run passed.
  • Reviewed feature-head CI (79fd2f2) passed.
  • Current-head CI (8f05e38) passed.

Notes For Reviewers

The fields are optional additions to the public display-adjustment API. Auto applies only to still images. Histograms use an image sample bounded to 1024px and remain anchored to the image during pan and zoom.

The WebGL checks used the locally built candidate. WebGPU declarations and actual queue-upload descriptors were checked; hardware WebGPU pixels and moving-video Gamma were not exercised. Production bytes are unchanged by the version preparation and reset-test extension.

This PR prepares candidate 0.2.9 under the 0.x patch-release policy, including matching package metadata and public install instructions. Consumers need a published release; merging this PR does not publish 0.2.9.

@cfviotti

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: REVIEW-CLEAN @ aa541e7 · round 1 · full scope

No confirmed Critical or Warning findings. One concrete Minor will be fixed before readiness: reduced lookup-table textures retain a larger buffer, which gives Pixi's WebGPU uploader the wrong row stride for tiny or extreme-aspect images. Normal still-image WebGL is unaffected.

Coverage and limits

Four independent native Codex reviewers, one outside Claude reviewer and root adjudication covered all seven changed files and all 13 lenses. Actual installed Pixi 8.20.1 contracts were checked. One outside resize hypothesis relied on recalled dependency behavior; the actual uploader source refutes it. Review-clean does not replace tests or CI. WebGL pixel checks passed separately; WebGPU pixel output has not been exercised.

Review Ledger

Open findings

  • packages/web/src/renderers/pixi-media-inspection-filter.ts:187:reduced-lut-buffer-stride | Minor | Resized texture height can be below 64 while resource retains 65,536 bytes. Installed gpuUploadBufferImageResource derives bytesPerPixel from resource byte length / texture area. WebGPU fallback plus a reduced histogram grid gives invalid or wrong row stride; source pixels remain intact and disabling Auto restores display. | round 1

Prior key statuses

  • None. First round.

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • Minor: scene tests establish repaint/filter ownership and CPU LUT behavior; shader pixels are proved separately by the local WebGL harness. A small uniform-delivery assertion will accompany the concrete upload regression.
  • Display-adjustment inputs follow the existing numeric API convention. Supported controls supply finite values; broader malformed-input validation is outside this addition.
  • No general fallback layer for speculative canvas allocation failures was added; no reproducible newly failing supported producer was established.

Refuted

  • WebGL resizing retains the old texture when either dimension matches: actual Pixi uses width AND height to select texSubImage2D; changed height allocates a new texture.
  • Public settings can arrive before scene initialization and be discarded: the public renderer initializes the scene synchronously before exposing readiness.
  • Inspection naming requires another abstraction: private renderer terminology has no broken consumer.

Implausible

  • None independently established.

Pre-existing

  • ColorMatrix allocation on adjustment changes exists on the base.

@cfviotti

cfviotti commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: REVIEW-CLEAN @ 79fd2f2 · round 2 · narrowed to aa541e7..79fd2f2

No confirmed Critical or Warning findings. One fresh native Codex lane, one outside Claude lane and root adjudication covered both complete changed files and all 13 lenses. CI verify and both security checks passed. Convergence is temporarily withheld for the small test improvement identified in later reviewer feedback. Review-clean does not replace tests or CI.

Fixed since last round

  • Reduced CLAHE grids now expose the filled RGBA buffer before texture resize/update, producing the correct GPU upload stride. The regression invokes Pixi's actual uploader for tiny and extreme-aspect images.
  • Scene assertions now check delivered Gamma and Auto uniform values.

Non-blocking

One accepted coverage note

The production assignment order is correct. A future edit moving it below synchronous resize could evade the final-state upload test; this durability-only gap is accepted for this focused repair.

Later reviewer feedback, Oct 14

Review comment: Minor, confirmed test sensitivity. The documented {} reset currently defaults Gamma to 1 and Auto to false in pixi-media-scene.ts:1462-1463; present behavior is correct. The scene test explicitly supplies those defaults, so it does not protect this reset after non-default Gamma/Auto. We will extend the existing scenario with a few assertions, without changing renderer behavior or adding another fixture.

Caveats

Narrowed round: the remaining PR surface retains round 1 coverage. The local WebGL pixel checks passed separately. Queue-descriptor tests exercise the real uploader, but hardware WebGPU pixels remain unverified.

Review Ledger

Open findings

  • None.

Prior key statuses

  • packages/web/src/renderers/pixi-media-inspection-filter.ts:187:reduced-lut-buffer-stride | resolved | The filled byte prefix is assigned before resize/update; real Pixi uploader tests establish 1024 bytes per row for reduced grids at this head.

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-media-scene.push.test.ts:1350:lut-prefix-order-unpinned | Minor, accepted | Current production ordering is correct; future event-time ordering coverage is deferred. No present-head runtime defect.
  • Uniform assertions inspect the bound CPU-side object; shader pixels are checked separately by the local WebGL harness. Hardware WebGPU pixels remain unexecuted.
  • Display-adjustment inputs retain the existing numeric API convention. Broader malformed-input validation remains outside this addition.
  • No general fallback layer for speculative canvas-allocation failures was added; no reproducible newly failing supported producer was established.

Refuted

  • WebGL resize retains old height when width matches: installed Pixi requires width AND height to match before texSubImage2D.
  • Public settings arrive before scene initialization: the public renderer initializes the scene before exposing readiness.
  • Inspection naming requires another abstraction: no broken public consumer was established.

Implausible

  • None established.

Pre-existing

  • ColorMatrix allocation on adjustment changes exists at the pinned base and is unchanged.

@cfviotti cfviotti added the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 14, 2026
@cfviotti
cfviotti marked this pull request as ready for review October 14, 2026 16:34
@cfviotti
cfviotti requested a review from a team as a code owner October 14, 2026 16:34
Comment thread packages/web/src/renderers/pixi-media-scene.push.test.ts
@cfviotti cfviotti removed the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 14, 2026
@cfviotti

cfviotti commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Mega Review

Status: REVIEW-CLEAN @ 8f05e38 · round 3 · narrowed to 79fd2f2..8f05e38

No confirmed Critical or Warning findings. Converged after three rounds. Fresh Codex and Claude reviews, including completed whole-file coverage, and root adjudication covered all 13 lenses. Current-head CI and both security checks passed. Review-clean does not replace tests or CI.

Fixed since last round

  • The scene test now applies {} after enabling Gamma and Auto, then verifies another repaint and identity filtering. This addresses the reset-test request; the GitHub thread is resolved and João’s review has been re-requested.
  • Candidate 0.2.9 synchronizes the public package manifest, lockfile, docs toolbar and migration install references under the repository's patch-release policy.

Non-blocking

Accepted coverage limits

Current reset behavior is correct. The new activation step relies on earlier uniform assertions and the subsequent reset proof; additional activation assertions are deferred. The prior accepted event-time LUT ordering note remains unchanged. Non-default brightness reset coverage predates this addition and is outside this repair.

Caveats

This narrowed round covers the repair and carried concerns; untouched runtime retains earlier review coverage. Local WebGL pixel evidence remains separate. Hardware WebGPU pixels and moving-video Gamma remain unverified. Candidate 0.2.9 is unpublished; merge and the protected manual release are separate steps. Ticket compliance: NOT REQUESTED for this public SDK review.

Review Ledger

Open findings

  • None.

Prior key statuses

  • packages/web/src/renderers/pixi-media-inspection-filter.ts:187:reduced-lut-buffer-stride | resolved | Filled byte prefix still precedes resize/update at current lines 187-189; the actual Pixi uploader regression remains at test lines 1343-1388.
  • packages/web/src/renderers/pixi-media-scene.push.test.ts:1316:empty-adjustments-reset-uncovered | resolved | Human comment 4274844706 is addressed by current lines 1327-1337: non-default Gamma/Auto, then {}, repaint, only identity filter with default brightness/contrast. Present omitted-setting retention mutation fails; former proof alone permits it.

Unresolved prior open findings

  • None.

Deliberate tradeoffs

  • packages/web/src/renderers/pixi-media-scene.push.test.ts:1350:lut-prefix-order-unpinned | Minor, accepted | Production prefix assignment order remains correct; final-state uploader proof does not pin future event-time ordering. No new present-head defect.
  • packages/web/src/renderers/pixi-media-scene.push.test.ts:1327:scene-reset-activation-unasserted | Minor, accepted | Activation has earlier uniform coverage; the appended step checks repaint and subsequent delivered reset filtering. A combined future activation failure and omitted-setting retention could evade it; both paths work now, and isolated retention mutation fails.
  • Uniform delivery assertions and separate historical local WebGL pixel checks remain distinct; hardware WebGPU pixels are unexecuted.
  • Existing numeric API convention and scoped canvas-allocation behavior remain unchanged; no general malformed-input or speculative fallback layer was added.

Refuted

  • WebGL matching-width resize retains old height: actual locked Pixi 8.20.1 uploader requires both width and height equality before texSubImage2D.
  • Successful public renderer creation exposes settings before media initialization: media-renderer-core initializes media at line 1361 before publishing Ready and returning the successfully initialized renderer.
  • Inspection naming requires another abstraction: existing display-adjustment API and private filter responsibilities remain coherent; no violated repository requirement or broken public consumer was established.

Implausible

  • None established.

Pre-existing

  • ColorMatrix allocation on adjustment changes exists in the pinned-base setter and is unchanged.
  • packages/web/src/renderers/pixi-media-scene.push.test.ts:1336:empty-reset-brightness-unexercised | Minor, non-blocking | Non-default brightness reset is not exercised here; pinned-base and current setters correctly use brightness ?? 1. The changed promise and reviewer request concern Gamma/Auto; no new brightness failure was introduced.
  • Docs can expose the candidate version between a main merge and protected manual npm publication; that sequence is the unchanged base release procedure.

@cfviotti cfviotti added the LGTMegaReview 🤖 Approved by Jarbas Mega Review label Oct 14, 2026

@joaomarcoscrs joaomarcoscrs left a comment

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.

LGTM

@cfviotti
cfviotti merged commit 344066e into main Oct 14, 2026
3 checks passed
@cfviotti
cfviotti deleted the caio/dataman-677-media-adjustments branch October 14, 2026 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTMegaReview 🤖 Approved by Jarbas Mega Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants