Skip to content

Improved rendering engine, recording resilience, and analytics trackingRendering engine tweaks#1674

Merged
richiemcilroy merged 35 commits intomainfrom
rendering-engine-tweaks
Mar 21, 2026
Merged

Improved rendering engine, recording resilience, and analytics trackingRendering engine tweaks#1674
richiemcilroy merged 35 commits intomainfrom
rendering-engine-tweaks

Conversation

@richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Mar 21, 2026

  • Rendering engine: Rewrite cursor interpolation to use a fixed-timestep spring simulation with lookahead click targeting, replace densification with decimation, and simplify both cursor and display motion blur shaders into cleaner box/radial kernels. Add cursor x-axis tilt based on movement delta, anticipatory click animations, idle fade lookahead, edge-snapped zoom follow, and continuous zoom-out transitions with ramp easing. Switch zoom focus interpolation to lazy on-demand precomputation with cluster-based targeting for auto-zoom segments.
  • Recording pipeline resilience: Extract shared blocking-thread-finish and mux-send-error helpers used across all macOS/Windows muxer implementations. Propagate encoder failures as errors instead of warn-and-continue. Track optional pipeline (mic, camera, system audio) failures separately from display so a degraded track no longer aborts the entire recording — failures are captured in a recording-diagnostics.json sidecar.
  • Editor playback: Tune mid-start warmup parameters, drain prefetch buffers more aggressively, add extended retry with forward-skip recovery on frame misses, and switch to blocking render sends with a larger channel and drain-flush cycle.
  • Decoder: Use forward-only cache fallback during sequential playback (non-scrubbing) and add decoder pool bounds checking to prevent out-of-range access.
  • Encoding: Fix AVFoundation asset writer failures on UYVY camera sample buffers by rebuilding the sample buffer with a fresh format description instead of using copy_with_new_timing.
  • Analytics: Associate desktop PostHog events with the authenticated user's distinct ID, add auth_surface property to all auth tracking events, fix is_signup flags on login forms, add a 7-day window guard for signup tracking, and correct the Stripe webhook platform field to a proper enum.
  • Defaults: Tune click spring (530/1.0/40), default motion blur (1.0), cursor rotation amount (0.15), and add a cursor tilt slider to the editor config sidebar.

Greptile Summary

This is a broad, well-scoped improvement PR touching the rendering engine, recording resilience, editor playback, decoder, encoder, and analytics layers. The changes are generally sound and well-tested, but two issues in the rendering pipeline need attention before merge.

Key changes:

  • Cursor interpolation: Replaced event-driven spring simulation with a fixed-timestep (60 fps) simulation loop; densification replaced with decimation; click lookahead and idle-fade lookahead added. Previously flagged O(n) linear scans replaced with binary-search partition_point.
  • Recording resilience: Optional pipeline tracks (mic, camera, system audio) now fail gracefully and log to a recording-diagnostics.json sidecar instead of aborting the entire recording. Encoder failures now propagate as errors rather than warn-and-continue. Shared helpers extracted for blocking-thread-finish and mux-send-error patterns.
  • Encoder fix: AVFoundation UYVY camera sample buffers are now rebuilt with a fresh format description instead of copy_with_new_timing, fixing writer failures on camera overlay tracks.
  • Decoder: Forward-only cache fallback during sequential playback; decoder pool bounds check fixes a potential out-of-range access (best_id vs decoder_idx mismatch).
  • Zoom focus: Lazy on-demand incremental precomputation replaces full upfront precompute; cluster-based cursor targeting for auto-zoom segments.
  • Analytics: PostHog desktop events associated with authenticated user's distinct ID; auth_surface added to all auth tracking events; is_signup flags corrected on login forms; 7-day signup tracking window guard; Stripe webhook platform field corrected to a proper enum.
  • loadOp: "load" in webgpu-renderer.ts: Both render paths changed from "clear" to "load", which risks undefined/stale pixels on the first frame or after a GPU device-lost event.
  • cursor.wgsl offset_base: The expression offset_base = -vel_len / 2.0 / vel_len - 0.5 always simplifies to -1.0, making the motion blur sampling range a fixed constant. This is likely unintentional and could produce incorrect visual results depending on the design intent.

Confidence Score: 3/5

  • Safe to merge for recording/analytics changes; two rendering issues (loadOp and cursor shader offset_base) need verification before merge.
  • The recording pipeline, encoder, decoder, and analytics changes are well-implemented and well-tested. The cursor spring simulation rewrite is solid. However, both rendering issues are in the hot visual path: the loadOp: "load" change risks first-frame artifacts in the WebGPU renderer for both playback paths, and the offset_base expression in cursor.wgsl always evaluates to -1.0 regardless of velocity, which may or may not be the intended blur range but is at minimum misleading and could produce incorrect visual output.
  • apps/desktop/src/utils/webgpu-renderer.ts (both loadOp changes) and crates/rendering/src/shaders/cursor.wgsl (offset_base expression).

Important Files Changed

Filename Overview
crates/rendering/src/shaders/cursor.wgsl Cursor motion blur simplified from Gaussian to box kernel; offset_base expression always simplifies to -1.0 due to vel_len cancellation, making the sampling range a constant regardless of intent.
apps/desktop/src/utils/webgpu-renderer.ts loadOp changed from "clear" to "load" on both render paths; risks stale/garbage pixels on first frame render or after GPU device loss events.
crates/rendering/src/cursor_interpolation.rs Full rewrite from event-driven to fixed-timestep spring simulation; replaces densification with decimation, adds click lookahead and idle-fade lookahead; O(n) scans from previous review replaced with binary-search partition_point.
crates/recording/src/output_pipeline/core.rs Shared helpers extracted for blocking-thread-finish and mux-send-error; encoder failures now propagate as errors rather than warn-and-continue; covered by new unit tests.
crates/recording/src/studio_recording.rs Optional pipeline tracks (mic, camera, system audio) now fail gracefully instead of aborting the recording; diagnostics are captured in a recording-diagnostics.json sidecar; well-tested.
crates/enc-avfoundation/src/mp4.rs Fixes AVFoundation asset writer failures on UYVY camera buffers by rebuilding sample buffers with fresh format descriptions; comprehensive test matrix added for UYVY camera scenarios.
crates/rendering/src/decoder/multi_position.rs Adds bounds check for empty decoder pool and filters pool positions by decoder_count, preventing out-of-range access; fixes best_id vs decoder_idx mismatch in update_decoder_position.
crates/rendering/src/zoom_focus_interpolation.rs Replaces full upfront precompute with lazy on-demand incremental precomputation; adds cluster-based cursor targeting for auto-zoom segments; adds click_spring propagation to cursor interpolation.
apps/web/actions/analytics/track-user-signed-up.ts Moves signup-tracking window check from SQL CURRENT_DATE() to a JS-side 7-day window guard; adds getAffectedRows helper for cross-ORM result shape compatibility.
apps/web/app/api/webhooks/stripe/route.ts Stripe webhook platform field corrected from a boolean expression to a proper `"desktop"

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Recording Input] --> B{Track Type}
    B -->|Display - required| C[Display Pipeline]
    B -->|Mic/Camera/System - optional| D[Optional Track Pipelines]

    C --> E{Display Error?}
    E -->|Yes| F[Abort Recording]
    E -->|No| G[Continue]

    D --> H{Track Error?}
    H -->|Yes - Runtime| I[record_track_failure\nRuntime stage]
    H -->|Yes - Stop| J[finalize_optional_track\nStop stage]
    H -->|No| K[Track finished OK]

    I --> L[SharedTrackFailures\nArc-Mutex-Vec]
    J --> L
    K --> G

    G --> M[stop_recording]
    M --> N[take_track_failures]
    N --> O{Any failures?}
    O -->|Yes| P[Write recording-diagnostics.json]
    O -->|No| Q[Clean finish]
    P --> Q

    subgraph Cursor Rendering
        R[CursorEvents] --> S[decimate_cursor_moves]
        S --> T[build_smoothed_timeline\nFixed-timestep 60fps loop]
        T --> U[Click lookahead target\n500ms window]
        T --> V[Spring profile\nbinary search]
        U --> W[interpolate_timeline\nIndex lookup]
        V --> W
        W --> X[InterpolatedCursorPosition]
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/rendering/src/shaders/cursor.wgsl
Line: 97

Comment:
**`offset_base` always simplifies to `-1.0`**

The expression:
```wgsl
let offset_base = -vel_len / 2.0 / vel_len - 0.5;
```
cancels to `-(vel_len / 2.0) / vel_len - 0.5 = -0.5 - 0.5 = -1.0` regardless of `vel_len`. The loop then samples from `input.uv - velocity_uv` (i=0) to `input.uv - 0.05 * velocity_uv` (i=19), placing all 20 trailing samples strictly behind the cursor with no forward coverage.

If the intent was to center the kernel symmetrically (samples from `-0.5*velocity_uv` to `+0.5*velocity_uv`), `offset_base` should be `-0.5`. If the intent was to trail behind the cursor (samples from `-velocity_uv` to `0.0`), `offset_base` should simply be `-1.0`.

Either way, using the current expression with `vel_len` terms that cancel is misleading and masks the actual behavior. Suggest replacing with the explicit constant:
```suggestion
    let offset_base = -1.0;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/desktop/src/utils/webgpu-renderer.ts
Line: 302

Comment:
**`loadOp: "load"` may expose stale pixels on first frame**

Switching from `"clear"` to `"load"` skips resetting the attachment to `clearValue` before the render pass. In WebGPU, on the very first frame (or after a resize/GPU-device-lost recovery), the texture contents are **undefined**`"load"` will then composite on top of garbage memory rather than a clean black background. This can manifest as single-frame visual glitches during playback start or after seeking.

The same change is applied to both `renderFrameWebGPU` (line 301) and `renderNv12FrameWebGPU` (line 415).

If the goal is to avoid the clear cost when the composite shader always writes every pixel, an alternative is to keep `"clear"` but only for the first frame render of a new session. Otherwise, at minimum add a comment confirming the shader guarantees full-pixel coverage so future readers understand the invariant.

```suggestion
				loadOp: "clear",
				clearValue: { r: 0, g: 0, b: 0, a: 1 },
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/desktop/src/utils/webgpu-renderer.ts
Line: 415

Comment:
**Same `loadOp: "load"` concern for Nv12 path**

This is the same `"clear"``"load"` change applied to `renderNv12FrameWebGPU`. If the composite pass does not guarantee full pixel coverage on the first frame, the Nv12 render path will have the same stale-pixel risk.

```suggestion
				loadOp: "clear",
				clearValue: { r: 0, g: 0, b: 0, a: 1 },
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: "fix(web): handle arr..."

Greptile also left 3 inline comments on this PR.

@richiemcilroy
Copy link
Member Author

@greptileai please re-review the PR

let kernel_size = 21;
let k = kernel_size - 1;
let offset_base = -vel_len / 2.0 / vel_len - 0.5;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 offset_base always simplifies to -1.0

The expression:

let offset_base = -vel_len / 2.0 / vel_len - 0.5;

cancels to -(vel_len / 2.0) / vel_len - 0.5 = -0.5 - 0.5 = -1.0 regardless of vel_len. The loop then samples from input.uv - velocity_uv (i=0) to input.uv - 0.05 * velocity_uv (i=19), placing all 20 trailing samples strictly behind the cursor with no forward coverage.

If the intent was to center the kernel symmetrically (samples from -0.5*velocity_uv to +0.5*velocity_uv), offset_base should be -0.5. If the intent was to trail behind the cursor (samples from -velocity_uv to 0.0), offset_base should simply be -1.0.

Either way, using the current expression with vel_len terms that cancel is misleading and masks the actual behavior. Suggest replacing with the explicit constant:

Suggested change
let offset_base = -1.0;
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/rendering/src/shaders/cursor.wgsl
Line: 97

Comment:
**`offset_base` always simplifies to `-1.0`**

The expression:
```wgsl
let offset_base = -vel_len / 2.0 / vel_len - 0.5;
```
cancels to `-(vel_len / 2.0) / vel_len - 0.5 = -0.5 - 0.5 = -1.0` regardless of `vel_len`. The loop then samples from `input.uv - velocity_uv` (i=0) to `input.uv - 0.05 * velocity_uv` (i=19), placing all 20 trailing samples strictly behind the cursor with no forward coverage.

If the intent was to center the kernel symmetrically (samples from `-0.5*velocity_uv` to `+0.5*velocity_uv`), `offset_base` should be `-0.5`. If the intent was to trail behind the cursor (samples from `-velocity_uv` to `0.0`), `offset_base` should simply be `-1.0`.

Either way, using the current expression with `vel_len` terms that cancel is misleading and masks the actual behavior. Suggest replacing with the explicit constant:
```suggestion
    let offset_base = -1.0;
```

How can I resolve this? If you propose a fix, please make it concise.

view: currentTexture.createView(),
clearValue: { r: 0, g: 0, b: 0, a: 1 },
loadOp: "clear",
loadOp: "load",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 loadOp: "load" may expose stale pixels on first frame

Switching from "clear" to "load" skips resetting the attachment to clearValue before the render pass. In WebGPU, on the very first frame (or after a resize/GPU-device-lost recovery), the texture contents are undefined"load" will then composite on top of garbage memory rather than a clean black background. This can manifest as single-frame visual glitches during playback start or after seeking.

The same change is applied to both renderFrameWebGPU (line 301) and renderNv12FrameWebGPU (line 415).

If the goal is to avoid the clear cost when the composite shader always writes every pixel, an alternative is to keep "clear" but only for the first frame render of a new session. Otherwise, at minimum add a comment confirming the shader guarantees full-pixel coverage so future readers understand the invariant.

Suggested change
loadOp: "load",
loadOp: "clear",
clearValue: { r: 0, g: 0, b: 0, a: 1 },
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/utils/webgpu-renderer.ts
Line: 302

Comment:
**`loadOp: "load"` may expose stale pixels on first frame**

Switching from `"clear"` to `"load"` skips resetting the attachment to `clearValue` before the render pass. In WebGPU, on the very first frame (or after a resize/GPU-device-lost recovery), the texture contents are **undefined**`"load"` will then composite on top of garbage memory rather than a clean black background. This can manifest as single-frame visual glitches during playback start or after seeking.

The same change is applied to both `renderFrameWebGPU` (line 301) and `renderNv12FrameWebGPU` (line 415).

If the goal is to avoid the clear cost when the composite shader always writes every pixel, an alternative is to keep `"clear"` but only for the first frame render of a new session. Otherwise, at minimum add a comment confirming the shader guarantees full-pixel coverage so future readers understand the invariant.

```suggestion
				loadOp: "clear",
				clearValue: { r: 0, g: 0, b: 0, a: 1 },
```

How can I resolve this? If you propose a fix, please make it concise.

view: context.getCurrentTexture().createView(),
clearValue: { r: 0, g: 0, b: 0, a: 1 },
loadOp: "clear",
loadOp: "load",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Same loadOp: "load" concern for Nv12 path

This is the same "clear""load" change applied to renderNv12FrameWebGPU. If the composite pass does not guarantee full pixel coverage on the first frame, the Nv12 render path will have the same stale-pixel risk.

Suggested change
loadOp: "load",
loadOp: "clear",
clearValue: { r: 0, g: 0, b: 0, a: 1 },
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/utils/webgpu-renderer.ts
Line: 415

Comment:
**Same `loadOp: "load"` concern for Nv12 path**

This is the same `"clear"``"load"` change applied to `renderNv12FrameWebGPU`. If the composite pass does not guarantee full pixel coverage on the first frame, the Nv12 render path will have the same stale-pixel risk.

```suggestion
				loadOp: "clear",
				clearValue: { r: 0, g: 0, b: 0, a: 1 },
```

How can I resolve this? If you propose a fix, please make it concise.

@richiemcilroy richiemcilroy merged commit 8be2327 into main Mar 21, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant