feat: Split View scene + timeline trim indicator #985
Conversation
WalkthroughIntroduces “Split View” scene mode across UI, types, timeline logic, and rendering. Extracts SceneSegmentConfig into its own component with split-view controls and duplication/copy actions. Adds context actions, start-resize preview in ClipTrack, new rendering uniforms/layers (including ShadowLayer), shader rounding mask, and Rust config/type updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as SceneSegmentConfig (UI)
participant Ctx as EditorContext (projectActions)
participant State as Project State
User->>UI: Open segment config
UI->>State: Read segment + splitViewSettings
User->>UI: Change Mode → Split View
UI->>State: setProject: segment.mode="splitView"
User->>UI: Adjust Split View settings (side/pos/zoom/fullscreen)
UI->>State: setProject: update splitViewSettings
User->>UI: Copy settings from original
UI->>Ctx: copySceneSettingsFromOriginal(idx)
Ctx->>State: Apply settings if source found
sequenceDiagram
autonumber
participant TL as Timeline (ClipTrack)
participant UI as ClipTrack UI
participant Ctx as EditorContext
participant State as Project State
TL->>UI: Mousedown on start handle
UI->>UI: Compute constrained previewStart
UI->>UI: Show preview overlay
UI-->>UI: Mouseup
UI->>Ctx: Commit start = previewStart
Ctx->>State: setProject: update segment.start
UI->>UI: Clear preview overlay
sequenceDiagram
autonumber
participant R as Renderer
participant S as InterpolatedScene
participant L as Layers (Display/Camera/Shadow)
participant U as Uniforms
R->>S: Query mode/transition (SplitView?)
S-->>R: is_split_view / opacities
R->>U: Build split_view_* uniforms (positions, zooms, rounding_mask)
R->>L: prepare(split_view_display/camera/shadow)
alt Split View fullscreen
R->>L: Render split_view_* only
else Split View divided or transition
R->>L: Render background + regular with adjusted opacity
R->>L: Render split_view_shadow/display/camera
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/rendering/src/shaders/composite-video-frame.wgsl (1)
207-241: Per-corner rounding logic: fix SDF and minor correctness nits.
- Use
u32forcorner_bitto avoid repeated casts.- Replace
abs(length(...))with proper SDF form for a quarter circle:length(max(p, 0.0)) - r.- Keep anti-alias width configurable; current
1.0can look harsh at different scales.- let target_coord = target_uv * uniforms.target_size - uniforms.target_size / 2.0; - let abs_coord = abs(target_coord); + let target_coord = target_uv * uniforms.target_size - uniforms.target_size / 2.0; + let abs_coord = abs(target_coord); let rounding_point = uniforms.target_size / 2.0 - uniforms.rounding_px; - let target_rounding_coord = abs_coord - rounding_point; + let target_rounding_coord = abs_coord - rounding_point; - // Determine which corner we're in - let is_left = target_coord.x < 0.0; - let is_top = target_coord.y < 0.0; + // Determine which corner we're in + let is_left = target_coord.x < 0.0; + let is_top = target_coord.y < 0.0; - // Calculate corner mask bit (1=TL, 2=TR, 4=BL, 8=BR) - var corner_bit: f32 = 0.0; - if is_top && is_left { - corner_bit = 1.0; // Top-left - } else if is_top && !is_left { - corner_bit = 2.0; // Top-right - } else if !is_top && is_left { - corner_bit = 4.0; // Bottom-left - } else { - corner_bit = 8.0; // Bottom-right - } + // Calculate corner mask bit (1=TL, 2=TR, 4=BL, 8=BR) + var corner_bit: u32 = 0u; + if is_top && is_left { + corner_bit = 1u; // Top-left + } else if is_top && !is_left { + corner_bit = 2u; // Top-right + } else if !is_top && is_left { + corner_bit = 4u; // Bottom-left + } else { + corner_bit = 8u; // Bottom-right + } - // Check if this corner should be rounded - let should_round = (u32(uniforms.rounding_mask) & u32(corner_bit)) != 0u; + // Check if this corner should be rounded + let should_round = ((uniforms.rounding_mask) & corner_bit) != 0u; - if target_rounding_coord.x >= 0.0 && target_rounding_coord.y >= 0.0 && should_round { - let distance = abs(length(target_rounding_coord)) - uniforms.rounding_px; - let distance_blur = 1.0; - - if distance >= -distance_blur/2.0 { - return vec4<f32>(0.0); - } - } + if target_rounding_coord.x >= 0.0 && target_rounding_coord.y >= 0.0 && should_round { + // Proper quarter-circle SDF around the corner + let d = length(max(target_rounding_coord, vec2<f32>(0.0))) - uniforms.rounding_px; + let aa = 1.0; // TODO: consider scaling by fwidth + if d >= -aa * 0.5 { + return vec4<f32>(0.0); + } + } return current_color;Additionally, the shadow SDF currently ignores
rounding_mask, causing visual mismatches between content and shadowed silhouette. Consider updating the shadow distance computation accordingly (outside this hunk):// Replace shadow_dist computation near Lines 81-86 let local = p - center; let is_left = local.x < 0.0; let is_top = local.y < 0.0; var corner_bit: u32 = 0u; if is_top && is_left { corner_bit = 1u; } else if is_top && !is_left { corner_bit = 2u; } else if !is_top && is_left { corner_bit = 4u; } else { corner_bit = 8u; } let corner_rounded = ((uniforms.rounding_mask) & corner_bit) != 0u; let corner_r = select(0.0, uniforms.rounding_px, corner_rounded); let shadow_dist = sdf_rounded_rect(local, size, corner_r);apps/desktop/src/routes/editor/Timeline/SceneTrack.tsx (2)
90-96: Use currentTarget for bounds; target can be a child and break mathe.target may be a nested element, producing wrong time math and hover placement. Use currentTarget.
Apply this diff:
- const bounds = e.target.getBoundingClientRect()!; + const bounds = (e.currentTarget as HTMLElement).getBoundingClientRect()!;
170-184: Incorrect insertion index when adding a new scene segmentThe reverse scan inserts before the last segment with start > time, not the first. This can misplace the new segment. Use findIndex or a forward scan.
Apply one of these:
- let index = sceneSegments.length; - for (let i = sceneSegments.length - 1; i >= 0; i--) { - if (sceneSegments[i].start > time) { - index = i; - break; - } - } + const index = (() => { + const idx = sceneSegments.findIndex((s) => s.start > time); + return idx === -1 ? sceneSegments.length : idx; + })();Or:
- let index = sceneSegments.length; - for (let i = sceneSegments.length - 1; i >= 0; i--) { - if (sceneSegments[i].start > time) { - index = i; - break; - } - } + let index = sceneSegments.length; + for (let i = 0; i < sceneSegments.length; i++) { + if (sceneSegments[i].start > time) { + index = i; + break; + } + }apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (1)
418-442: Start-handle max duration math clamps too aggressivelymaxDuration currently ignores the clip’s current duration; start gets clamped near end when the timeline has limited free space. Use currentDuration + availableTimelineDuration.
- const maxDuration = Math.min( - maxSegmentDuration, - availableTimelineDuration, - ); + const currentDuration = segment.end - segment.start; + const maxAllowedDuration = Math.min( + maxSegmentDuration, + currentDuration + availableTimelineDuration, + ); @@ - Math.max( - newStart, - prevSegmentIsSameClip ? prevSegment.end : 0, - segment.end - maxDuration, - ), + Math.max( + newStart, + prevSegmentIsSameClip ? prevSegment.end : 0, + segment.end - maxAllowedDuration, + ),
🧹 Nitpick comments (13)
crates/rendering/src/composite_frame.rs (1)
29-31: Document bit layout and consider u32 for rounding_mask (avoids float→int casts in WGSL).
- Add a brief doc on the 4-bit layout (1=TL,2=TR,4=BL,8=BR).
- Optional: Switch
rounding_masktou32on both Rust and WGSL sides to remove casts and make intent explicit. Size/alignment remain unchanged.#[repr(C)] pub struct CompositeVideoFrameUniforms { @@ - pub opacity: f32, - pub rounding_mask: f32, + pub opacity: f32, + /// Bitmask: 1=TL, 2=TR, 4=BL, 8=BR + pub rounding_mask: u32, pub _padding: [f32; 2], } @@ - opacity: 1.0, - rounding_mask: 15.0, + opacity: 1.0, + rounding_mask: 0b1111, _padding: Default::default(),Additionally (outside this hunk), consider setting
min_binding_sizeto catch layout drift:// inside bind_group_layout(...) use std::num::NonZeroU64; ty: wgpu::BindingType::Buffer { ty: wgpu::BufferBindingType::Uniform, has_dynamic_offset: false, min_binding_size: NonZeroU64::new(std::mem::size_of::<CompositeVideoFrameUniforms>() as u64), },Also applies to: 51-51
crates/rendering/src/shaders/composite-video-frame.wgsl (1)
17-17: Prefer integer type for bitmask.Define
rounding_maskasu32to avoid runtime casts and clarify intent.- rounding_mask: f32, // Bitmask: 1=TL, 2=TR, 4=BL, 8=BR + rounding_mask: u32, // Bitmask: 1=TL, 2=TR, 4=BL, 8=BRcrates/rendering/src/layers/shadow.rs (1)
10-11: Simplify: bind_group is always Some; remove OptionYou initialize a bind group in new() and never set it to None. Storing Option adds branching for no gain.
- bind_group: Option<wgpu::BindGroup>, + bind_group: wgpu::BindGroup, ... - let bind_group = Some(pipeline.bind_group(device, &uniforms_buffer, &frame_texture_view)); + let bind_group = pipeline.bind_group(device, &uniforms_buffer, &frame_texture_view); ... - bind_group, + bind_group,And in render:
- if !self.hidden { - if let Some(bind_group) = &self.bind_group { - pass.set_pipeline(&self.pipeline.render_pipeline); - pass.set_bind_group(0, bind_group, &[]); - pass.draw(0..4, 0..1); - } - } + if !self.hidden { + pass.set_pipeline(&self.pipeline.render_pipeline); + pass.set_bind_group(0, &self.bind_group, &[]); + pass.draw(0..4, 0..1); + }Also applies to: 29-37
crates/rendering/src/scene.rs (1)
322-359: Minor: magic numbers and duplication in opacity logicThe repeated 0.7/0.3 fades are readable but brittle. Consider named constants and a helper for the fade math to reduce branching duplication.
crates/rendering/src/lib.rs (4)
1122-1122: Clippy: redundant field nameUse field init shorthand.
- split_view_shadow: split_view_shadow, + split_view_shadow,
1329-1337: More robust fullscreen detection for SplitView background skippingChecking only the top-left corner can misclassify. Compare target_bounds against output_size on both axes with a small epsilon.
- let split_view_fullscreen = uniforms - .split_view_display - .map(|u| u.target_bounds[0] < 1.0 && u.target_bounds[1] < 1.0) - .unwrap_or(false); + let split_view_fullscreen = uniforms + .split_view_display + .map(|u| { + let (w, h) = (u.output_size[0], u.output_size[1]); + let eps = 1.0; + u.target_bounds[0].abs() < eps + && u.target_bounds[1].abs() < eps + && (u.target_bounds[2] - u.target_bounds[0] - w).abs() < eps + && (u.target_bounds[3] - u.target_bounds[1] - h).abs() < eps + }) + .unwrap_or(false);
1274-1299: Avoid preparing SplitView layers every frameSmall perf nit: prepare split-view layers only when SplitView is active or transitioning.
- self.split_view_display.prepare( - &constants.device, - &constants.queue, - segment_frames, - constants.options.screen_size, - uniforms.split_view_display.unwrap_or_default(), - ); - - self.split_view_camera.prepare( - &constants.device, - &constants.queue, - (|| { - Some(( - uniforms.split_view_camera?, - constants.options.camera_size?, - segment_frames.camera_frame.as_ref()?, - )) - })(), - ); - - self.split_view_shadow.prepare( - &constants.device, - &constants.queue, - uniforms.split_view_shadow, - ); + if uniforms.scene.is_split_view() || uniforms.scene.is_transitioning_split_view() { + self.split_view_display.prepare( + &constants.device, + &constants.queue, + segment_frames, + constants.options.screen_size, + uniforms.split_view_display.unwrap_or_default(), + ); + self.split_view_camera.prepare( + &constants.device, + &constants.queue, + (|| { + Some(( + uniforms.split_view_camera?, + constants.options.camera_size?, + segment_frames.camera_frame.as_ref()?, + )) + })(), + ); + self.split_view_shadow.prepare( + &constants.device, + &constants.queue, + uniforms.split_view_shadow, + ); + }
958-966: Document rounding_mask constants (5.0/10.0 magic values)These imply per-corner masks (likely bitfields). Consider shared named constants to encode which corners are rounded for left/right halves.
Also applies to: 1051-1057
apps/desktop/src/routes/editor/Timeline/SceneTrack.tsx (1)
56-80: Type the scene mode to the generated union for safetyUse SceneMode (or SceneSegment["mode"]) instead of string to catch typos at compile time.
Apply this diff:
- const getSceneIcon = (mode: string | undefined) => { + const getSceneIcon = (mode: import("~/utils/tauri").SceneSegment["mode"]) => {- const getSceneLabel = (mode: string | undefined) => { + const getSceneLabel = (mode: import("~/utils/tauri").SceneSegment["mode"]) => {apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
49-49: Remove unused type importSplitViewSettings isn’t referenced in this file.
- type SplitViewSettings,apps/desktop/src/routes/editor/context.ts (1)
410-422: Tighten typing for meta; remove any-castsInitialize meta with a precise union and drop unknown/any casts to align with strict TS.
- let meta = null; + let meta: + | (MultipleSegments & { type: "multiple" }) + | (SingleSegment & { type: "single" }); @@ - meta = { + meta = { ...rawMeta, type: "multiple", - } as unknown as MultipleSegments & { type: "multiple" }; + } as MultipleSegments & { type: "multiple" }; @@ - meta = { + meta = { ...rawMeta, type: "single", - } as unknown as SingleSegment & { type: "single" }; + } as SingleSegment & { type: "single" };apps/desktop/src/routes/editor/SceneSegmentConfig.tsx (2)
5-17: Follow desktop guideline: rely on unplugin-icons auto-importsRemove manual icon imports; the desktop app auto-imports icons. Keeps bundles lean and consistent.
-import IconCapTrash from "~icons/iconoir/trash"; -import IconLucideAlignLeft from "~icons/lucide/align-left"; -import IconLucideAlignRight from "~icons/lucide/align-right"; -import IconLucideCheck from "~icons/lucide/check"; -import IconLucideClipboardCopy from "~icons/lucide/clipboard-copy"; -import IconLucideCopy from "~icons/lucide/copy"; -import IconLucideEyeOff from "~icons/lucide/eye-off"; -import IconLucideLayout from "~icons/lucide/layout"; -import IconLucideMaximize from "~icons/lucide/maximize"; -import IconLucideMinimize from "~icons/lucide/minimize"; -import IconLucideMonitor from "~icons/lucide/monitor"; -import IconLucideSettings from "~icons/lucide/settings"; -import IconLucideVideo from "~icons/lucide/video";
129-147: Overlap check: use newSegmentStart for claritySame logic, clearer intent.
- const newSegmentEnd = props.segment.end + segmentDuration; + const newSegmentStart = props.segment.end; + const newSegmentEnd = newSegmentStart + segmentDuration; @@ - const wouldOverlap = project.timeline?.sceneSegments?.some((s, i) => { - if (i === props.segmentIndex) return false; - return props.segment.end < s.end && newSegmentEnd > s.start; - }); + const wouldOverlap = project.timeline?.sceneSegments?.some((s, i) => { + if (i === props.segmentIndex) return false; + return newSegmentStart < s.end && newSegmentEnd > s.start; + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
apps/desktop/src/routes/editor/ConfigSidebar.tsx(3 hunks)apps/desktop/src/routes/editor/SceneSegmentConfig.tsx(1 hunks)apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx(4 hunks)apps/desktop/src/routes/editor/Timeline/SceneTrack.tsx(2 hunks)apps/desktop/src/routes/editor/context.ts(2 hunks)apps/desktop/src/utils/tauri.ts(1 hunks)crates/project/src/configuration.rs(2 hunks)crates/rendering/src/composite_frame.rs(2 hunks)crates/rendering/src/layers/mod.rs(1 hunks)crates/rendering/src/layers/shadow.rs(1 hunks)crates/rendering/src/lib.rs(12 hunks)crates/rendering/src/scene.rs(6 hunks)crates/rendering/src/shaders/composite-video-frame.wgsl(2 hunks)packages/ui-solid/src/auto-imports.d.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/editor/Timeline/SceneTrack.tsxapps/desktop/src/routes/editor/SceneSegmentConfig.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/context.tsapps/desktop/src/routes/editor/ConfigSidebar.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/editor/Timeline/SceneTrack.tsxapps/desktop/src/routes/editor/SceneSegmentConfig.tsxapps/desktop/src/utils/tauri.tspackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/context.tsapps/desktop/src/routes/editor/ConfigSidebar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/desktop/src/routes/editor/Timeline/SceneTrack.tsxapps/desktop/src/routes/editor/SceneSegmentConfig.tsxapps/desktop/src/utils/tauri.tspackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/editor/Timeline/ClipTrack.tsxapps/desktop/src/routes/editor/context.tsapps/desktop/src/routes/editor/ConfigSidebar.tsx
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file: tauri.ts
Files:
apps/desktop/src/utils/tauri.ts
🧠 Learnings (1)
📚 Learning: 2025-08-25T10:58:06.142Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T10:58:06.142Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Applied to files:
packages/ui-solid/src/auto-imports.d.ts
🧬 Code graph analysis (6)
apps/desktop/src/routes/editor/SceneSegmentConfig.tsx (3)
apps/desktop/src/utils/tauri.ts (2)
SceneSegment(429-429)SplitViewSettings(437-437)apps/desktop/src/routes/editor/ui.tsx (1)
EditorButton(353-418)apps/desktop/src/components/Toggle.tsx (1)
Toggle(37-50)
crates/rendering/src/layers/shadow.rs (2)
crates/rendering/src/lib.rs (9)
new(64-134)new(299-336)new(520-1128)new(1144-1149)new(1199-1212)new(1405-1435)prepare(1214-1301)render(1151-1181)render(1303-1395)crates/rendering/src/composite_frame.rs (1)
create_frame_texture(165-184)
crates/project/src/configuration.rs (1)
apps/desktop/src/utils/tauri.ts (3)
SplitViewSettings(437-437)XY(458-458)SplitViewSide(438-438)
crates/rendering/src/scene.rs (1)
apps/desktop/src/utils/tauri.ts (1)
SceneMode(428-428)
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (1)
apps/desktop/src/utils/tauri.ts (1)
TimelineSegment(444-444)
crates/rendering/src/lib.rs (3)
apps/desktop/src/utils/tauri.ts (3)
SplitViewSettings(437-437)SplitViewSide(438-438)XY(458-458)crates/rendering/src/composite_frame.rs (1)
default(34-54)crates/project/src/configuration.rs (8)
default(47-51)default(212-223)default(286-302)default(306-312)default(338-346)default(391-405)default(489-498)default(583-598)
🪛 GitHub Check: Typecheck
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
[failure] 481-481:
No overload matches this call.
🪛 GitHub Check: Clippy
crates/rendering/src/lib.rs
[warning] 1122-1122: redundant field names in struct initialization
warning: redundant field names in struct initialization
--> crates/rendering/src/lib.rs:1122:13
|
1122 | split_view_shadow: split_view_shadow,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: split_view_shadow
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
= note: #[warn(clippy::redundant_field_names)] on by default
⏰ 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: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (23)
crates/rendering/src/layers/mod.rs (1)
7-7: Re-export looks good.Module added and publicly re-exported cleanly. No conflicts expected if ShadowLayer items are
pub.Also applies to: 15-15
crates/rendering/src/layers/shadow.rs (1)
16-16: Verify API: CompositeVideoFramePipeline::create_frame_texture must existRelevant code shows a free function
create_frame_texture(...)in composite_frame.rs. Confirm there’s an associated fn on CompositeVideoFramePipeline; otherwise switch to the free function.Proposed fallback:
- let frame_texture = CompositeVideoFramePipeline::create_frame_texture(device, 1, 1); + let frame_texture = crate::composite_frame::create_frame_texture(device, 1, 1);crates/rendering/src/scene.rs (3)
100-101: LGTM: treating SplitView as a stable mode for small gapsAdding SplitView pairs to the “same mode” checks avoids unnecessary transitions. Looks correct and aligns with the other modes.
Also applies to: 129-130, 178-179
311-318: Screen rendering gate for SplitView: confirm cursor behaviorDisabling the regular screen outside transitions is fine, but note this also suppresses the regular cursor path in lib.rs (cursor is only drawn when the regular screen is drawn). If the cursor should be visible in SplitView, we’ll need a SplitView-aware cursor path.
I can propose a scoped plan if you want the cursor visible during SplitView.
366-373: LGTM: SplitView helpers and opacity curvesThe new helpers and transition opacities are coherent and line up with the rendering flow.
Also applies to: 375-391, 421-466
crates/rendering/src/lib.rs (1)
1360-1373: Cursor not rendered in SplitViewThe cursor is only drawn when the regular screen is rendered. In pure SplitView (non-transition), it disappears. If that’s not intentional, add a SplitView-aware cursor draw (and potentially a cursor uniform based on split_view_display bounds).
I can draft a small change to reuse CursorLayer with SplitView bounds if you’d like.
Also applies to: 1386-1394
apps/desktop/src/utils/tauri.ts (1)
428-439: Split View types exposed correctly; keep this file generated-onlyNew SceneMode/SceneSegment/SplitView types look consistent with Rust (camelCase). No changes requested; ensure these come from codegen and not manual edits.
crates/project/src/configuration.rs (3)
460-469: SceneMode::SplitView variant addition looks correctVariant naming + serde camelCase will map to "splitView" on the TS side.
470-506: SplitViewSettings + defaults are sane and TS-alignedFields, defaults (1.0 zoom, centered positions), and SplitViewSide mapping are coherent with the UI.
515-517: Optional split_view_settings with skip-serialize is backward compatibleOption + skip_serializing_if handles old configs cleanly.
packages/ui-solid/src/auto-imports.d.ts (1)
64-65: Icons auto-imports updatedAlignLeft/AlignRight declarations added; fine for the new UI.
apps/desktop/src/routes/editor/Timeline/SceneTrack.tsx (2)
62-64: Icon for Split View is wiredgetSceneIcon correctly returns IconLucideLayout.
75-77: Label for Split View is correct“Split View” label matches new mode.
apps/desktop/src/routes/editor/ConfigSidebar.tsx (2)
63-63: SceneSegmentConfig extraction looks goodImporting the new component keeps this file slimmer.
2355-2356: Helpful noteComment clarifies ownership of the position control.
apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx (5)
11-12: LGTM: signal import is necessary for preview state
170-174: LGTM: ephemeral start-resize preview state
181-186: LGTM: safer origin for updateZoomUsing previewTime ?? playbackTime avoids null assertions.
475-499: LGTM: trim preview overlayClear, bounded overlay; minimal reactivity surface.
451-472: Ensure event handlers match TypeScript overloads
createEventListenerMapinfers all callbacks as(e: Event), so passing yourupdate(e: MouseEvent)directly can trigger “No overload matches this call.” Either:
- Narrow the event with a cast:
createEventListenerMap(window, { mousemove: ev => update(ev as MouseEvent), mouseup: e => { … } });- Split into two typed listeners for proper inference:
createEventListener(window, "mousemove", update); createEventListener(window, "mouseup", e => { … });Verify the TS compiler error is resolved.
apps/desktop/src/routes/editor/SceneSegmentConfig.tsx (3)
219-252: LGTM: Split View panel gating and fullscreen toggle wiring
260-291: LGTM: Camera side radio group updates state correctly
293-391: LGTM: Position/zoom controls write back via setProject (immutable updates)
| duplicateSceneSegment: (segmentIndex: number) => { | ||
| if (!project.timeline?.sceneSegments?.[segmentIndex]) return; | ||
| const segment = project.timeline.sceneSegments[segmentIndex]; | ||
| const segmentDuration = segment.end - segment.start; | ||
| const newSegmentStart = segment.end; | ||
| const newSegmentEnd = newSegmentStart + segmentDuration; | ||
|
|
||
| const timelineDuration = totalDuration(); | ||
| if (newSegmentEnd > timelineDuration) { | ||
| return; | ||
| } | ||
|
|
||
| const wouldOverlap = project.timeline.sceneSegments.some((s, i) => { | ||
| if (i === segmentIndex) return false; // Skip the original segment | ||
| return newSegmentStart < s.end && newSegmentEnd > s.start; | ||
| }); | ||
|
|
||
| if (wouldOverlap) { | ||
| return; | ||
| } | ||
|
|
||
| batch(() => { | ||
| setProject( | ||
| "timeline", | ||
| "sceneSegments", | ||
| produce((s) => { | ||
| if (!s) return; | ||
| s.splice(segmentIndex + 1, 0, { | ||
| ...segment, | ||
| start: newSegmentStart, | ||
| end: newSegmentEnd, | ||
| splitViewSettings: segment.splitViewSettings | ||
| ? { ...segment.splitViewSettings } | ||
| : undefined, | ||
| }); | ||
| }), | ||
| ); | ||
| setEditorState("timeline", "selection", { | ||
| type: "scene", | ||
| index: segmentIndex + 1, | ||
| }); | ||
| setEditorState("playbackTime", newSegmentStart); | ||
| const currentZoom = editorState.timeline.transform.zoom; | ||
| const targetPosition = Math.max(0, newSegmentStart - currentZoom / 2); | ||
| editorState.timeline.transform.setPosition(targetPosition); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Deep-clone splitViewSettings when duplicating to avoid shared nested refs
Shallow-spreading splitViewSettings shares cameraPosition/screenPosition objects between segments, causing cross-segment coupling if any in-place mutation slips in elsewhere.
Apply:
- s.splice(segmentIndex + 1, 0, {
+ s.splice(segmentIndex + 1, 0, {
...segment,
start: newSegmentStart,
end: newSegmentEnd,
- splitViewSettings: segment.splitViewSettings
- ? { ...segment.splitViewSettings }
- : undefined,
+ splitViewSettings: segment.splitViewSettings
+ ? structuredClone(segment.splitViewSettings)
+ : undefined,
});📝 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.
| duplicateSceneSegment: (segmentIndex: number) => { | |
| if (!project.timeline?.sceneSegments?.[segmentIndex]) return; | |
| const segment = project.timeline.sceneSegments[segmentIndex]; | |
| const segmentDuration = segment.end - segment.start; | |
| const newSegmentStart = segment.end; | |
| const newSegmentEnd = newSegmentStart + segmentDuration; | |
| const timelineDuration = totalDuration(); | |
| if (newSegmentEnd > timelineDuration) { | |
| return; | |
| } | |
| const wouldOverlap = project.timeline.sceneSegments.some((s, i) => { | |
| if (i === segmentIndex) return false; // Skip the original segment | |
| return newSegmentStart < s.end && newSegmentEnd > s.start; | |
| }); | |
| if (wouldOverlap) { | |
| return; | |
| } | |
| batch(() => { | |
| setProject( | |
| "timeline", | |
| "sceneSegments", | |
| produce((s) => { | |
| if (!s) return; | |
| s.splice(segmentIndex + 1, 0, { | |
| ...segment, | |
| start: newSegmentStart, | |
| end: newSegmentEnd, | |
| splitViewSettings: segment.splitViewSettings | |
| ? { ...segment.splitViewSettings } | |
| : undefined, | |
| }); | |
| }), | |
| ); | |
| setEditorState("timeline", "selection", { | |
| type: "scene", | |
| index: segmentIndex + 1, | |
| }); | |
| setEditorState("playbackTime", newSegmentStart); | |
| const currentZoom = editorState.timeline.transform.zoom; | |
| const targetPosition = Math.max(0, newSegmentStart - currentZoom / 2); | |
| editorState.timeline.transform.setPosition(targetPosition); | |
| }); | |
| }, | |
| diff --git a/apps/desktop/src/routes/editor/context.ts b/apps/desktop/src/routes/editor/context.ts | |
| index e69de29..b7e23f8 100644 | |
| ++ b/apps/desktop/src/routes/editor/context.ts | |
| @@ -162,19 +162,19 @@ export const editorContext = createContext<EditorContextValue | undefined>(unde | |
| duplicateSceneSegment: (segmentIndex: number) => { | |
| if (!project.timeline?.sceneSegments?.[segmentIndex]) return; | |
| const segment = project.timeline.sceneSegments[segmentIndex]; | |
| const segmentDuration = segment.end - segment.start; | |
| const newSegmentStart = segment.end; | |
| const newSegmentEnd = newSegmentStart + segmentDuration; | |
| const timelineDuration = totalDuration(); | |
| if (newSegmentEnd > timelineDuration) { | |
| return; | |
| } | |
| const wouldOverlap = project.timeline.sceneSegments.some((s, i) => { | |
| if (i === segmentIndex) return false; // Skip the original segment | |
| return newSegmentStart < s.end && newSegmentEnd > s.start; | |
| }); | |
| if (wouldOverlap) { | |
| return; | |
| } | |
| batch(() => { | |
| setProject( | |
| "timeline", | |
| "sceneSegments", | |
| produce((s) => { | |
| if (!s) return; | |
| s.splice(segmentIndex + 1, 0, { | |
| ...segment, | |
| start: newSegmentStart, | |
| end: newSegmentEnd, | |
| - splitViewSettings: segment.splitViewSettings | |
| - ? { ...segment.splitViewSettings } | |
| splitViewSettings: segment.splitViewSettings | |
| ? structuredClone(segment.splitViewSettings) | |
| : undefined, | |
| }); | |
| }), | |
| ); | |
| setEditorState("timeline", "selection", { | |
| type: "scene", | |
| index: segmentIndex + 1, | |
| }); | |
| setEditorState("playbackTime", newSegmentStart); | |
| const currentZoom = editorState.timeline.transform.zoom; | |
| const targetPosition = Math.max(0, newSegmentStart - currentZoom / 2); | |
| editorState.timeline.transform.setPosition(targetPosition); | |
| }); | |
| }, |
🤖 Prompt for AI Agents
apps/desktop/src/routes/editor/context.ts around lines 162 to 208, the
duplication currently shallow-copies splitViewSettings which retains nested
object references (cameraPosition/screenPosition) and can cause cross-segment
coupling; fix by deep-cloning splitViewSettings when creating the new segment
(use structuredClone(segment.splitViewSettings) if available, otherwise use a
safe deep-clone utility such as lodash/cloneDeep or
JSON.parse(JSON.stringify(...))) and assign that cloned value to
splitViewSettings on the new segment so no nested object references are shared.
| copySceneSettingsFromOriginal: (segmentIndex: number) => { | ||
| if (!project.timeline?.sceneSegments?.[segmentIndex]) return; | ||
|
|
||
| const currentSegment = project.timeline.sceneSegments[segmentIndex]; | ||
| const originalSegment = project.timeline.sceneSegments.find( | ||
| (s, i) => i !== segmentIndex && s.mode === currentSegment.mode, | ||
| ); | ||
|
|
||
| if (!originalSegment) return; | ||
|
|
||
| setProject( | ||
| "timeline", | ||
| "sceneSegments", | ||
| segmentIndex, | ||
| produce((s) => { | ||
| if (!s) return; | ||
| if (s.mode === "splitView" && originalSegment.splitViewSettings) { | ||
| s.splitViewSettings = { ...originalSegment.splitViewSettings }; | ||
| } | ||
| }), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Also deep-clone when copying settings from the “original”
Mirror the duplication fix here to prevent shared nested references.
- if (s.mode === "splitView" && originalSegment.splitViewSettings) {
- s.splitViewSettings = { ...originalSegment.splitViewSettings };
- }
+ if (s.mode === "splitView" && originalSegment.splitViewSettings) {
+ s.splitViewSettings = structuredClone(originalSegment.splitViewSettings);
+ }📝 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.
| copySceneSettingsFromOriginal: (segmentIndex: number) => { | |
| if (!project.timeline?.sceneSegments?.[segmentIndex]) return; | |
| const currentSegment = project.timeline.sceneSegments[segmentIndex]; | |
| const originalSegment = project.timeline.sceneSegments.find( | |
| (s, i) => i !== segmentIndex && s.mode === currentSegment.mode, | |
| ); | |
| if (!originalSegment) return; | |
| setProject( | |
| "timeline", | |
| "sceneSegments", | |
| segmentIndex, | |
| produce((s) => { | |
| if (!s) return; | |
| if (s.mode === "splitView" && originalSegment.splitViewSettings) { | |
| s.splitViewSettings = { ...originalSegment.splitViewSettings }; | |
| } | |
| }), | |
| ); | |
| }, | |
| copySceneSettingsFromOriginal: (segmentIndex: number) => { | |
| if (!project.timeline?.sceneSegments?.[segmentIndex]) return; | |
| const currentSegment = project.timeline.sceneSegments[segmentIndex]; | |
| const originalSegment = project.timeline.sceneSegments.find( | |
| (s, i) => i !== segmentIndex && s.mode === currentSegment.mode, | |
| ); | |
| if (!originalSegment) return; | |
| setProject( | |
| "timeline", | |
| "sceneSegments", | |
| segmentIndex, | |
| produce((s) => { | |
| if (!s) return; | |
| if (s.mode === "splitView" && originalSegment.splitViewSettings) { | |
| s.splitViewSettings = structuredClone(originalSegment.splitViewSettings); | |
| } | |
| }), | |
| ); | |
| }, |
🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/context.ts around lines 209 to 230, when
copying settings from the original segment the code uses a shallow spread ({
...originalSegment.splitViewSettings }) which leaves nested objects shared;
replace the shallow copy with a deep-clone of originalSegment.splitViewSettings
(e.g., use structuredClone(originalSegment.splitViewSettings) or your project’s
cloneDeep utility) and assign that deep-clone to s.splitViewSettings, ensuring
you handle the case where splitViewSettings may be undefined.
| <Show | ||
| when={project.timeline?.sceneSegments?.some( | ||
| (s, i) => i !== props.segmentIndex && s.mode === props.segment.mode, | ||
| )} | ||
| > | ||
| <div class="pt-3 border-t border-gray-3"> | ||
| <EditorButton | ||
| onClick={() => { | ||
| projectActions.copySceneSettingsFromOriginal(props.segmentIndex); | ||
| }} | ||
| leftIcon={<IconLucideClipboardCopy />} | ||
| class="w-full" | ||
| > | ||
| Copy Settings from Original | ||
| </EditorButton> | ||
| </div> | ||
| </Show> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Copy settings should deep-clone to avoid shared references
Align with context.ts fixes.
- onClick={() => {
- projectActions.copySceneSettingsFromOriginal(props.segmentIndex);
- }}
+ onClick={() => {
+ projectActions.copySceneSettingsFromOriginal(props.segmentIndex);
+ }}And ensure the action itself uses structuredClone (see suggested patch in context.ts).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/SceneSegmentConfig.tsx around lines 396-412,
the "Copy Settings from Original" flow currently risks sharing object
references; update the action implementation
(projectActions.copySceneSettingsFromOriginal) to deep-clone the source segment
data using structuredClone before applying it to the target segment so no nested
objects are shared, and ensure any imports or polyfills for structuredClone are
added where that action is defined (align with the fixes in context.ts) — no
change needed to the button here besides calling the action with the same
segmentIndex.
| if self.frame_texture.width() != 1 || self.frame_texture.height() != 1 { | ||
| self.frame_texture = CompositeVideoFramePipeline::create_frame_texture(device, 1, 1); | ||
| self.frame_texture_view = self.frame_texture.create_view(&Default::default()); | ||
|
|
||
| self.bind_group = Some(self.pipeline.bind_group( | ||
| device, | ||
| &self.uniforms_buffer, | ||
| &self.frame_texture_view, | ||
| )); | ||
| } |
There was a problem hiding this comment.
Fix: wgpu::Texture has no width()/height(); this block won’t compile
wgpu’s Texture is opaque; querying size like this isn’t supported. Since this layer always uses a 1x1 placeholder, just drop the conditional (or store size separately if you intend to vary it later).
Apply:
- if self.frame_texture.width() != 1 || self.frame_texture.height() != 1 {
- self.frame_texture = CompositeVideoFramePipeline::create_frame_texture(device, 1, 1);
- self.frame_texture_view = self.frame_texture.create_view(&Default::default());
-
- self.bind_group = Some(self.pipeline.bind_group(
- device,
- &self.uniforms_buffer,
- &self.frame_texture_view,
- ));
- }
+ // Always 1x1 placeholder; no need to recreate per frame.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/rendering/src/layers/shadow.rs around lines 53 to 62, the code calls
self.frame_texture.width()/height() which is invalid because wgpu::Texture is
opaque; remove the conditional that checks texture size and instead always (or
at least when None) create the 1x1 placeholder texture and view and update
self.bind_group accordingly; if you anticipate variable sizes later, track the
texture dimensions in a separate field and check that instead before recreating.
| if !self.hidden | ||
| && let Some(bind_group) = &self.bind_group | ||
| { | ||
| pass.set_pipeline(&self.pipeline.render_pipeline); | ||
| pass.set_bind_group(0, bind_group, &[]); | ||
| pass.draw(0..4, 0..1); | ||
| } |
There was a problem hiding this comment.
Fix: invalid let-chain ordering in if condition
The pattern if !cond && let Some(...) = ... is not valid. Use nested ifs.
- pub fn render(&self, pass: &mut wgpu::RenderPass<'_>) {
- if !self.hidden
- && let Some(bind_group) = &self.bind_group
- {
- pass.set_pipeline(&self.pipeline.render_pipeline);
- pass.set_bind_group(0, bind_group, &[]);
- pass.draw(0..4, 0..1);
- }
- }
+ pub fn render(&self, pass: &mut wgpu::RenderPass<'_>) {
+ if !self.hidden {
+ if let Some(bind_group) = &self.bind_group {
+ pass.set_pipeline(&self.pipeline.render_pipeline);
+ pass.set_bind_group(0, bind_group, &[]);
+ pass.draw(0..4, 0..1);
+ }
+ }
+ }🤖 Prompt for AI Agents
In crates/rendering/src/layers/shadow.rs around lines 68–74, the if condition
uses an invalid let-chain ordering ("!self.hidden && let Some(...) = ...");
change to nested ifs so the let binding is evaluated inside the outer condition.
Concretely, first check visibility (if !self.hidden) and inside that block use
if let Some(bind_group) = &self.bind_group to unwrap the bind group, then run
the pipeline, set the bind group and draw.
| let zoomed_crop_size = [crop_size[0] / zoom, crop_size[1] / zoom]; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against divide-by-zero zoom values
User/config could set zoom=0.0. Clamp to a sane minimum to avoid NaNs/infs.
- let zoom = split_settings.screen_zoom as f32;
+ let zoom = (split_settings.screen_zoom as f32).max(0.01);
...
- let cam_zoom = split_settings.camera_zoom as f32;
+ let cam_zoom = (split_settings.camera_zoom as f32).max(0.01);Also applies to: 980-981
🤖 Prompt for AI Agents
In crates/rendering/src/lib.rs around lines 887-888 (and also apply same fix at
~980-981), the code divides by the variable zoom which may be zero; clamp zoom
to a sane minimum before any division to prevent NaN/inf results. Modify the
function(s) to replace direct use of zoom with a validated value (e.g., let
safe_zoom = zoom.max(MIN_ZOOM) where MIN_ZOOM is a small positive constant like
1e-6 or f32::EPSILON), then compute zoomed_crop_size and any other divisions
using safe_zoom; ensure MIN_ZOOM is defined near the top of the module or
function and document its purpose with a short comment.
| }) | ||
| }; | ||
|
|
||
| let split_camera = options.camera_size.map(|camera_size| { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Respect camera.hide for split-view camera
Hide camera should also suppress the split-view camera half.
- let split_camera = options.camera_size.map(|camera_size| {
+ let split_camera = options
+ .camera_size
+ .filter(|_| !project.camera.hide)
+ .map(|camera_size| {📝 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.
| let split_camera = options.camera_size.map(|camera_size| { | |
| let split_camera = options | |
| .camera_size | |
| .filter(|_| !project.camera.hide) | |
| .map(|camera_size| { |
🤖 Prompt for AI Agents
In crates/rendering/src/lib.rs around line 971, the split_camera created from
options.camera_size does not account for camera.hide; update the mapping so it
returns None when the active camera is hidden (e.g., check options.camera or
options.camera.as_ref().map_or(false, |c| c.hide) and short-circuit to None),
ensuring the split-view half is suppressed when hide is set; propagate the None
result as before so callers receive no split camera when hidden.
Introduces support for a new "Split View" mode for scene segments in the editor, enabling more flexible layouts for camera and screen recordings. Also implements a new trimming indicator for the screen timeline element.
Split View demo:
https://cap.link/8y9hwvyfcrpfpqf
Timeline trim indicator demo:
https://cap.link/tszstaqa5vc5n9q
Summary by CodeRabbit