feat(apple): gamepad on macOS/iOS + iOS AVAudioSession#125
Open
proggeramlug wants to merge 2 commits into
Open
feat(apple): gamepad on macOS/iOS + iOS AVAudioSession#125proggeramlug wants to merge 2 commits into
proggeramlug wants to merge 2 commits into
Conversation
…docs) Main CI was red on ffi-parity, and fixing that exposed a second failure (the file-line gate) that had been hidden behind it — the EN-063 "gate nobody reads" pattern, recurring. CI: - Export bloom_is_key_repeated on web (input_ffi.rs) and watchOS (regenerated ffi_stubs.rs via gen_stubs.js). validate-ffi green. - Split renderer/mod.rs 13156 -> 12556 by moving ~30 GPU-uniform POD structs + build_card_ortho_v2 into types.rs (their documented home). Struct/field order is byte-identical, so this is layout-neutral; the golden-image suite (raster) and 128 unit tests pass. Baseline ratcheted to 12556. (EN-052) Silent-failure FFI bugs (TS API): - loadMaterial called bloom_set_material_params, which is NOT in the manifest, so Perry silently no-op'd it and every material's params array was dropped. Reroute through the working _scratch path. - createTextureArray/createTextureArrayEx were exported but uncallable from Perry (number[] into an i64 pointer). Reroute through the same scratch path so they work; non-breaking (no Perry caller could reach them before). Audio RT-safety (audio/render.rs): - The 65th concurrent voice reallocated the voices Vec ON the audio thread. Add voice-stealing (drop the quietest) so the render thread never grows past its preallocated cap; same for music. Add 2 regression tests to a file that had none. Docs: - README: Windows audio XAudio2 -> WASAPI, Linux PulseAudio -> ALSA, drop the Wayland claim. CLAUDE.md: same ALSA fix. - lumen-roadmap platform matrix: HW ray-query is opt-in, not auto. - pt-roadmap ticket table: add the shipped PT-3b/6/7/8 rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two audit findings: the shared gamepad FFI reads engine().input, but only tvOS/visionOS ever populated it — macOS and iOS had NO gamepad support (isGamepadAvailable permanently false), despite the EN-031 ticket claiming "a pad drives the shooter on iPhone." And iOS/tvOS/visionOS bring up RemoteIO with no AVAudioSession, so audio defaults to SoloAmbient (silenced by the ring/silent switch, stops on screen lock). Gamepad (macOS compile-verified; iOS identical code, blind): - Poll the first connected controller via the GameController framework, resolved dynamically through the ObjC runtime (no GameController link dep). No-op when the framework or a controller is absent. Button/axis indices match the tvOS/visionOS map so controls carry across targets. - Wrote the CORRECT GameController API: GCControllerAxisInput.value and GCControllerButtonInput.value are ObjC `float`, read as f32. The tvOS reference reads them as f64 (an ABI mismatch — float and double return in different-width registers) and uses a bogus `xAxis_value` selector; those crates aren't in CI so it was never caught. This version is right. - Shared input::reset_gamepad() clears polled axes/buttons at the top of a poll so a released button reads released (begin_frame does not reset gamepad state — the same latent stuck-button bug the tvOS poller has). iOS AVAudioSession (blind; the msg_send syntax was type-checked via an equivalent macOS snippet): - Set the Playback category + activate before RemoteIO so game audio isn't killed by the silent switch. Done via the ObjC runtime; the category constant's NSString value equals its symbol name, so no AVFAudio link. VERIFICATION STATUS: macOS gamepad compiles + is the reference. iOS gamepad is byte-identical logic to the compiled macOS version; iOS AVAudioSession msg_send patterns type-check on macOS. The iOS crate itself cannot be compiled in this environment (its minimp3/Jolt C deps need the iOS SDK), so the iOS paths need an on-device build+run to fully confirm. tvOS/visionOS gamepad ABI fix and the lifecycle/backgrounding handlers are NOT in this commit — see the PR description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request centralizes renderer uniform types, adds native controller and audio handling, migrates TypeScript FFI operations to scratch buffers, exports key-repeat queries, and updates architecture and rendering roadmap documentation. ChangesRenderer type centralization
Native input and audio runtime
Scratch-buffer FFI paths
Architecture and roadmap documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/perf/lumen-roadmap.md`:
- Around line 41-44: Align the as-built correction with the platform behavior
documented in the PT roadmap: either narrow the opt-in statement to platforms
gated by BLOOM_HW_GI, BLOOM_PT, or --pt, or update the platform defaults so
macOS/Linux and Windows share the same contract. Ensure the “HW path”
capability-versus-default clarification remains accurate across both roadmap
documents.
In `@src/models/index.ts`:
- Around line 588-590: Update both texel packing expressions in
src/models/index.ts at lines 588-590 and 630-632 to wrap the bitwise combination
in parentheses and apply an unsigned right shift (>>> 0), ensuring values with
the high bit set remain positive unsigned 32-bit values before being passed to
bloom_mesh_scratch_push_u32.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 74a871e4-d305-47c7-8f1a-8ef98b45c51f
📒 Files selected for processing (15)
CLAUDE.mdREADME.mddocs/perf/lumen-roadmap.mddocs/pt/pt-roadmap.mdnative/ios/src/lib.rsnative/macos/src/lib.rsnative/shared/src/audio/render.rsnative/shared/src/input.rsnative/shared/src/renderer/mod.rsnative/shared/src/renderer/types.rsnative/watchos/src/ffi_stubs.rsnative/web/src/input_ffi.rssrc/core/index.tssrc/models/index.tstools/file-lines-baseline.json
💤 Files with no reviewable changes (1)
- src/core/index.ts
Comment on lines
+41
to
+44
| > **As-built correction (2026-07):** the "HW path" column below describes | ||
| > *capability*, not the default. The HW ray-query path is **opt-in** | ||
| > everywhere (`BLOOM_HW_GI=1` / `BLOOM_PT` / `--pt`); SW GI is the shipping | ||
| > default on every platform. See the as-built note under the table. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the opt-in claim with the actual platform behavior.
Line 43 says HW ray-query is opt-in everywhere, but docs/pt/pt-roadmap.md Lines 105-110 says macOS/Linux request ray query by default and only Windows uses the explicit opt-in gates. Narrow this note to the platforms that implement opt-in gating, or update the platform behavior so both roadmaps describe the same contract.
🤖 Prompt for 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.
In `@docs/perf/lumen-roadmap.md` around lines 41 - 44, Align the as-built
correction with the platform behavior documented in the PT roadmap: either
narrow the opt-in statement to platforms gated by BLOOM_HW_GI, BLOOM_PT, or
--pt, or update the platform defaults so macOS/Linux and Windows share the same
contract. Ensure the “HW path” capability-versus-default clarification remains
accurate across both roadmap documents.
Comment on lines
+588
to
+590
| bloom_mesh_scratch_push_u32( | ||
| bytes[b] | (bytes[b + 1] << 8) | (bytes[b + 2] << 16) | (bytes[b + 3] << 24), | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Prevent texel data corruption by using unsigned bitwise shift.
In JavaScript and TypeScript, bitwise operators return signed 32-bit integers. If the fourth byte (e.g., an alpha channel of 128 or greater) sets the highest bit, the packed result evaluates as a negative integer. When this negative float is passed across the FFI to Rust and cast to u32, it saturates to 0 (since Rust 1.45+), resulting in completely corrupted texels. Appending >>> 0 forces the result to an unsigned 32-bit integer, preserving the bit pattern correctly as a positive f64.
src/models/index.ts#L588-L590: wrap the bitwise packing expression in parentheses and apply>>> 0.src/models/index.ts#L630-L632: apply the same>>> 0unsigned right shift to the texel packing logic.
🐛 Proposed fix
- bytes[b] | (bytes[b + 1] << 8) | (bytes[b + 2] << 16) | (bytes[b + 3] << 24),
+ ((bytes[b] | (bytes[b + 1] << 8) | (bytes[b + 2] << 16) | (bytes[b + 3] << 24)) >>> 0),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| bloom_mesh_scratch_push_u32( | |
| bytes[b] | (bytes[b + 1] << 8) | (bytes[b + 2] << 16) | (bytes[b + 3] << 24), | |
| ); | |
| bloom_mesh_scratch_push_u32( | |
| ((bytes[b] | (bytes[b + 1] << 8) | (bytes[b + 2] << 16) | (bytes[b + 3] << 24)) >>> 0), | |
| ); |
📍 Affects 1 file
src/models/index.ts#L588-L590(this comment)src/models/index.ts#L630-L632
🤖 Prompt for 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.
In `@src/models/index.ts` around lines 588 - 590, Update both texel packing
expressions in src/models/index.ts at lines 588-590 and 630-632 to wrap the
bitwise combination in parentheses and apply an unsigned right shift (>>> 0),
ensuring values with the high bit set remain positive unsigned 32-bit values
before being passed to bloom_mesh_scratch_push_u32.
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
1 participant
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.
Second batch from the 2026-07-19 audit — the Apple native work. Unlike #124, parts of this need on-device testing: this environment has only CommandLineTools (no Xcode/iOS SDK), so the iOS crate can't be compiled here (its minimp3/Jolt C deps need the SDK). Verification status is called out per-item below.
Gamepad — macOS & iOS
Two audit findings in one: the shared gamepad FFI reads
engine().input, but only tvOS/visionOS ever populated it — macOS and iOS had zero gamepad support (isGamepadAvailablepermanently false), despite EN-031 being marked "shipped — a pad drives the shooter on iPhone." It never did on Apple.GCControllerAxisInput.value(an ObjCfloat) asf64— an ABI mismatch (float vs double return in different-width registers) — and calls a bogusxAxis_valueselector. Those crates aren't in CI, so it was never caught. This version uses the correct two-stepxAxis→valueasf32.input::reset_gamepad()clears polled axes/buttons at the top of each poll so a released button reads released (begin_framedoesn't reset gamepad state — the same latent stuck-button bug the tvOS poller has).Verification: macOS compiles and is the reference implementation. iOS is byte-identical logic.
iOS AVAudioSession
iOS/tvOS/visionOS brought up RemoteIO with no AVAudioSession, so audio defaulted to SoloAmbient — silenced by the ring/silent switch, stopped on screen-lock. Set the Playback category + activate before RemoteIO. Done via the ObjC runtime; the category constant's NSString value equals its symbol name, so no AVFAudio link needed.
Verification: the
msg_sendpatterns (setCategory:error:,setActive:error:,NSString::from_str,Boolreturn) were type-checked via an equivalent snippet compiled on macOS (all dynamicAnyClass/AnyObject, so it type-checks without the class existing).What I deliberately did NOT do, and why
acquire_frame(draw2d.rs:100) already handles every surface-loss error by logging, reconfiguring, and skipping the frame — no panic. So backgrounding doesn't crash; explicit observers would be a battery/behavior refinement, not a crash fix. Not worth writing blind NSNotification/scene-delegate wiring for.f64/xAxis_valuebug above) — a correctness fix to code I can't compile here; lower priority since it only triggers when a controller connects. Straightforward to port from this PR's correct macOS/iOS version.Test plan (needs a Mac with Xcode)
isGamepadAvailabletrue, sticks/buttons/triggers/d-pad drive input.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation