Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 173 additions & 3 deletions apps/desktop/src-tauri/src/recording.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3636,9 +3636,86 @@ async fn finalize_studio_recording(
Ok(())
}

const AUTO_ZOOM_DEFAULT_AMOUNT: f64 = 2.0;
const AUTO_ZOOM_MIN_AMOUNT: f64 = 1.5;
const AUTO_ZOOM_MAX_AMOUNT: f64 = 2.5;
const AUTO_ZOOM_FOCUS_COVERAGE: f64 = 0.85;
const AUTO_ZOOM_MIN_ACTIVITY_SPREAD: f64 = 0.34;

struct MovementBurst {
start_ms: f64,
end_ms: f64,
travel: f64,
}

fn detect_movement_bursts(moves: &[CursorMoveEvent], idle_gap_ms: f64) -> Vec<MovementBurst> {
let mut bursts = Vec::new();
let mut iter = moves.iter();
let Some(first) = iter.next() else {
return bursts;
};

let mut start_ms = first.time_ms;
let mut last_ms = first.time_ms;
let mut last_pos = (first.x, first.y);
let mut travel = 0.0;

for m in iter {
if m.time_ms - last_ms > idle_gap_ms {
bursts.push(MovementBurst {
start_ms,
end_ms: last_ms,
travel,
});
start_ms = m.time_ms;
travel = 0.0;
} else {
let dx = m.x - last_pos.0;
let dy = m.y - last_pos.1;
travel += (dx * dx + dy * dy).sqrt();
Comment on lines +3673 to +3675

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Cursor Streams Share Travel

This computes travel between consecutive move events without checking whether they came from the same cursor_id. If two cursor streams are interleaved by time, a small move from cursor A followed by a small move from cursor B can look like one large jump, causing a false movement burst and an auto-zoom segment that neither cursor produced on its own.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/recording.rs
Line: 3673-3675

Comment:
**Cursor Streams Share Travel**

This computes travel between consecutive move events without checking whether they came from the same `cursor_id`. If two cursor streams are interleaved by time, a small move from cursor A followed by a small move from cursor B can look like one large jump, causing a false movement burst and an auto-zoom segment that neither cursor produced on its own.

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

}
last_ms = m.time_ms;
last_pos = (m.x, m.y);
}

bursts.push(MovementBurst {
start_ms,
end_ms: last_ms,
travel,
});
bursts
}

fn zoom_amount_for_interval(moves: &[CursorMoveEvent], start_ms: f64, end_ms: f64) -> f64 {
let mut min_x = f64::MAX;
let mut max_x = f64::MIN;
let mut min_y = f64::MAX;
let mut max_y = f64::MIN;
let mut found = false;

for m in moves {
if m.time_ms < start_ms || m.time_ms > end_ms {
continue;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice. Since moves is sorted by time_ms now, you can break once you pass end_ms to avoid scanning the full list for every segment.

Suggested change
}
for m in moves {
if m.time_ms < start_ms {
continue;
}
if m.time_ms > end_ms {
break;
}
min_x = min_x.min(m.x);
max_x = max_x.max(m.x);
min_y = min_y.min(m.y);
max_y = max_y.max(m.y);
found = true;
}

min_x = min_x.min(m.x);
max_x = max_x.max(m.x);
min_y = min_y.min(m.y);
max_y = max_y.max(m.y);
found = true;
}

if !found {
return AUTO_ZOOM_DEFAULT_AMOUNT;
}

let spread = (max_x - min_x).max(max_y - min_y);
(AUTO_ZOOM_FOCUS_COVERAGE / spread.max(AUTO_ZOOM_MIN_ACTIVITY_SPREAD))
.clamp(AUTO_ZOOM_MIN_AMOUNT, AUTO_ZOOM_MAX_AMOUNT)
}

fn generate_zoom_segments_from_clicks_impl(
mut clicks: Vec<CursorClickEvent>,
_moves: Vec<CursorMoveEvent>,
mut moves: Vec<CursorMoveEvent>,
max_duration: f64,
) -> Vec<ZoomSegment> {
const MS_PER_SECOND: f64 = 1000.0;
Expand All @@ -3648,7 +3725,11 @@ fn generate_zoom_segments_from_clicks_impl(
const CLICK_END_CLAMP_PADDING_MS: f64 = 800.0;
const TRAILING_CLICK_IGNORE_MS: f64 = 1000.0;
const MERGE_GAP_MS: f64 = 2500.0;
const AUTO_ZOOM_AMOUNT: f64 = 2.0;
const MOVE_IDLE_GAP_MS: f64 = 500.0;
const MOVE_PRE_PADDING_MS: f64 = 300.0;
const MOVE_POST_PADDING_MS: f64 = 1500.0;
const MOVE_MIN_TRAVEL: f64 = 0.1;
const MOVE_MIN_DURATION_MS: f64 = 240.0;

if max_duration <= 0.0 {
return Vec::new();
Expand All @@ -3666,6 +3747,11 @@ fn generate_zoom_segments_from_clicks_impl(
.partial_cmp(&b.time_ms)
.unwrap_or(std::cmp::Ordering::Equal)
});
moves.sort_by(|a, b| {
a.time_ms
.partial_cmp(&b.time_ms)
.unwrap_or(std::cmp::Ordering::Equal)
});

let mut intervals: Vec<(f64, f64)> = Vec::new();
for click in clicks {
Expand All @@ -3682,6 +3768,22 @@ fn generate_zoom_segments_from_clicks_impl(
}
}

for burst in detect_movement_bursts(&moves, MOVE_IDLE_GAP_MS) {
if burst.start_ms >= click_cutoff_ms
|| burst.travel < MOVE_MIN_TRAVEL
|| burst.end_ms - burst.start_ms < MOVE_MIN_DURATION_MS

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Tiny Oscillation Becomes Zoom

When the OS reports repeated pixel-level cursor changes while the cursor is effectively idle, burst.travel can pass this total-distance check even though the pointer stays in a tiny area. That creates a movement-only zoom segment, and the small spread then drives the adaptive amount toward the maximum zoom, so idle jitter can show up as an unintended zoom in the final recording.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/recording.rs
Line: 3774

Comment:
**Tiny Oscillation Becomes Zoom**

When the OS reports repeated pixel-level cursor changes while the cursor is effectively idle, `burst.travel` can pass this total-distance check even though the pointer stays in a tiny area. That creates a movement-only zoom segment, and the small spread then drives the adaptive amount toward the maximum zoom, so idle jitter can show up as an unintended zoom in the final recording.

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

{
continue;
}

let start = (burst.start_ms - MOVE_PRE_PADDING_MS).max(START_MIN_MS);
let end = (burst.end_ms + MOVE_POST_PADDING_MS).min(end_limit_ms);

if end > start {
intervals.push((start, end));
}
}

if intervals.is_empty() {
return Vec::new();
}
Expand All @@ -3704,7 +3806,7 @@ fn generate_zoom_segments_from_clicks_impl(
.map(|(start, end)| ZoomSegment {
start: start.round() / MS_PER_SECOND,
end: end.round() / MS_PER_SECOND,
amount: AUTO_ZOOM_AMOUNT,
amount: zoom_amount_for_interval(&moves, start, end),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Merged Movement Dilutes Click Zoom

After click and movement intervals are merged, the amount is computed from every move inside the merged time window. A click near one screen area followed within the merge gap by movement far away can reduce the whole segment to the minimum zoom, so the click that previously received the fixed 2.0x auto-zoom becomes noticeably less focused.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/recording.rs
Line: 3809

Comment:
**Merged Movement Dilutes Click Zoom**

After click and movement intervals are merged, the amount is computed from every move inside the merged time window. A click near one screen area followed within the merge gap by movement far away can reduce the whole segment to the minimum zoom, so the click that previously received the fixed 2.0x auto-zoom becomes noticeably less focused.

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

mode: ZoomMode::Auto,
glide_direction: GlideDirection::None,
glide_speed: 0.5,
Expand Down Expand Up @@ -4274,6 +4376,74 @@ mod tests {
);
}

#[test]
fn generates_segment_for_sustained_movement_without_clicks() {
let moves = (0..40)
.map(|i| {
let t = 2_000.0 + (i as f64) * 30.0;
let x = 0.2 + (i as f64) * 0.01;
move_event(t, x, 0.5)
})
.collect::<Vec<_>>();

let segments = generate_zoom_segments_from_clicks_impl(Vec::new(), moves, 15.0);

assert_eq!(segments.len(), 1);
assert_eq!(segments[0].start, 1.7);
assert_eq!(segments[0].end, 4.67);
}

#[test]
fn zoom_amount_adapts_to_activity_spread() {
let tight_moves = (0..40)
.map(|i| {
let t = 2_000.0 + (i as f64) * 30.0;
let x = 0.5 + ((i % 2) as f64) * 0.005;
move_event(t, x, 0.5)
})
.collect::<Vec<_>>();
let tight = generate_zoom_segments_from_clicks_impl(Vec::new(), tight_moves, 15.0);

let wide_moves = (0..40)
.map(|i| {
let t = 2_000.0 + (i as f64) * 30.0;
let x = 0.1 + (i as f64) * 0.02;
move_event(t, x, 0.5)
})
.collect::<Vec<_>>();
let wide = generate_zoom_segments_from_clicks_impl(Vec::new(), wide_moves, 15.0);

assert_eq!(tight.len(), 1);
assert_eq!(wide.len(), 1);
assert!(
tight[0].amount > wide[0].amount,
"tight activity should zoom closer than wide activity: {} vs {}",
tight[0].amount,
wide[0].amount
);
}

#[test]
fn separate_movement_bursts_stay_split_across_idle() {
let mut moves = Vec::new();
for i in 0..20 {
let t = 1_000.0 + (i as f64) * 30.0;
moves.push(move_event(t, 0.1 + (i as f64) * 0.01, 0.3));
}
for i in 0..20 {
let t = 9_000.0 + (i as f64) * 30.0;
moves.push(move_event(t, 0.9 - (i as f64) * 0.01, 0.8));
}

let segments = generate_zoom_segments_from_clicks_impl(Vec::new(), moves, 20.0);

assert_eq!(
segments.len(),
2,
"bursts separated by a long idle gap should stay separate segments"
);
}

#[test]
fn marks_fragmented_recordings_for_ffmpeg_export() {
let dir = tempdir().unwrap();
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/routes/editor/ShareButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function ShareButton() {
x: RESOLUTION_OPTIONS._1080p.width,
y: RESOLUTION_OPTIONS._1080p.height,
},
compression: "Web",
compression: "Social",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bumping the default compression makes sense. Might be worth a quick rg "compression: \"Web\"" to make sure there aren’t other export entry points still defaulting to Web (to avoid inconsistent output quality across flows).

custom_bpp: null,
},
(msg) => {
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/src/routes/recordings-overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ function createRecordingMutations(
format: "Mp4",
fps: FPS,
resolution_base: OUTPUT_SIZE,
compression: "Web",
compression: "Social",
custom_bpp: null,
},
onProgress,
Expand Down Expand Up @@ -667,7 +667,7 @@ function createRecordingMutations(
format: "Mp4",
fps: FPS,
resolution_base: OUTPUT_SIZE,
compression: "Web",
compression: "Social",
custom_bpp: null,
},
fullFileName,
Expand Down