Editor multi-track, gradient noise, permissions, and performance fixes#1669
Editor multi-track, gradient noise, permissions, and performance fixes#1669richiemcilroy merged 38 commits intomainfrom
Conversation
Paragon SummaryThis pull request review analyzed 45 files and found no issues. The review examined code changes, potential bugs, security vulnerabilities, performance issues, and code quality concerns using automated analysis tools. Paragon did not detect any problems in the current diff. Proceed with merge after your normal checks. This PR adds multi-track timeline support for mask and text segments, introduces configurable gradient noise textures via WGSL shaders, improves macOS camera/microphone permission handling, and optimizes camera preview performance, WebSocket cleanup, and rendering efficiency. Key changes:
Confidence score: 5/5
45 files reviewed, 0 comments |
![paragon-review[bot]](https://avatars.githubusercontent.com/in/1544258?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| style={{ "background-color": text() }} | ||
| class="size-[2rem] rounded-[0.5rem] cursor-pointer transition-[box-shadow]" | ||
| style={{ | ||
| "background-color": text(), |
There was a problem hiding this comment.
Using text() directly here means the swatch border can go invalid while a user is mid-edit (e.g. #12), since color-mix(...) will fail to parse. Since you already have normalizeHexInput, could use that for the preview so it stays stable.
| "background-color": text(), | |
| style={{ | |
| "background-color": normalizeHexInput(text(), prevColor), | |
| "box-shadow": `inset 0 0 0 1px ${getColorPreviewBorderColor(normalizeHexInput(text(), prevColor))}`, | |
| }} |
| class="size-[3rem] rounded-[0.5rem]" | ||
| class="size-[2rem] rounded-[0.5rem]" | ||
| style={{ | ||
| "background-color": text(), |
There was a problem hiding this comment.
Same idea here: while typing, text() can be an invalid color which makes color-mix(...) fail. Consider using a simple hex guard for the preview.
| "background-color": text(), | |
| style={{ | |
| "background-color": /^#[0-9A-F]{6}$/i.test(text()) ? text() : prevColor, | |
| "box-shadow": `inset 0 0 0 1px ${getColorPreviewBorderColor(/^#[0-9A-F]{6}$/i.test(text()) ? text() : prevColor)}`, | |
| }} |
apps/desktop/src-tauri/src/lib.rs
Outdated
| std::env::var("CAP_BUNDLE_ID").unwrap_or_else(|_| "com.apple.Terminal".to_string()); | ||
| #[cfg(not(debug_assertions))] | ||
| let bundle_id = "so.cap.desktop"; | ||
| let bundle_id = app.config().identifier.clone(); |
There was a problem hiding this comment.
Switching to app.config().identifier makes sense for release, but if local dev permissions end up attributed to a different bundle id (e.g. when running unbundled), an env override can keep this command useful.
| let bundle_id = app.config().identifier.clone(); | |
| let bundle_id = std::env::var("CAP_BUNDLE_ID").unwrap_or_else(|_| app.config().identifier.clone()); |
| let bundle_id = "so.cap.desktop"; | ||
| #[instrument(skip(app))] | ||
| async fn reset_microphone_permissions(app: AppHandle) -> Result<(), ()> { | ||
| let bundle_id = app.config().identifier.clone(); |
There was a problem hiding this comment.
Same override pattern here (if you want to support resetting a different id during dev/testing).
| let bundle_id = app.config().identifier.clone(); | |
| let bundle_id = std::env::var("CAP_BUNDLE_ID").unwrap_or_else(|_| app.config().identifier.clone()); |
| } else if (type === "microphone") { | ||
| await commands.resetMicrophonePermissions(); | ||
| const window = getCurrentWindow(); | ||
| await window.setAlwaysOnTop(false); |
There was a problem hiding this comment.
This always-on-top toggle seems intended to keep OS dialogs/settings visible; the currentStatus === "denied" early-return path above currently bypasses it. Might be worth wrapping that path in the same try/finally so Settings can’t open behind the always-on-top window.
| await window.setAlwaysOnTop(false); | |
| const window = getCurrentWindow(); | |
| await window.setAlwaysOnTop(false); | |
| try { | |
| if (currentStatus === "denied") { | |
| await commands.openPermissionSettings(type); | |
| return; | |
| } | |
| await commands.requestPermission(type); | |
| const check = await commands.doPermissionsCheck(false); | |
| const status = type === "camera" ? check.camera : check.microphone; | |
| if (status !== "granted") { | |
| await commands.openPermissionSettings(type); | |
| } | |
| } finally { | |
| await window.setAlwaysOnTop(true); | |
| } |
|
@greptileai please rereview |
| warn!("Screen capture audio stream stopped by the system; awaiting restart"); | ||
| continue; | ||
| warn!("Screen capture audio stream stopped by the system"); | ||
| return Err(anyhow!(system_stop_message())); |
There was a problem hiding this comment.
Worth considering a cancellation guard here: SystemAudioSource::stop() cancels cancel_token, but this task currently has no way to see that and can still return the system-stopped message during an intentional stop.
1 participant
Greptile Summary
This PR delivers a broad set of improvements across the editor, recording pipeline, and renderer. The four headline areas are: (1) gradient noise textures via a new WGSL
fbmshader and matchingGradientEditorcomponent, (2) multi-lane support for mask and text timeline tracks with add/delete UI, (3) making camera and microphone permissions optional during setup with a proper macOS activation-policy toggle for the permission dialog, and (4) a collection of performance and cleanup fixes (ImageData buffer reuse, channel pre-check to skip clones, improved WebSocket disposal, per-render mask uniform buffer).Key changes to note:
background.rs— colours inBackgroundSource::ColorandBackgroundSource::Gradientare now passed through as rawvalue / 255.0rather than being converted to linear space. This is a deliberate visual change covered by a new test (test_color_conversion_uses_normalized_byte_values), but will alter the appearance of existing projects that relied on the previous gamma-correct rendering.apps/src/utils/tauri.tsis a newly added file that appears to be a duplicate of the generated bindings atapps/desktop/src/utils/tauri.ts; the intent and ownership of this copy should be clarified to prevent the two from diverging.animated/animation_speedfields are added toBackgroundSource::Gradientin the config and exposed in the TypeScript bindings but are silently discarded with..in every match arm — this is forward-looking scaffolding but should be documented to avoid confusion.Confidence Score: 3/5
apps/src/utils/tauri.tsfile could lead to stale generated types.Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph Editor["Editor UI"] GE[GradientEditor.tsx\nnoise_intensity / noise_scale sliders] TM[TrackManager\nAdd / toggle multi-track] TI[Timeline/index.tsx\nFor each textTrackRows / maskTrackRows] MT[MaskTrack / TextTrack\nlaneIndex prop] end subgraph Config["Project Config (Rust)"] BS[BackgroundSource::Gradient\nnoise_intensity, noise_scale\nanimated ⚠️ unused] MS[MaskSegment / TextSegment\ntrack: u32] end subgraph Rendering["Rendering Pipeline (wgpu)"] BG[background.rs\nGradientOrColorUniforms\nnoise_intensity, noise_scale] WGSL[gradient-or-color.wgsl\nfbm noise → overlay blend] MASK[mask.rs\nper-render uniform buffer\nfade_duration=0 for Sensitive] end subgraph Recording["Recording (macOS)"] CAM[camera.rs\nis_full pre-check → skip clone] SC[screen_capture/macos.rs\nsystem-stop → Err instead of restart] PERM[permissions.rs\nactivation policy toggle] end GE -->|setProject background.source| BS TM -->|handleAddTrack / handleToggleTrack| TI TI -->|laneIndex| MT MT -->|setProject maskSegments| MS BS -->|From BackgroundSource| BG BG -->|GradientOrColorUniforms| WGSL MS -->|interpolate_masks| MASK CAM -->|frame drop optimisation| SC PERM -->|optional camera/mic| SCComments Outside Diff (2)
apps/desktop/src/routes/camera.tsx, line 1175-1183 (link)The guard condition
currentFrame.data !== imageData.datacompares the innerImageDatareference — which is always the same reusable buffer returned bygetReusableFrameData. After the very first frame is rendered:currentFrame.data→ the reusableImageDataobjectimageData.data→ the same reusableImageDataobjectcurrentFrame.data !== imageData.datais alwaysfalsesetLatestFrameis never called againIf canvas redraws are driven by the
latestFramesignal (which this component relies on), the preview freezes on the first frame even though the pixel buffer is still being updated in-place.Compare this with the correctly-implemented
CameraPreviewInlineintarget-select-overlay.tsx, which callsdrawFrame(imageData)unconditionally on every frame so the canvas is always painted, and only usessetFramefor auxiliary state:If unconditional signal updates are too expensive, add a direct
putImageDatacall (as done intarget-select-overlay.tsx) and then keep the guard only for the dimensions-tracking branch.Prompt To Fix With AI
crates/rendering/src/layers/background.rs, line 33-68 (link)The previous code called
srgb_to_linear(from[0])(which applies the ~2.2 gamma curve) before uploading values to the GPU. This PR replaces it with a straightfrom[0] as f32 / 255.0, which keeps values in sRGB space.The difference is non-trivial for mid-range values. For example, R=128:
srgb_to_linear(128)≈ 0.216 (linear)128 / 255.0≈ 0.502 (sRGB, ~2.3× brighter in linear terms)If the wgpu surface/swap-chain is configured for sRGB output (i.e. the hardware applies the inverse gamma on write), passing pre-gamma values was correct and this change will produce overly saturated / bright backgrounds in the exported video. If the surface is linear, the old code was wrong and this fixes it.
This looks intentional (the corresponding gradient-editor CSS preview now matches the WGSL shader since both operate on the same 0-255 sRGB space), but it is a visible rendering behaviour change for every existing project that uses a gradient or solid-color background. It should be explicitly documented and the render-test baselines updated so it doesn't silently regress.
Prompt To Fix With AI
Prompt To Fix All With AI
Last reviewed commit: "fmt"