feat(gfx): v2.2.8 "Aperture II" — gamma-correct + sharper scanlines - #345
Merged
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughVersion 2.2.8 adds gamma-aware CRT scanline processing and configurable sharpness. Frontend, Android, and iOS hosts upload the new shader parameters. Release metadata and documentation identify v2.2.8 “Aperture II.” ChangesCRT presentation update
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CrtFilter
participant AndroidGfx
participant IOSMetal
participant CRT_WGSL
CrtFilter->>CRT_WGSL: upload sharpness and linearization values
AndroidGfx->>CRT_WGSL: upload CRT auxiliary values
IOSMetal->>CRT_WGSL: upload CRT auxiliary values
CRT_WGSL->>CRT_WGSL: decode, process, and encode CRT colors
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Updates RustyNES to the v2.2.8 “Aperture II” presentation-fidelity release, focusing on gamma-correct scanline/mask darkening and a sharper scanline profile in the shared CRT WGSL shader while keeping emulation/core outputs byte-identical by default.
Changes:
- Extend the base
CRT_WGSLuniform block (12 → 16 floats) and implement gamma linearization (aux.y) plus scanline sharpness blending (aux.x). - Wire the new CRT uniform layout and defaults through the desktop frontend (
crt.rs) and Android renderer (gfx.rs). - Bump release/version metadata across docs and workspace manifests (README/STATUS/CHANGELOG/AGENTS, Cargo.toml/Cargo.lock).
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Bumps visible version strings and updates “Current Release” narrative to v2.2.8. |
| docs/STATUS.md | Updates the top-of-file “current release” banner to v2.2.8 with presentation details. |
| docs/crt-composite.md | Documents the new base CRT pass aux semantics (gamma + sharpness). |
| crates/rustynes-gfx-shaders/src/lib.rs | Implements the CRT shader’s new aux behavior and expands the uniform layout to 16 floats. |
| crates/rustynes-frontend/src/crt.rs | Updates uniform packing and defaults (sharpness + linearize flag) for the desktop CRT pass. |
| crates/rustynes-android/src/gfx.rs | Updates uniform semantics/packing for Android and sets CRT aux values for filters 1/2. |
| CHANGELOG.md | Adds the v2.2.8 release entry describing the presentation changes. |
| Cargo.toml | Bumps workspace package version to 2.2.8. |
| Cargo.lock | Propagates the 2.2.8 version bump across workspace crates. |
| AGENTS.md | Updates the “Current release” line to v2.2.8. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
v2.2.8 "Aperture II" addresses the NESdev-forum feedback on gamma-aware resampling and bilinear-soft scanlines. It is presentation-only: nothing here touches the emulation core, so the pre-shader framebuffer, save-states, and every golden vector are byte-identical (AccuracyCoin 141/141, nestest 0-diff), and the shipped native default is byte-identical to v2.2.7 (the new behavior is gated behind aux = 0). The base BLEP audio and the advanced CRT stacks (royale/guest/megatron, already gamma-correct via their own gamma_in/gamma_out) are untouched. A prior investigation established that the native path is already gamma-correct (the NES framebuffer texture is Rgba8UnormSrgb, so the sampler decodes to linear before filtering + the scanline math, and the surface re-encodes on write) and the base BLEP is an 81.6 dB-SFDR band-limited decimator -- so the real gaps were the WebGL2 non-sRGB path and scanline sharpness, which this release targets. The base CRT/scanline pass (CRT_WGSL) uniform grows from 12 to 16 floats (rect + crop + params + aux); both hosts already had a 16-float-capable buffer (Android shares it with the NTSC pass; the desktop CrtFilter is extended here). Gamma-correct scanlines + aperture mask (aux.y) - The scanline/mask darkening now happens in linear light. aux.y = 0 (native: sRGB texture + surface) leaves the value linear -- byte-identical shipped output. aux.y = 1 (a plain UNORM path, e.g. WebGL2, which neither decodes on sample nor encodes on write) makes the shader sRGB-decode on read and re-encode before output, so a scanline valley is 50% of the LINEAR luminance, not the gamma-encoded value. This is a real browser-only gamma fix. Sharper scanlines (aux.x, default 0.5) - The scanline profile blends from the original soft parabola (0) to a narrow Gaussian beam (1) for crisp vertical row boundaries instead of the linear-sampler blur -- the sharper scanlines the feedback asked for. aux.x = 0 reproduces the pre-v2.2.8 profile exactly; the effect is only visible when scanlines are enabled. Hosts - Desktop (crates/rustynes-frontend/src/crt.rs): CrtFilter's uniform extended to 16 floats; sharpness (0.5) + linearize (from surface_format.is_srgb()) fields wired in new() and the per-frame render(). - Android (crates/rustynes-android/src/gfx.rs): the CRT/scanline filters (1, 2) now set aux = (0.5, linearize, 0, 0); linearize derives from the surface format (sRGB there, so 0 -- Android output unchanged). NTSC/Bisqwit aux paths untouched. Verification - naga validation (crt::tests::shader_parses_and_validates + the CRT stack) green; desktop frontend builds; native + wasm32 clippy -D warnings clean; cargo fmt + markdownlint clean; rustynes-frontend tests 464/464. The CRT filter output is not golden-tested (presentation-only), so no snapshots move. - VISUAL VERIFICATION PENDING: naga proves the WGSL compiles, not that it looks right. The sharper-scanline default and the WebGL2 gamma round-trip must be confirmed on a real display + a browser before merge. Docs: docs/crt-composite.md gains a "gamma + sharpness" section; STATUS/README/AGENTS/ CHANGELOG updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ilot #345) Address the three Copilot review threads on #345 ("Aperture II"): - **iOS Metal renderer parity (the substantive one).** `write_uniforms` in `crates/rustynes-ios/src/gfx_metal.rs` hard-wrote `aux = 0` for every filter, so the scanline (1) and CRT (2) filters on iOS never received the v2.2.8 `aux.x` scanline-sharpness or `aux.y` gamma-linearize knobs that the shared `CRT_WGSL` now consumes — iOS silently kept the old soft-parabola, gamma- encoded scanlines while desktop (`crt.rs`) and Android (`gfx.rs`) got the corrected path. It now computes `crt_linearize` from the surface's sRGB-ness (0 on the native sRGB Metal surface, 1 on a non-sRGB surface) and passes `aux = [0.5, crt_linearize, 0, 0]` for filters 1/2, byte-for-byte matching the Android host. On the native sRGB surface `crt_linearize == 0`, so the Metal CRT/scanline output stays byte-identical to pre-v2.2.8 — this closes a cross-platform-consistency gap, not a behavior change on the shipped default. - **CHANGELOG + AGENTS wording (backwards phrasing).** "the new behavior is gated behind `aux = 0`" read the flag inside-out: `aux = 0` is the *compatibility* path (the exact pre-v2.2.8 scanline profile), and the new linear-light + sharper-scanline path is what activates for a *non-zero* `aux` (the WebGL2 non-sRGB path, or when the scanline knob is raised). Both the CHANGELOG entry and the AGENTS.md current-release note now say so. Verification: `rustfmt --edition 2024 --check` clean; `cargo check -p rustynes-ios` clean on Linux (the uniform-writing path is not Metal-gated, so it type-checks off-device); `pre-commit run markdownlint` passes on the two docs. Presentation-only — the emulation core, AccuracyCoin 141/141, and nestest 0-diff are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
force-pushed
the
feat/v2.2.8-aperture-ii-video
branch
from
August 4, 2026 21:09
6240145 to
3badffd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 30: Update the v2.2.4 history entry in AGENTS.md to remove its stale
“current release” wording, replacing it with “the v2.2.4 release” or equivalent.
Preserve the v2.2.8 entry as the sole current-release record and leave the
historical release details unchanged.
In `@CHANGELOG.md`:
- Around line 47-49: Update the v2.2.8 release entry to document the iOS Metal
host wiring alongside the existing desktop and Android hosts, referencing the
shared 16-float CRT uniform; if iOS is intentionally excluded, explicitly state
the reason instead.
In `@crates/rustynes-gfx-shaders/src/lib.rs`:
- Around line 102-105: Replace the power-gamma conversion in the shader’s
linearize path at crates/rustynes-gfx-shaders/src/lib.rs lines 102-105 with the
sRGB EOTF using the 0.04045 breakpoint, and update the inverse conversion at
lines 149-152 to the sRGB OETF using the 0.0031308 breakpoint. Align the
corresponding transfer-function documentation at docs/crt-composite.md lines
55-61 with these piecewise formulas.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 75093a99-2f9a-4c6b-895b-82bd0281106e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (10)
AGENTS.mdCHANGELOG.mdCargo.tomlREADME.mdcrates/rustynes-android/src/gfx.rscrates/rustynes-frontend/src/crt.rscrates/rustynes-gfx-shaders/src/lib.rscrates/rustynes-ios/src/gfx_metal.rsdocs/STATUS.mddocs/crt-composite.md
…bbit #345) Address CodeRabbit's re-review of the iOS-aux commit — three findings: - **Major (functional correctness): use the exact sRGB piecewise transfer, not pow(2.2).** The `aux.y = 1` WebGL2 round-trip is meant to reproduce what the native path gets for free from a hardware sRGB texture/surface. But that hardware applies the IEC 61966-2-1 piecewise curve (a linear segment below `0.04045` / `0.0031308`, a 2.4-exponent power segment above), whereas the shader linearized with `pow(rgb, 2.2)` / re-encoded with `pow(rgb, 1/2.2)` — a plain power gamma that diverges from true sRGB most in the shadows. That made the WebGL2 result subtly *disagree* with the native sRGB path it exists to match. `CRT_WGSL` now carries `srgb_to_linear` / `linear_to_srgb` helpers implementing the exact transfer (component-wise `select` on the breakpoint), and the two `pow` sites call them. Presentation-only and native-path-inert: the native surface sets `aux.y = 0`, so this branch never runs there and the shipped native default stays byte-identical; only the WebGL2 appearance changes, now correctly. naga validation (`crt::tests::shader_parses_and_validates`) green. - **iOS host in the CHANGELOG.** The v2.2.8 "Changed" host-wiring sentence now lists the iOS Metal host alongside desktop and Android (the parity the prior commit added), and the gamma bullet documents the exact-sRGB transfer. - **Stale "current release" label.** The v2.2.4 entry in the AGENTS.md lineage paragraph still called v2.2.4 "the current release"; reworded to point at the actual current-release paragraph, so AGENTS.md carries one current-release record. `docs/crt-composite.md` aligned to the exact-transfer description. rustfmt + markdownlint clean; the emulation core, AccuracyCoin 141/141, and nestest 0-diff are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Antigravity review (Gemini via Ultra)This PR bumps the crate workspace to v2.2.8 "Aperture II" and updates the CRT presentation pipeline to perform scanline and aperture-mask darkening in linear light with a selectable Gaussian beam sharpness profile. Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
doublegate
added a commit
that referenced
this pull request
Aug 4, 2026
…ilot #346) Copilot flagged a real UX regression in the v2.2.9 detachable-window conversion: routing every tool panel through the shared `detachable_window` helper dropped each panel's bespoke `egui::Window` builder options — `default_pos`, `default_size`, `default_width` / `min_width`, and `resizable(false)` on ROM Info / Input Display / ROM Database / Performance. Losing the `default_pos` values in particular collapsed the debugger's designed workspace layout into egui's default overlap cascade on first open, and four fixed-size panels silently became resizable. `detachable_window` now takes a `WindowCfg { default_pos, default_size, default_width, min_width, resizable }` (all `Option`, `Copy + Default`) and applies each set field to the docked `egui::Window`; all 19 call sites (18 panels; `cheat_panel` has a native + a wasm variant) pass back their exact prior values, so first-open placement/size and the four non-resizable panels are restored. The config applies to the docked form only — a detached panel is a real OS window the window manager sizes and places (egui persists the docked window's own position/size by id after first open, so `WindowCfg` only seeds the first appearance). Native + wasm32 `clippy -D warnings` clean on both feature sets. Also (proactive, matching the CodeRabbit finding already fixed on #345): the v2.2.4 entry in the AGENTS.md lineage paragraph still called v2.2.4 "the current release" — reworded to point at the actual current-release paragraph so AGENTS.md carries a single current-release record. Frontend-only; the emulation core, AccuracyCoin 141/141, and nestest 0-diff are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
added a commit
that referenced
this pull request
Aug 4, 2026
…ilot #346) Copilot flagged a real UX regression in the v2.2.9 detachable-window conversion: routing every tool panel through the shared `detachable_window` helper dropped each panel's bespoke `egui::Window` builder options — `default_pos`, `default_size`, `default_width` / `min_width`, and `resizable(false)` on ROM Info / Input Display / ROM Database / Performance. Losing the `default_pos` values in particular collapsed the debugger's designed workspace layout into egui's default overlap cascade on first open, and four fixed-size panels silently became resizable. `detachable_window` now takes a `WindowCfg { default_pos, default_size, default_width, min_width, resizable }` (all `Option`, `Copy + Default`) and applies each set field to the docked `egui::Window`; all 19 call sites (18 panels; `cheat_panel` has a native + a wasm variant) pass back their exact prior values, so first-open placement/size and the four non-resizable panels are restored. The config applies to the docked form only — a detached panel is a real OS window the window manager sizes and places (egui persists the docked window's own position/size by id after first open, so `WindowCfg` only seeds the first appearance). Native + wasm32 `clippy -D warnings` clean on both feature sets. Also (proactive, matching the CodeRabbit finding already fixed on #345): the v2.2.4 entry in the AGENTS.md lineage paragraph still called v2.2.4 "the current release" — reworded to point at the actual current-release paragraph so AGENTS.md carries a single current-release record. Frontend-only; the emulation core, AccuracyCoin 141/141, and nestest 0-diff are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v2.2.8 "Aperture II" — gamma-correct + sharper scanlines
Third of the v2.2.6 → v2.3.0 NESdev-remediation line. Addresses the forum
feedback on gamma-aware resampling and bilinear-soft scanlines.
Presentation-only — core untouched
Nothing here touches emulation, so the pre-shader framebuffer + save-states + every
golden vector are byte-identical (AccuracyCoin 141/141, nestest 0-diff). A prior
investigation established the native path is already gamma-correct (the NES texture
is
Rgba8UnormSrgb, so the sampler decodes to linear before filtering + the scanlinemath), and the base BLEP is an 81.6 dB-SFDR decimator — so the real gaps were the
WebGL2 non-sRGB path and scanline sharpness, which this targets. The advanced
CRT stacks (royale/guest/megatron) were already gamma-correct and are untouched.
Changed
aux.y). The darkening now runs in linearlight.
aux.y = 0(native sRGB) leaves it linear — byte-identical output;aux.y = 1(plain UNORM, e.g. WebGL2) sRGB-decodes on read + re-encodes on output,fixing a browser-only gamma error.
aux.x, default 0.5). Blends the soft parabola → a narrowGaussian beam for crisp vertical boundaries.
aux.x = 0= the exact pre-v2.2.8profile; visible only when scanlines are enabled.
CRT_WGSLuniform 12 → 16 floats (rect+crop+params+aux); wired on both thedesktop (
crt.rs) and Android (gfx.rs) hosts.Checks
naga validation green · desktop builds · native + wasm32
clippy -D warningsclean ·cargo fmt+ markdownlint clean ·rustynes-frontendtests 464/464 · no goldensnapshots move (CRT output isn't golden-tested). Docs:
docs/crt-composite.md+STATUS/README/AGENTS/CHANGELOG.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation