-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(desktop): auto zoom from cursor movement + higher default export quality #1995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| 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; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. Since
Suggested change
|
||||||||||||||||||||||||||||||
| 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; | ||||||||||||||||||||||||||||||
|
|
@@ -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(); | ||||||||||||||||||||||||||||||
|
|
@@ -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 { | ||||||||||||||||||||||||||||||
|
|
@@ -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 | ||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the OS reports repeated pixel-level cursor changes while the cursor is effectively idle, Prompt To Fix With AIThis 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(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
@@ -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), | ||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 AIThis 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, | ||||||||||||||||||||||||||||||
|
|
@@ -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(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,7 +86,7 @@ function ShareButton() { | |
| x: RESOLUTION_OPTIONS._1080p.width, | ||
| y: RESOLUTION_OPTIONS._1080p.height, | ||
| }, | ||
| compression: "Web", | ||
| compression: "Social", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bumping the default compression makes sense. Might be worth a quick |
||
| custom_bpp: null, | ||
| }, | ||
| (msg) => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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