feat: implement request_animation_frame + cancel_animation_frame with…#34
Merged
Conversation
… raf-demo example Adds vsync-aligned one-shot animation frame callbacks (queues via new HostState field, drained in LiveModule::tick before timers/on_frame, fires via existing on_timer callback). Reuses timer ID/counter pattern per CLAUDE.md guidelines for host capabilities. No changes to guest contract or on_frame export. Includes: - AnimationRequest struct + drain helper in capabilities.rs - SDK wrappers + updated API tables/docs - New examples/raf-demo (bouncing ball physics in rAF callback, rendering + UI in on_frame) - Workspace Cargo.toml update Why: Fulfills ROADMAP Phase 4 priority for better animation control (decouples update from render loop). All checks (fmt, clippy, build, wasm) pass. Follows surgical/minimal changes rule. (Closes implicit request from previous session) Made-with: Cursor
📝 WalkthroughWalkthroughThe PR introduces requestAnimationFrame (rAF) support to the oxide runtime. A new Changes
Sequence DiagramsequenceDiagram
participant Guest as Guest (WASM App)
participant SDK as SDK<br/>(oxide-sdk)
participant Host as Host Runtime<br/>(oxide-browser)
Guest->>SDK: request_animation_frame(callback_id)
SDK->>Host: _api_request_animation_frame()
Host->>Host: Allocate ID, queue AnimationRequest
Host->>SDK: Return request_id
SDK->>Guest: Return request_id
Note over Host: Each frame tick
Host->>Host: drain_animation_frame_requests()
Host->>Host: For each queued callback_id:<br/>set fuel, call on_timer(callback_id)
Host->>Guest: on_timer(callback_id)
Guest->>Guest: Update animation state
Host->>Guest: on_frame(dt_ms)
Guest->>Guest: Render animation
Guest->>SDK: cancel_animation_frame(request_id)
SDK->>Host: _api_cancel_animation_frame(request_id)
Host->>Host: Remove matching request from queue
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 136-141: Update the CLAUDE.md WASM contract text so item 3
explicitly states that the exported function on_timer(callback_id: u32) is
called not only for set_timeout/set_interval but also for animation-frame
callbacks delivered via request_animation_frame; change the wording of the third
bullet (and any nearby references to on_timer) to mention "set_timeout /
set_interval and request_animation_frame (animation-frame callbacks)" so readers
know on_timer handles both timer and animation-frame events.
- Around line 34-54: The fenced code block that lists the repository tree lacks
a language tag and triggers markdownlint MD040; update the opening fence from
``` to ```text (i.e., add the "text" language) for the diagram block so the
linter recognizes it as a code block (the block beginning with the
triple-backticks that contains the oxide/ tree diagram).
In `@examples/raf-demo/src/lib.rs`:
- Around line 38-45: When clamping BALL_X/BALL_Y ensure the clamp's min <= max
to avoid panics when the canvas is smaller than the ball diameter: compute safe
bounds first (e.g. let min_x = radius; let max_x = (width - radius).max(min_x))
and use BALL_X = BALL_X.clamp(min_x, max_x) (and similarly for Y with
min_y/max_y), keeping the existing bounce logic that flips VX/VY; reference the
symbols BALL_X, BALL_Y, VX, VY, radius, width, height, and clamp.
In `@oxide-browser/src/capabilities.rs`:
- Around line 1437-1463: Pending animation_requests stored on HostState can
survive across guest loads; ensure the animation_requests queue is reset on
module reload/navigation the same way other per-page state is cleared. In the
BrowserHost::fetch_and_run and BrowserHost::run_bytes paths, acquire
HostState.animation_requests (the Mutex<Vec<AnimationRequest>> used by the
api_request_animation_frame/api_cancel_animation_frame handlers) and clear or
replace it with an empty vector when initializing a new module/page so no stale
AnimationRequest entries from a previous guest can fire against the new guest's
on_timer.
In `@oxide-browser/src/runtime.rs`:
- Around line 54-76: When on_timer_fn is None the existing rAF queue (populated
by api_request_animation_frame) is left intact; drain the pending animation
frame requests even when self.on_timer_fn is missing by calling
drain_animation_frame_requests against self.store.data().animation_requests and
for each returned callback_id emit a clear failure log (via
crate::capabilities::console_log with ConsoleLevel::Error) indicating on_timer
is not exported (e.g., "request_animation_frame callback {callback_id} could not
be delivered: on_timer not exported"), and ensure any fuel or other state
cleanup as done in the on_timer branch is handled or skipped safely; reference
drain_animation_frame_requests, api_request_animation_frame, self.on_timer_fn,
and self.store.data().animation_requests to locate where to add this behavior.
🪄 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
Run ID: 755c6ef7-4674-4902-903a-05c3b8dcb512
📒 Files selected for processing (7)
CLAUDE.mdCargo.tomlexamples/raf-demo/Cargo.tomlexamples/raf-demo/src/lib.rsoxide-browser/src/capabilities.rsoxide-browser/src/runtime.rsoxide-sdk/src/lib.rs
Comment on lines
+34
to
+54
| ``` | ||
| oxide/ | ||
| ├── oxide-browser/src/ | ||
| │ ├── capabilities.rs # All host functions registered into the wasmtime Linker | ||
| │ ├── engine.rs # WasmEngine, SandboxPolicy, fuel/memory bounds | ||
| │ ├── runtime.rs # BrowserHost — fetch, compile, instantiate, frame loop | ||
| │ ├── ui.rs # GPUI shell — toolbar, canvas, console, widgets | ||
| │ ├── navigation.rs # History stack | ||
| │ ├── url.rs # URL parsing (http, https, file, oxide schemes) | ||
| │ ├── rtc.rs # WebRTC (register_rtc_functions) | ||
| │ ├── websocket.rs # WebSocket (register_ws_functions) | ||
| │ └── gpu.rs, audio_format.rs, video.rs, media_capture.rs, … | ||
| ├── oxide-sdk/src/ | ||
| │ ├── lib.rs # FFI declarations + safe public wrappers | ||
| │ └── proto.rs # Zero-dependency protobuf codec | ||
| └── examples/ | ||
| ├── hello-oxide/ # Minimal guest app | ||
| ├── ws-chat/ # WebSocket demo | ||
| ├── rtc-chat/ # WebRTC demo | ||
| └── … | ||
| ``` |
There was a problem hiding this comment.
Add a language to this fenced block.
This trips markdownlint (MD040) as written.
Suggested fix
-```
+```text
oxide/
├── oxide-browser/src/
│ ├── capabilities.rs # All host functions registered into the wasmtime Linker
│ ├── engine.rs # WasmEngine, SandboxPolicy, fuel/memory bounds
│ ├── runtime.rs # BrowserHost — fetch, compile, instantiate, frame loop
│ ├── ui.rs # GPUI shell — toolbar, canvas, console, widgets
│ ├── navigation.rs # History stack
│ ├── url.rs # URL parsing (http, https, file, oxide schemes)
│ ├── rtc.rs # WebRTC (register_rtc_functions)
│ ├── websocket.rs # WebSocket (register_ws_functions)
│ └── gpu.rs, audio_format.rs, video.rs, media_capture.rs, …
├── oxide-sdk/src/
│ ├── lib.rs # FFI declarations + safe public wrappers
│ └── proto.rs # Zero-dependency protobuf codec
└── examples/
├── hello-oxide/ # Minimal guest app
├── ws-chat/ # WebSocket demo
├── rtc-chat/ # WebRTC demo
└── …</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 34 - 54, The fenced code block that lists the
repository tree lacks a language tag and triggers markdownlint MD040; update the
opening fence from ``` to ```text (i.e., add the "text" language) for the
diagram block so the linter recognizes it as a code block (the block beginning
with the triple-backticks that contains the oxide/ tree diagram).
Comment on lines
+136
to
+141
| Every `.wasm` module must: | ||
| 1. Export `start_app()` — called once on load. | ||
| 2. Optionally export `on_frame(dt_ms: u32)` — called every frame (fuel replenished each call). | ||
| 3. Optionally export `on_timer(callback_id: u32)` — called when a `set_timeout`/`set_interval` fires. | ||
| 4. Compile as `[lib] crate-type = ["cdylib"]` targeting `wasm32-unknown-unknown`. | ||
| 5. Import everything from the `"oxide"` wasm import module — never WASI. |
There was a problem hiding this comment.
Document that on_timer also receives animation-frame callbacks.
This contract is now stale: the runtime delivers request_animation_frame through on_timer(callback_id), not just set_timeout / set_interval.
Suggested fix
-3. Optionally export `on_timer(callback_id: u32)` — called when a `set_timeout`/`set_interval` fires.
+3. Optionally export `on_timer(callback_id: u32)` — called when a `set_timeout`/`set_interval` fires, and for `request_animation_frame` callbacks.📝 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
| Every `.wasm` module must: | |
| 1. Export `start_app()` — called once on load. | |
| 2. Optionally export `on_frame(dt_ms: u32)` — called every frame (fuel replenished each call). | |
| 3. Optionally export `on_timer(callback_id: u32)` — called when a `set_timeout`/`set_interval` fires. | |
| 4. Compile as `[lib] crate-type = ["cdylib"]` targeting `wasm32-unknown-unknown`. | |
| 5. Import everything from the `"oxide"` wasm import module — never WASI. | |
| Every `.wasm` module must: | |
| 1. Export `start_app()` — called once on load. | |
| 2. Optionally export `on_frame(dt_ms: u32)` — called every frame (fuel replenished each call). | |
| 3. Optionally export `on_timer(callback_id: u32)` — called when a `set_timeout`/`set_interval` fires, and for `request_animation_frame` callbacks. | |
| 4. Compile as `[lib] crate-type = ["cdylib"]` targeting `wasm32-unknown-unknown`. | |
| 5. Import everything from the `"oxide"` wasm import module — never WASI. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 136 - 141, Update the CLAUDE.md WASM contract text so
item 3 explicitly states that the exported function on_timer(callback_id: u32)
is called not only for set_timeout/set_interval but also for animation-frame
callbacks delivered via request_animation_frame; change the wording of the third
bullet (and any nearby references to on_timer) to mention "set_timeout /
set_interval and request_animation_frame (animation-frame callbacks)" so readers
know on_timer handles both timer and animation-frame events.
Comment on lines
+38
to
+45
| // Bounce off walls | ||
| if BALL_X - radius < 0.0 || BALL_X + radius > width { | ||
| VX = -VX; | ||
| BALL_X = BALL_X.clamp(radius, width - radius); | ||
| } | ||
| if BALL_Y - radius < 0.0 || BALL_Y + radius > height { | ||
| VY = -VY; | ||
| BALL_Y = BALL_Y.clamp(radius, height - radius); |
There was a problem hiding this comment.
Guard the clamp bounds for tiny canvases.
If the canvas shrinks below the ball diameter, width - radius / height - radius becomes smaller than radius, and f32::clamp will panic. Resizing the window small enough will trap the demo.
Suggested fix
if BALL_X - radius < 0.0 || BALL_X + radius > width {
VX = -VX;
- BALL_X = BALL_X.clamp(radius, width - radius);
+ if width <= radius * 2.0 {
+ BALL_X = width * 0.5;
+ } else {
+ BALL_X = BALL_X.clamp(radius, width - radius);
+ }
}
if BALL_Y - radius < 0.0 || BALL_Y + radius > height {
VY = -VY;
- BALL_Y = BALL_Y.clamp(radius, height - radius);
+ if height <= radius * 2.0 {
+ BALL_Y = height * 0.5;
+ } else {
+ BALL_Y = BALL_Y.clamp(radius, height - radius);
+ }
}📝 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
| // Bounce off walls | |
| if BALL_X - radius < 0.0 || BALL_X + radius > width { | |
| VX = -VX; | |
| BALL_X = BALL_X.clamp(radius, width - radius); | |
| } | |
| if BALL_Y - radius < 0.0 || BALL_Y + radius > height { | |
| VY = -VY; | |
| BALL_Y = BALL_Y.clamp(radius, height - radius); | |
| // Bounce off walls | |
| if BALL_X - radius < 0.0 || BALL_X + radius > width { | |
| VX = -VX; | |
| if width <= radius * 2.0 { | |
| BALL_X = width * 0.5; | |
| } else { | |
| BALL_X = BALL_X.clamp(radius, width - radius); | |
| } | |
| } | |
| if BALL_Y - radius < 0.0 || BALL_Y + radius > height { | |
| VY = -VY; | |
| if height <= radius * 2.0 { | |
| BALL_Y = height * 0.5; | |
| } else { | |
| BALL_Y = BALL_Y.clamp(radius, height - radius); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/raf-demo/src/lib.rs` around lines 38 - 45, When clamping
BALL_X/BALL_Y ensure the clamp's min <= max to avoid panics when the canvas is
smaller than the ball diameter: compute safe bounds first (e.g. let min_x =
radius; let max_x = (width - radius).max(min_x)) and use BALL_X =
BALL_X.clamp(min_x, max_x) (and similarly for Y with min_y/max_y), keeping the
existing bounce logic that flips VX/VY; reference the symbols BALL_X, BALL_Y,
VX, VY, radius, width, height, and clamp.
Comment on lines
+1437
to
+1463
| linker.func_wrap( | ||
| "oxide", | ||
| "api_request_animation_frame", | ||
| |caller: Caller<'_, HostState>, callback_id: u32| -> u32 { | ||
| let mut next = caller.data().timer_next_id.lock().unwrap(); | ||
| let id = *next; | ||
| *next = next.wrapping_add(1).max(1); | ||
| drop(next); | ||
|
|
||
| let req = AnimationRequest { id, callback_id }; | ||
| caller.data().animation_requests.lock().unwrap().push(req); | ||
| id | ||
| }, | ||
| )?; | ||
|
|
||
| linker.func_wrap( | ||
| "oxide", | ||
| "api_cancel_animation_frame", | ||
| |caller: Caller<'_, HostState>, request_id: u32| { | ||
| caller | ||
| .data() | ||
| .animation_requests | ||
| .lock() | ||
| .unwrap() | ||
| .retain(|r| r.id != request_id); | ||
| }, | ||
| )?; |
There was a problem hiding this comment.
Clear this queue on module reload/navigation.
HostState is reused across loads, so pending animation_requests from the previous guest can survive into the next module and fire against the new guest's on_timer. The new queue needs the same reset treatment as other per-page state.
Suggested fix
# in `oxide-browser/src/runtime.rs`, alongside the existing per-load clears
self.host_state.canvas.lock().unwrap().commands.clear();
self.host_state.console.lock().unwrap().clear();
self.host_state.hyperlinks.lock().unwrap().clear();
+self.host_state.animation_requests.lock().unwrap().clear();Apply the same reset in both BrowserHost::fetch_and_run and BrowserHost::run_bytes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@oxide-browser/src/capabilities.rs` around lines 1437 - 1463, Pending
animation_requests stored on HostState can survive across guest loads; ensure
the animation_requests queue is reset on module reload/navigation the same way
other per-page state is cleared. In the BrowserHost::fetch_and_run and
BrowserHost::run_bytes paths, acquire HostState.animation_requests (the
Mutex<Vec<AnimationRequest>> used by the
api_request_animation_frame/api_cancel_animation_frame handlers) and clear or
replace it with an empty vector when initializing a new module/page so no stale
AnimationRequest entries from a previous guest can fire against the new guest's
on_timer.
Comment on lines
54
to
+76
| if let Some(ref on_timer) = self.on_timer_fn { | ||
| // Animation frames first (vsync-aligned, one-shot). | ||
| let anim = self.store.data().animation_requests.clone(); | ||
| let fired_anim = drain_animation_frame_requests(&anim); | ||
| for callback_id in fired_anim { | ||
| self.store | ||
| .set_fuel(FRAME_FUEL_LIMIT) | ||
| .context("failed to set animation frame fuel")?; | ||
| if let Err(e) = on_timer.call(&mut self.store, callback_id) { | ||
| let msg = if e.to_string().contains("fuel") { | ||
| format!("on_timer(raf:{callback_id}) fuel limit exceeded") | ||
| } else { | ||
| format!("on_timer(raf:{callback_id}) trapped: {e}") | ||
| }; | ||
| crate::capabilities::console_log( | ||
| &self.store.data().console, | ||
| crate::capabilities::ConsoleLevel::Error, | ||
| msg, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Regular timers. |
There was a problem hiding this comment.
Don't leave queued rAF requests behind when on_timer is missing.
This block is skipped entirely when the module does not export on_timer, but api_request_animation_frame can still enqueue requests. A guest that calls request_animation_frame from start_app or on_frame will silently accumulate pending entries forever instead of getting a clear failure mode.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@oxide-browser/src/runtime.rs` around lines 54 - 76, When on_timer_fn is None
the existing rAF queue (populated by api_request_animation_frame) is left
intact; drain the pending animation frame requests even when self.on_timer_fn is
missing by calling drain_animation_frame_requests against
self.store.data().animation_requests and for each returned callback_id emit a
clear failure log (via crate::capabilities::console_log with
ConsoleLevel::Error) indicating on_timer is not exported (e.g.,
"request_animation_frame callback {callback_id} could not be delivered: on_timer
not exported"), and ensure any fuel or other state cleanup as done in the
on_timer branch is handled or skipped safely; reference
drain_animation_frame_requests, api_request_animation_frame, self.on_timer_fn,
and self.store.data().animation_requests to locate where to add this behavior.
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.
… raf-demo example
Adds vsync-aligned one-shot animation frame callbacks (queues via new HostState field, drained in LiveModule::tick before timers/on_frame, fires via existing on_timer callback). Reuses timer ID/counter pattern per CLAUDE.md guidelines for host capabilities. No changes to guest contract or on_frame export.
Includes:
Why: Fulfills ROADMAP Phase 4 priority for better animation control (decouples update from render loop). All checks (fmt, clippy, build, wasm) pass. Follows surgical/minimal changes rule.
(Closes implicit request from previous session)
Made-with: Cursor
Summary by CodeRabbit
New Features
request_animation_frame()andcancel_animation_frame()APIs for scheduling one-shot animation callbacks, with callbacks routed through the existingon_timerhandler and processed before regular timers each frame.Documentation