Studio Mode movement, motion blur and styling overhaul #1370
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds cursor physics and presets, motion-blur and zoom analysis, corner-rounding support, cursor hotspot updates, spring-mass-damper and interpolation changes, shader and uniform updates, editor serialization for corner styles, and a desktop package version bump to 0.3.83. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Editor UI
participant Context as Editor Context
participant Renderer as Rendering
participant Interp as Cursor Interpolator
participant Spring as Spring Simulation
participant Shader as GPU Shader
Editor->>Context: change cursor preset / corner style
Context->>Context: applyCursorStylePreset / setCursorPhysics
Editor->>Context: save project
Context->>Renderer: serializeProjectConfiguration(project)
Renderer->>Interp: provide cursor events & smoothing config
Interp->>Interp: densify_cursor_moves()
Interp->>Spring: run(dt_ms) per tick
Spring-->>Interp: position + velocity
Interp->>Renderer: fill uniforms (motion_blur_vector, prev_cursor, rounding_type)
Renderer->>Shader: upload uniforms (motion_blur_params, zoom_center, rounding_type)
Shader->>Shader: select blur mode (directional or zoom) and apply rounding
Shader-->>Renderer: rendered frame
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
richiemcilroy
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/rendering/src/cursor_interpolation.rs (1)
153-177: Interpolate raw cursor positionsIn the non-smoothed branch we now return
c.x/c.y(the earlier sample) for every time betweenchunk[0]andchunk[1]. That turns the raw cursor into a step function until the next event arrives, which is a regression from the previous continuous motion. Please interpolate between the two samples using the elapsed time before constructingInterpolatedCursorPosition. (forums.autodesk.com)- Some(( - XY::new(c.x as f32, c.y as f32), - c.cursor_id.clone(), - velocity, - )) + let alpha = ((time_ms - c.time_ms) as f32 / delta_ms).clamp(0.0, 1.0); + let pos = XY::new( + (c.x + (next.x - c.x) * alpha) as f32, + (c.y + (next.y - c.y) * alpha) as f32, + ); + Some((pos, c.cursor_id.clone(), velocity))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
Cargo.lockis excluded by!**/*.lockcrates/cursor-info/assets/mac/arrow.svgis excluded by!**/*.svgcrates/cursor-info/assets/mac/ibeam.svgis excluded by!**/*.svgcrates/cursor-info/assets/mac/pointing_hand.svgis excluded by!**/*.svgcrates/cursor-info/assets/mac/tahoe/default.svgis excluded by!**/*.svgcrates/cursor-info/assets/mac/tahoe/pointer.svgis excluded by!**/*.svgcrates/cursor-info/assets/mac/tahoe/tahoe.svgis excluded by!**/*.svgcrates/cursor-info/assets/mac/tahoe/text.svgis excluded by!**/*.svgcrates/cursor-info/assets/windows/arrow.svgis excluded by!**/*.svgcrates/cursor-info/assets/windows/hand.svgis excluded by!**/*.svgcrates/cursor-info/assets/windows/ibeam.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/src/camera.wgsl(2 hunks)apps/desktop/src-tauri/src/recording.rs(2 hunks)apps/desktop/src/routes/camera.tsx(2 hunks)apps/desktop/src/routes/editor/ConfigSidebar.tsx(12 hunks)apps/desktop/src/routes/editor/Player.tsx(2 hunks)apps/desktop/src/routes/editor/context.ts(2 hunks)apps/desktop/src/utils/tauri.ts(2 hunks)crates/cursor-info/cursors.html(8 hunks)crates/cursor-info/src/macos.rs(6 hunks)crates/cursor-info/src/windows.rs(2 hunks)crates/project/src/configuration.rs(7 hunks)crates/rendering/src/composite_frame.rs(2 hunks)crates/rendering/src/cursor_interpolation.rs(8 hunks)crates/rendering/src/layers/cursor.rs(5 hunks)crates/rendering/src/lib.rs(13 hunks)crates/rendering/src/shaders/composite-video-frame.wgsl(7 hunks)crates/rendering/src/shaders/cursor.wgsl(2 hunks)crates/rendering/src/spring_mass_damper.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/recording.rscrates/rendering/src/layers/cursor.rscrates/cursor-info/src/macos.rscrates/rendering/src/spring_mass_damper.rscrates/rendering/src/cursor_interpolation.rscrates/rendering/src/composite_frame.rscrates/project/src/configuration.rscrates/rendering/src/lib.rscrates/cursor-info/src/windows.rs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/camera.tsxapps/desktop/src/routes/editor/Player.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/context.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/camera.tsxapps/desktop/src/routes/editor/Player.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/context.ts
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/rendering/src/layers/cursor.rscrates/cursor-info/src/macos.rscrates/rendering/src/spring_mass_damper.rscrates/rendering/src/shaders/cursor.wgslcrates/rendering/src/cursor_interpolation.rscrates/rendering/src/composite_frame.rscrates/project/src/configuration.rscrates/rendering/src/shaders/composite-video-frame.wgslcrates/rendering/src/lib.rscrates/cursor-info/src/windows.rs
**/tauri.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not edit auto-generated files named
tauri.ts.
Files:
apps/desktop/src/utils/tauri.ts
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated files named `tauri.ts`.
Applied to files:
apps/desktop/src/utils/tauri.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/desktop/**/src-tauri/gen/** : Do not edit auto-generated files in `apps/desktop/src-tauri/gen/`.
Applied to files:
apps/desktop/src-tauri/Cargo.toml
🧬 Code graph analysis (8)
crates/rendering/src/layers/cursor.rs (2)
crates/rendering/src/composite_frame.rs (1)
new(93-105)crates/rendering/src/lib.rs (7)
new(70-140)new(323-366)new(406-408)new(992-1436)new(1452-1457)new(1504-1514)new(1657-1690)
apps/desktop/src/routes/editor/Player.tsx (1)
apps/desktop/src/routes/editor/context.ts (1)
serializeProjectConfiguration(99-118)
crates/rendering/src/cursor_interpolation.rs (3)
crates/rendering/src/lib.rs (8)
new(70-140)new(323-366)new(406-408)new(992-1436)new(1452-1457)new(1504-1514)new(1657-1690)cursor_move(1908-1916)crates/rendering/src/spring_mass_damper.rs (1)
new(22-31)crates/project/src/cursor.rs (1)
moves(209-213)
crates/rendering/src/composite_frame.rs (2)
crates/project/src/configuration.rs (8)
default(54-58)default(240-247)default(251-264)default(329-346)default(350-356)default(382-390)default(464-488)default(629-644)crates/rendering/src/lib.rs (1)
default(636-644)
crates/project/src/configuration.rs (2)
apps/desktop/src/utils/tauri.ts (3)
CornerStyle(384-384)CursorAnimationStyle(389-389)CursorType(392-392)crates/rendering/src/composite_frame.rs (1)
default(40-66)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (4)
apps/desktop/src/routes/editor/context.ts (1)
CornerRoundingType(64-64)apps/desktop/src/utils/tauri.ts (1)
CursorAnimationStyle(389-389)crates/project/src/configuration.rs (3)
preset(419-433)div(139-144)div(150-155)apps/desktop/src/routes/editor/ui.tsx (6)
Field(25-47)Slider(65-147)MenuItem(264-279)PopperContent(287-293)topSlideAnimateClasses(432-433)MenuItemList(295-309)
apps/desktop/src/routes/editor/context.ts (1)
apps/desktop/src/utils/tauri.ts (3)
ProjectConfiguration(447-447)SerializedEditorInstance(470-470)commands(7-292)
crates/rendering/src/lib.rs (4)
apps/desktop/src/utils/tauri.ts (8)
AspectRatio(350-350)CameraShape(370-370)CameraXPosition(371-371)CameraYPosition(372-372)ClipOffsets(382-382)CornerStyle(384-384)Crop(385-385)XY(499-499)crates/project/src/configuration.rs (9)
new(69-71)default(54-58)default(240-247)default(251-264)default(329-346)default(350-356)default(382-390)default(464-488)default(629-644)crates/rendering/src/composite_frame.rs (2)
new(93-105)default(40-66)crates/rendering/src/cursor_interpolation.rs (1)
interpolate_cursor(111-179)
🪛 GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
crates/rendering/src/lib.rs
[warning] 430-430: methods with the following characteristics: (to_* and self type is Copy) usually take self by value
warning: methods with the following characteristics: (to_* and self type is Copy) usually take self by value
--> crates/rendering/src/lib.rs:430:14
|
430 | fn to_uv(&self, point: XY) -> XY {
| ^^^^^
|
= help: consider choosing a less ambiguous name
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#wrong_self_convention
= note: #[warn(clippy::wrong_self_convention)] on by default
🔇 Additional comments (5)
apps/desktop/src-tauri/Cargo.toml (1)
3-3: Version bump is appropriate.The patch version increment from 0.3.82 to 0.3.83 aligns with the feature and styling updates (motion blur, physics-based cursor motion, corner rounding styles) described in the PR. No dependencies have changed, and the manifest structure remains intact.
crates/cursor-info/src/windows.rs (1)
73-73: LGTM! Hotspot coordinates updated correctly.The hotspot coordinate updates for Arrow, IBeam, and Hand cursors are within valid ranges and maintain consistent precision. These changes align with the corresponding updates in the HTML testing file.
Also applies to: 77-77, 117-117
crates/cursor-info/src/macos.rs (1)
74-74: LGTM! Hotspot coordinates updated correctly.The hotspot coordinate updates for both standard and Tahoe cursor variants (Arrow, IBeam, PointingHand) are within valid ranges and maintain consistent precision. These changes align with the corresponding updates in the HTML testing file.
Also applies to: 99-99, 111-111, 144-144, 170-170, 182-182
crates/cursor-info/cursors.html (1)
166-166: LGTM! Hotspot data synchronized correctly.The hotspot coordinate updates in this testing utility are fully consistent with the corresponding changes in
windows.rsandmacos.rs. All values are within valid ranges and maintain appropriate precision. The HTML visualization tool provides a good way to validate hotspot positioning during development.Also applies to: 202-202, 220-220, 268-268, 311-311, 329-329, 391-391, 397-397, 433-433
crates/rendering/src/spring_mass_damper.rs (1)
49-72: Verify callers switched to millisecond input before merging.
runused to treatdtas seconds; now it expects milliseconds and converts back with/ 1000.0. If any existing caller is still passing a seconds-based delta (which was the previous contract), the simulation will run 1000× slower. Please confirm every call site is now supplying milliseconds (or consider keeping the API in seconds and doing the conversion internally).
| if (motion_len < 1e-4 || blur_strength < 0.001) { | ||
| return textureSample(t_cursor, s_cursor, input.uv) * opacity; | ||
| } |
There was a problem hiding this comment.
Restore shadow blend on the no-blur path
The new early-exit returns textureSample(...) * opacity directly, so whenever motion is below the blur threshold the shader skips mix(shadow_color, …) entirely. That removes the cursor shadow for the common “not moving” case. Please return the same shadow-composited color here. (community.adobe.com)
- if (motion_len < 1e-4 || blur_strength < 0.001) {
- return textureSample(t_cursor, s_cursor, input.uv) * opacity;
- }
+ if (motion_len < 1e-4 || blur_strength < 0.001) {
+ let sample = textureSample(t_cursor, s_cursor, input.uv) * opacity;
+ return mix(shadow_color, sample, sample.a);
+ }📝 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.
| if (motion_len < 1e-4 || blur_strength < 0.001) { | |
| return textureSample(t_cursor, s_cursor, input.uv) * opacity; | |
| } | |
| if (motion_len < 1e-4 || blur_strength < 0.001) { | |
| let sample = textureSample(t_cursor, s_cursor, input.uv) * opacity; | |
| return mix(shadow_color, sample, sample.a); | |
| } |
🤖 Prompt for AI Agents
In crates/rendering/src/shaders/cursor.wgsl around lines 69–71, the early return
bypasses the shadow compositing and returns textureSample(...) * opacity
directly; change the return to the same shadow-composited color used on the
normal path so the cursor shadow remains when not blurring. Replace the direct
return with computing the sampled color = textureSample(t_cursor, s_cursor,
input.uv) * opacity (or whatever local name is used) and then return
mix(shadow_color, sampled_color, shadow_alpha) (or the existing shadow blend
expression) so the branch yields an identical shadow-blended result as the
non-early-exit path.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/rendering/src/shaders/cursor.wgsl (1)
68-71: Avoid redundant texture sample in early exit.Line 70 resamples the texture, but
base_samplewas already computed at line 59. Reusing it would save a texture fetch.Apply this diff:
let motion_len = length(motion_vec); if (motion_len < 1e-4 || blur_strength < 0.001) { - return textureSample(t_cursor, s_cursor, input.uv) * opacity; + return base_sample * opacity; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/routes/editor/context.ts(2 hunks)crates/project/src/configuration.rs(7 hunks)crates/rendering/src/shaders/composite-video-frame.wgsl(7 hunks)crates/rendering/src/shaders/cursor.wgsl(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/rendering/src/shaders/cursor.wgslcrates/project/src/configuration.rscrates/rendering/src/shaders/composite-video-frame.wgsl
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/editor/context.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/context.ts
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/project/src/configuration.rs
🧬 Code graph analysis (2)
apps/desktop/src/routes/editor/context.ts (1)
apps/desktop/src/utils/tauri.ts (2)
ProjectConfiguration(447-447)SerializedEditorInstance(470-470)
crates/project/src/configuration.rs (2)
apps/desktop/src/utils/tauri.ts (3)
CornerStyle(384-384)CursorAnimationStyle(389-389)CursorType(392-392)crates/rendering/src/composite_frame.rs (1)
default(40-66)
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/editor/context.ts
[failure] 111-111:
Object literal may only specify known properties, but 'rounding_type' does not exist in type 'BackgroundConfiguration'. Did you mean to write 'roundingType'?
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (11)
crates/rendering/src/shaders/cursor.wgsl (5)
59-61: LGTM! Proper weighted accumulation initialization.Starting with the base sample and weight 1.0 correctly includes the center tap in the weighted average.
64-66: LGTM! Clear uniform unpacking.Extracting motion parameters from the packed vec4 is straightforward and well-named.
73-74: LGTM! Direction normalization is correct.Dividing by
motion_lenis safe here since the early exit already ensuresmotion_len >= 1e-4.
76-89: LGTM! Weighted blur sampling is correctly implemented.The loop properly starts at i=1 since the base sample is already included, and the weighted accumulation with smoothstep easing produces a smooth blur trail.
91-93: LGTM! Proper weighted normalization and opacity.Dividing by
weight_sumcorrectly normalizes the weighted accumulation, and applying opacity at the end is appropriate.crates/rendering/src/shaders/composite-video-frame.wgsl (6)
48-61: LGTM! Superellipse math is correct.The superellipse formula correctly computes
(|x|^p + |y|^p)^(1/p)with power=4 for squircle rounding, and the conditional inrounded_corner_normproperly switches between circular and squircle styles.
63-68: LGTM! SDF correctly integrates custom rounding norm.Replacing
length(outside)withrounded_corner_norm(outside, rounding_type)properly extends the SDF to support both circular and squircle corner styles.
143-149: LGTM! Clean blur parameter extraction and early exit.The early exit correctly preserves shadow blending when blur is disabled or negligible.
154-180: LGTM! Directional blur with jitter is well-implemented.The mode correctly samples along the motion vector with smoothstep easing, adaptive sample counts, and jittering to reduce banding. The bounds checking prevents invalid samples.
181-207: LGTM! Radial zoom blur is correctly implemented.The mode properly creates a zoom blur effect by sampling radially toward the zoom center with jittering, adaptive sample counts, and bounds checking.
272-280: LGTM! Rounded corners now respect rounding_type.The corner clipping correctly uses
rounded_corner_normto support both circular and squircle corner styles.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/rendering/src/layers/cursor.rs (2)
19-24: Consider adding doc comments to motion constants.The new motion-related constants would benefit from brief doc comments explaining their purpose and units (e.g.,
CURSOR_VECTOR_CAPis in pixels,CURSOR_MIN_MOTION_NORMALIZEDis a ratio). This improves maintainability for future developers tuning the motion system.
214-253: Motion computation logic is correct and well-guarded.The motion computation properly handles edge cases:
- FPS and screen diagonal guarded against division by zero
- Cursor ID filtering prevents motion blending between different cursor types
- Strength gating avoids unnecessary computation
Consider extracting this block into a helper method like
compute_cursor_motion_vector()to improve modularity and testability, though the current inline approach is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/rendering/src/layers/cursor.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/rendering/src/layers/cursor.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/rendering/src/layers/cursor.rs
🧬 Code graph analysis (1)
crates/rendering/src/layers/cursor.rs (1)
crates/rendering/src/composite_frame.rs (1)
new(93-105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
crates/rendering/src/layers/cursor.rs (2)
460-467: LGTM: Standard vector clamping with proper guards.The implementation correctly clamps the motion vector magnitude to
CURSOR_VECTOR_CAPwhile protecting against division by zero. The logic is sound.
413-418: Field rename accurately reflects the new motion system.The rename from
velocity_blur_opacitytomotion_vector_strengthwith the new structure[x, y, strength, opacity]clearly communicates the motion vector (x, y), strength multiplier, and opacity. The uniform assignment (lines 413-418) correctly maps the computed motion values.Also applies to: 475-475
| fn combine_cursor_motion(parent: XY<f32>, child: XY<f32>) -> XY<f32> { | ||
| fn combine_axis(parent: f32, child: f32) -> f32 { | ||
| if parent.abs() > CURSOR_MIN_MOTION_PX | ||
| && child.abs() > CURSOR_MIN_MOTION_PX | ||
| && parent.signum() != child.signum() | ||
| { | ||
| 0.0 | ||
| } else { | ||
| parent + child | ||
| } | ||
| } | ||
|
|
||
| XY::new( | ||
| combine_axis(parent.x, child.x), | ||
| combine_axis(parent.y, child.y), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Document the directional conflict logic.
The function zeros out motion when parent and child movement oppose each other (line 448), which is clever behavior to avoid conflicting motion blur artifacts. However, this non-obvious logic deserves a comment explaining the rationale.
Apply this diff to add clarifying documentation:
+/// Combines parent (display) and child (cursor) motion vectors.
+/// Returns zero for an axis when parent and child move in opposite directions
+/// to avoid conflicting motion blur artifacts.
fn combine_cursor_motion(parent: XY<f32>, child: XY<f32>) -> XY<f32> {
fn combine_axis(parent: f32, child: f32) -> f32 {
+ // Zero out motion when movements conflict (opposite directions)
if parent.abs() > CURSOR_MIN_MOTION_PX
&& child.abs() > CURSOR_MIN_MOTION_PX
&& parent.signum() != child.signum()
{
0.0
} else {
parent + child
}
}📝 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.
| fn combine_cursor_motion(parent: XY<f32>, child: XY<f32>) -> XY<f32> { | |
| fn combine_axis(parent: f32, child: f32) -> f32 { | |
| if parent.abs() > CURSOR_MIN_MOTION_PX | |
| && child.abs() > CURSOR_MIN_MOTION_PX | |
| && parent.signum() != child.signum() | |
| { | |
| 0.0 | |
| } else { | |
| parent + child | |
| } | |
| } | |
| XY::new( | |
| combine_axis(parent.x, child.x), | |
| combine_axis(parent.y, child.y), | |
| ) | |
| } | |
| /// Combines parent (display) and child (cursor) motion vectors. | |
| /// Returns zero for an axis when parent and child move in opposite directions | |
| /// to avoid conflicting motion blur artifacts. | |
| fn combine_cursor_motion(parent: XY<f32>, child: XY<f32>) -> XY<f32> { | |
| fn combine_axis(parent: f32, child: f32) -> f32 { | |
| // Zero out motion when movements conflict (opposite directions) | |
| if parent.abs() > CURSOR_MIN_MOTION_PX | |
| && child.abs() > CURSOR_MIN_MOTION_PX | |
| && parent.signum() != child.signum() | |
| { | |
| 0.0 | |
| } else { | |
| parent + child | |
| } | |
| } | |
| XY::new( | |
| combine_axis(parent.x, child.x), | |
| combine_axis(parent.y, child.y), | |
| ) | |
| } |
🤖 Prompt for AI Agents
In crates/rendering/src/layers/cursor.rs around lines 442 to 458, the
combine_cursor_motion function contains non-obvious logic that zeros out axis
motion when parent and child movements exceed CURSOR_MIN_MOTION_PX and have
opposite signs; add a concise comment above the combine_axis function explaining
that this intentionally cancels opposing axis motion to avoid conflicting
motion-blur artifacts (i.e., when both inputs represent significant movement in
opposite directions), mention the CURSOR_MIN_MOTION_PX threshold and the sign
check, and note that otherwise motions are summed — place the comment directly
above combine_axis so future readers understand the rationale.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/project/src/configuration.rs (1)
718-724: Remove these unused constants.These constants are not referenced anywhere in the codebase. They belonged to the old velocity-based cursor smoothing approach and should be removed now that the PR replaces it with spring-mass profiles.
Remove lines 718-724 from
crates/project/src/configuration.rs:
SLOW_SMOOTHING_SAMPLESREGULAR_SMOOTHING_SAMPLESFAST_SMOOTHING_SAMPLESSLOW_VELOCITY_THRESHOLDREGULAR_VELOCITY_THRESHOLDFAST_VELOCITY_THRESHOLD
🧹 Nitpick comments (1)
crates/project/src/configuration.rs (1)
463-488: Approve with optional refactor suggestion.The implementation correctly applies presets to override default values. Note that the initial defaults (tension: 65.0, mass: 1.8, friction: 16.0) match the
Slowpreset exactly, making the override redundant for the default case. However, this pattern enhances maintainability and provides clear fallback values.If you prefer a more concise approach, consider:
impl Default for CursorConfiguration { fn default() -> Self { - let animation_style = CursorAnimationStyle::default(); - let mut config = Self { + let animation_style = CursorAnimationStyle::default(); + let preset = animation_style.preset().unwrap_or(CursorSmoothingPreset { + tension: 65.0, + mass: 1.8, + friction: 16.0, + }); + + Self { hide: false, hide_when_idle: false, hide_when_idle_delay: Self::default_hide_when_idle_delay(), size: 100, r#type: CursorType::default(), animation_style, - tension: 65.0, - mass: 1.8, - friction: 16.0, + tension: preset.tension, + mass: preset.mass, + friction: preset.friction, raw: false, motion_blur: 0.5, use_svg: true, - }; - - if let Some(preset) = animation_style.preset() { - config.tension = preset.tension; - config.mass = preset.mass; - config.friction = preset.friction; } - - config } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/project/src/configuration.rs(8 hunks)crates/rendering/src/shaders/composite-video-frame.wgsl(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/rendering/src/shaders/composite-video-frame.wgsl
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/project/src/configuration.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/project/src/configuration.rs
🧬 Code graph analysis (1)
crates/project/src/configuration.rs (2)
apps/desktop/src/utils/tauri.ts (3)
CornerStyle(384-384)CursorAnimationStyle(389-389)CursorType(392-392)crates/rendering/src/composite_frame.rs (1)
default(40-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
crates/project/src/configuration.rs (7)
183-189: LGTM! Clean enum definition.The
CornerStyleenum is well-defined with appropriate derives and matches the TypeScript interface. The serde aliases and default implementation are correct.
227-228: LGTM! Proper backward-compatible field addition.The
rounding_typefield is correctly annotated with#[serde(default)]to ensure backward compatibility with existing serialized configurations.
306-307: LGTM! Proper backward-compatible field addition.The
rounding_typefield is correctly added with#[serde(default)]for backward compatibility.Also applies to: 344-344
401-409: LGTM! Excellent backward compatibility approach.The serde aliases ensure that existing configurations using "regular", "quick", "rapid", or "fast" will seamlessly map to the new
Mellowvariant, preventing breaking changes.
411-416: LGTM! Clean preset structure.The
CursorSmoothingPresetstruct is well-defined with appropriate derives for returning preset parameters.
418-434: LGTM! Clean preset mapping.The
preset()method provides sensible spring-mass-damper parameters for each animation style, withCustomcorrectly returningNoneto allow user-specified values.
324-324: Confirm whether the camera rounding increase from 30.0 to 100.0 was UX-tested and document the design decision.The change is confirmed to be intentional (commit:
3d90fcc9 Increase default camera rounding value), and it does affect all new Camera instances via the serde default. However, the codebase contains no tests verifying this rounding behavior, no design documentation, and no comments explaining the rationale. Please confirm this change was UX-tested and update the code with a comment explaining why this 3.3x increase was necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
2243-2307: Consider making the label prop required.The
labelprop is marked optional (label?: string), but it's always provided at both call sites (lines 1814, 2168). Making it required would better reflect actual usage and prevent accidentally omitting it.Apply this diff if you'd like to make it required:
function CornerStyleSelect(props: { - label?: string; + label: string; value: CornerRoundingType; onChange: (value: CornerRoundingType) => void; }) { return ( <div class="flex flex-col gap-1.5"> - <Show when={props.label}> - {(label) => ( - <span class="text-[0.65rem] uppercase tracking-wide text-gray-11"> - {label()} - </span> - )} - </Show> + <span class="text-[0.65rem] uppercase tracking-wide text-gray-11"> + {props.label} + </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/routes/editor/ConfigSidebar.tsx(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/editor/ConfigSidebar.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/ConfigSidebar.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (4)
apps/desktop/src/routes/editor/context.ts (1)
CornerRoundingType(64-64)apps/desktop/src/utils/tauri.ts (1)
CursorAnimationStyle(389-389)crates/project/src/configuration.rs (3)
preset(419-433)div(139-144)div(150-155)apps/desktop/src/routes/editor/ui.tsx (5)
Field(25-47)MenuItem(264-279)PopperContent(287-293)topSlideAnimateClasses(432-433)MenuItemList(295-309)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
605-605: Type assertions are unnecessary—cursor properties are fully defined in CursorConfiguration type.The
CursorConfigurationtype (defined inapps/desktop/src/utils/tauri.ts:390) includes all three properties:hideWhenIdleDelay?: number,useSvg?: boolean, andmotionBlur?: number. The type is complete. Theas anyassertions on string keys are a Solid.jscreateStoretyping workaround, not evidence of missing type definitions. No fixes required.Likely an incorrect or invalid review comment.
| <Field name="Motion Blur" icon={<IconLucideWind class="size-4" />}> | ||
| <Slider | ||
| value={[project.background.rounding]} | ||
| onChange={(v) => setProject("background", "rounding", v[0])} | ||
| value={[project.cursor.motionBlur ?? DEFAULT_CURSOR_MOTION_BLUR]} | ||
| onChange={(v) => setProject("cursor", "motionBlur" as any, v[0])} | ||
| minValue={0} | ||
| maxValue={100} | ||
| step={0.1} | ||
| formatTooltip="%" | ||
| maxValue={1} | ||
| step={0.01} | ||
| formatTooltip={(value) => `${Math.round(value * 100)}%`} | ||
| /> | ||
| </Field> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Motion Blur field placement and type definition.
Two concerns:
-
Type safety:
"motionBlur" as anyon line 1825 suggests the property isn't properly typed in the cursor configuration. This bypasses type checking. -
UI placement: The Motion Blur control is in the Background tab (
BackgroundConfig) but modifiesproject.cursor.motionBlur. Users looking for cursor-related settings would naturally check the Cursor tab, not Background. Consider moving this field to the Cursor tab (around line 708) or document why it belongs here.
Verify that the cursor type definitions include motionBlur to remove the type assertion:
🏁 Script executed:
#!/bin/bash
# Search for cursor type definitions and motionBlur property
rg -nP --type=ts 'interface.*Cursor|type.*Cursor.*=' -A 20 | rg -C 3 'motionBlur'Length of output: 742
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for other motionBlur references in ConfigSidebar.tsx
rg -n 'motionBlur' apps/desktop/src/routes/editor/ConfigSidebar.tsx
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Check setProject function signature and usage context
rg -nP --type=ts '\bsetProject\s*\(' apps/desktop/src/routes/editor/ConfigSidebar.tsx -B 2 -A 2 | head -50Length of output: 1587
---
🏁 Script executed:
```shell
#!/bin/bash
# Find setProject function definition and its type signature
rg -nP --type=ts 'const setProject|function setProject' apps/desktop/src/routes/editor/ConfigSidebar.tsx -A 5
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Alternative search for setProject in imports or context setup
rg -n 'setProject' apps/desktop/src/routes/editor/ConfigSidebar.tsx | head -5Length of output: 235
---
🏁 Script executed:
```shell
#!/bin/bash
# Find setProject import
rg -n 'import.*setProject|setProject.*from' apps/desktop/src/routes/editor/ConfigSidebar.tsx -B 2 -A 2
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check top 50 lines for imports
head -100 apps/desktop/src/routes/editor/ConfigSidebar.tsx | rg -n 'import|setProject'Length of output: 1718
---
🏁 Script executed:
```shell
#!/bin/bash
# Find where setProject is destructured/defined
sed -n '280,310p' apps/desktop/src/routes/editor/ConfigSidebar.tsx
Length of output: 621
🏁 Script executed:
#!/bin/bash
# Check useEditorContext source to understand setProject
rg -n 'setProject' apps/desktop/src/routes/editor/context.ts -B 3 -A 1 | head -30Length of output: 736
---
🏁 Script executed:
```shell
#!/bin/bash
# Find EditorProjectConfiguration type definition
rg -n 'type EditorProjectConfiguration|interface EditorProjectConfiguration' apps/desktop/src/routes/editor/context.ts -A 30
Length of output: 840
🏁 Script executed:
#!/bin/bash
# Alternative: search in all files for the type
rg -n 'type EditorProjectConfiguration|interface EditorProjectConfiguration' --type=ts -A 30 | head -80Length of output: 2142
---
🏁 Script executed:
```shell
#!/bin/bash
# Find ProjectConfiguration type
rg -n 'type ProjectConfiguration|interface ProjectConfiguration' --type=ts -A 20 | head -80
Length of output: 4365
Remove unnecessary type assertion on motionBlur property.
Verification found that motionBlur is properly defined in CursorConfiguration as an optional number. The "as any" assertion on line 1825 is unnecessary and inconsistent—other cursor properties like tension, mass, and friction don't require it.
Changes needed:
- Remove
as anyfrom line 1825; change"motionBlur" as anyto"motionBlur" - Reconsider placement: Motion Blur controls
project.cursor.motionBlurbut lives in the Background tab. Users naturally look for cursor settings in the Cursor tab. Either move this field to the Cursor tab (around line 708) or document why it belongs in Background.
🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/ConfigSidebar.tsx around lines 1822 to 1831,
remove the unnecessary type assertion on the motionBlur property by changing the
setProject call to use "motionBlur" (no "as any"); then either move this Field
out of the Background tab and place it with the other cursor controls near the
Cursor tab (around line ~708) so users find cursor settings together, or leave
it in Background but add a brief comment/documentation in the file explaining
why motionBlur is scoped there instead of with the Cursor settings.
Summary by CodeRabbit
New Features
Bug Fixes
Chores