Repository navigation
Experiment: opt-in SDR-to-EDR presentation - #1
oliverames wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92d3a70d3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| NVN_EDR_MSL | ||
| "fragment float4 fs(V in [[stage_in]], texture2d<float> t [[texture(0)]], sampler s [[sampler(0)]], constant Crop& c [[buffer(0)]]) { float4 color=t.sample(s, clamp(in.uv * c.rect.zw + c.rect.xy, c.edge.xy, c.edge.zw));\n" | ||
| "#if EDR_PRESENT\n color.rgb=edr_map(color.rgb,c.edr.x,c.edr.y>0.0f);\n#endif\n return color; }\n"; | ||
| NSError *e = nil; |
There was a problem hiding this comment.
This changes the shared presentation shader and its constant-buffer layout even when EDR is disabled, but the commit's validation explicitly reports no five-game route and only a Silksong smoke test rather than a gameplay route. Synthetic GPU readbacks do not exercise integration with the other games' swapchains and presentation paths. Before merging, run scripts/regress.py check --games for the five local games most likely to be affected, or document why a smaller selection establishes a narrow reach, and attach the named-game report required by AGENTS.md.
AGENTS.md reference: AGENTS.md:L30-L30
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I agree that the shared uniform/shader layout changes even with EDR off. The PR description now includes a named eight-game coverage table and the narrower final-presentation report: current GPU checks cover both presentation implementations, SDR byte parity, crop/bars, conversion/headroom and MetalFX screenshots, with separate native layer checks. The only real-game evidence is the explicitly dated short Silksong off/on smoke, not a gameplay route. Only BotW/TotK currently have prepared Mac hosts, routes and saves; five routes have not been run. Normal Mac BGRA8 routes would test shared SDR integration, not the opt-in iOS float output. Multi-game gameplay acceptance remains pending before production/default adoption; I am leaving this validation thread open rather than presenting the synthetic checks as named-game passes.
The walk from the load back to the bounds check noticed copies and masks but no other write: cmp w18,#4; b.hi; add w18,w18,#1; ldrsw x18,[x4,x18,lsl #2] read entries 0 to 4 while the code uses 1 to 5, so one case's target was missing with no sign of it. A site with a write of the index after its check, or a call with a caller-saved index, is now refused, as jump_adr_offsets and half_count already refuse a write. A refusal goes on to the other recognizers and ends in the jump table assertion. Writes before the check do not count, which keeps the sites check_before_copy takes. A write that keeps the value, such as sxtw x0,w0, is refused too; no module on the drive has one. Checked: test_jump_copies.py (the new cases fail on the previous rewriter). Of the 345 unique modules on the drive, old and new recognition are the same on 281, and 60 more mains were scanned on the previous code with no site this refuses. That includes Smash 13.0.5, EA SPORTS FC 27 and Well Dweller. Four could not be checked: New Super Mario Bros. U Deluxe's main, sdk and rtld fail alike (no profile), and Smash 1.0.0's main stops at a table the game's note already lists. No regress.py run: nothing changes in the modules checked.
|
Closing: experiment evidence and testing are tracked in the upstream nxnative PRs, so the fork-only experiment copy comes down. Keeping the branch here for reference. |
… that would put the table in page zero, since no path of a working game carries it to the load (issue seedds#1532) main+0x59dc90 interleaves four dispatch loops that share registers: x12 is the table base of one (adrp + add at +0x59dba4) and the constant 1 of another (mov w12,#1 at +0x59dbdc, in a block that leaves by b to its own dispatch), kept apart only by the value of w8. Since e5620b1 the base's writer walk sees a write of wN, met that mov first and gave up, and the NSP script stopped at `jump table 59dc90` on a build the profile names. patch_nso.py null_base: before the walk has found the base's add, a mov of a constant (or of wzr / xzr) that puts the whole table inside page zero is passed; Horizon never maps that page, so the load would fault through it on the console too. Any other writer still ends the chain, and a constant between the adrp and the add is the base's own input. Checked: test_jump_copies.py NullPageBase (4 of its cases fail on the rewriter before), lint_python 0 new, test_docs. The site's ten targets are instructions of the function, three of them shared with its sibling tables in x15 and x17, and Cuphead's main.sites.tsv is line for line the one of the rewriter before e5620b1. Rewritten before and after: 34 other modules of 12 games (Okami HD, Octopath Traveler 0 and II, Three Houses, Sonic Superstars, Minecraft Dungeons II, Splatoon Raiders, Ocarina of Time, DK Bananza, Pokopia, Mario Tennis Fever, Cuphead's sdk and movie module) are byte-identical. Cuphead's NSP build completes, and the game plays from a fresh save to the Inkwell Isle map at 1.3.7 with no red line (the route does not reach this function). No regress.py run: nothing changes in any module that rewrote before.
…d jump_adr_offsets took the nearest cmp of the index, whatever followed it (issue seedds#977, PR seedds#1515) guarded_count takes a compare only when, between it and the table load, the first branch on an unsigned condition comes with the compare's flags still in place: b.hi out of the dispatch reads N + 1 entries, b.hs / b.cs N (read as N + 1 before), b.ls / b.lo into it the same counts when the instruction before its target ends the path that failed. A compare with no such branch only sets the flags and the walk goes on to the check before it; with none the site is refused. Before, `cmp w18,#4; b.hi exit; cmp w18,#1; ldrsw ...` read 2 entries of a table of 5 with no error. Checked (by the author): 165 sites of 27 modules of 6 games, all cmp; b.hi, rewritten byte-identically; GuardedCheck, 8 cases, 7 failing before. Checked here, as seedds#977 asks before a stricter rule goes in: 35 modules of 12 games on the T7 (Cuphead, Three Houses, Minecraft Dungeons II, Octopath Traveler 0 and II, Okami HD, Sonic Superstars, Ocarina of Time, DK Bananza, Mario Tennis Fever, Pokopia, Splatoon Raiders) rewrite byte-identically before and after, and so do Super Smash Bros. Ultimate 13.0.5, Mario Kart 8 Deluxe and Super Mario Odyssey's main, sdk and subsdk0 (against origin/main, with the page-zero rule of the commit before); test_jump_copies 75 OK, lint_python 0 new, test_docs. No regress.py run: no module that rewrote before changes. Modules with b.hs or a branch into the dispatch are still tested only synthetically. Co-authored-by: Atomsk-debug <135618857+Atomsk-debug@users.noreply.github.com>
This is a fork-only experiment targeting
codex/experiment-base. Keep it isolated frommainand the existing upstream PR heads. The switch is off by default.What and why
The rendering-scene foundation reports EDR capacity but still presents SDR.
DM_PRESENT_EDR=1selects extended-linear sRGB float output and expands SDR highlights within current scene headroom and an experimental 2x ceiling. It does not recover native game HDR. Game HUD/text shares those SDR pixels and can brighten; separate UIKit controls are unchanged. MetalFX stays SDR before the final conversion, and screenshots stay SDR.Checked
Production iOS build; 12 policy checks, 47 real Mac GPU readbacks, and 88 crop checks in each mode. Source/docs/lint checks pass.
Newt, iOS 27.2: 47 GPU checks with synthetic headroom; actual production layer: 28 default and 31 opt-in checks.
At feature commit
92d3a70, a separately identified Silksong 1.0.30000 build passes fresh-process smoke in both modes near 60 fps, without new RED errors. Final GPU/scene checks were rerun after the prerequisite merge; that gameplay smoke was not repeated at the final head. Actual scene headroom increased from 1.2 to 2. This is not a gameplay route, luminance measurement or sustained power comparison.Other games, visual acceptance, headroom transitions during extended play and sustained cost remain pending. GPU format/crop/readback tests exercise the changed final-present path directly; no five-game route was run.
The standalone fixture now clearly identifies itself as a verification app that uses no game files; its result records the actual visible label. The final source passes the same native checks.
Named-game validation and remaining integration checks
The shared presentation shader and uniform layout change even with EDR off. The verified boundary is final presentation: profiles, translated game shaders and guest commands are unchanged. Real GPU tests check both presentation implementations, SDR byte parity, crop/bars, linear conversion, headroom and MetalFX screenshots. Native checks exercise the actual iOS layer. These results do not establish every game’s swapchain integration.
The GPU checks above are synthetic presentation tests, not named-game passes. The present section is byte-identical between92d3a70 and the current head; final GPU/layer checks were rerun after the prerequisite merge. Ordinary Mac layers stay BGRA8, so Mac routes would check shared SDR integration rather than the opt-in float-layer EDR mode. Five prepared routes are unavailable in the current local library. Multi-game gameplay validation remains pending before production/default adoption; this fork-only experiment stays disabled and unmerged.
Docs