feat: dead zone logic for auto-zoom to prevent jittery segments#1664
feat: dead zone logic for auto-zoom to prevent jittery segments#1664namearth5005 wants to merge 7 commits intoCapSoftware:mainfrom
Conversation
…nfigurable fields
…placing hardcoded constants
| } else { | ||
| let count = group_positions.len() as f64; | ||
| let centroid_x = group_positions.iter().map(|(x, _)| x).sum::<f64>() / count; | ||
| let centroid_y = group_positions.iter().map(|(_, y)| y).sum::<f64>() / count; | ||
| let dx = cx - centroid_x; | ||
| let dy = cy - centroid_y; | ||
| (dx * dx + dy * dy).sqrt() < dead_zone_radius | ||
| } |
There was a problem hiding this comment.
Centroid drift: repeated near-boundary clicks can shift the dead zone across the screen
The centroid used for the dead zone check is recomputed as the average of all positions already in the group, including those added via previous dead zone merges. Each time a click near the boundary of the dead zone is accepted, the centroid shifts slightly in that direction, which may then accept the next click that is slightly further away, and so on.
Concretely with dead_zone_radius = 0.1:
- Click at
(0.50, 0.50)→ centroid(0.50, 0.50) - Click at
(0.56, 0.56)(dist ≈ 0.085 < 0.1) → merges, centroid shifts to(0.53, 0.53) - Click at
(0.62, 0.62)(dist from new centroid ≈ 0.127 > 0.1) — just misses in this case, but with slightly smaller steps the group can creep across the screen.
For the typical use case (same button clicked repeatedly) this is harmless, but a user doing a slow drag-with-pauses could inadvertently merge distant clicks into one group. One mitigation is to compute the centroid only from the first click in the group (i.e. the "anchor"), rather than from the running average of all members.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/recording.rs
Line: 2101-2108
Comment:
**Centroid drift: repeated near-boundary clicks can shift the dead zone across the screen**
The centroid used for the dead zone check is recomputed as the average of **all** positions already in the group, including those added via previous dead zone merges. Each time a click near the boundary of the dead zone is accepted, the centroid shifts slightly in that direction, which may then accept the next click that is slightly further away, and so on.
Concretely with `dead_zone_radius = 0.1`:
- Click at `(0.50, 0.50)` → centroid `(0.50, 0.50)`
- Click at `(0.56, 0.56)` (dist ≈ 0.085 < 0.1) → merges, centroid shifts to `(0.53, 0.53)`
- Click at `(0.62, 0.62)` (dist from new centroid ≈ 0.127 > 0.1) — just misses in this case, but with slightly smaller steps the group can creep across the screen.
For the typical use case (same button clicked repeatedly) this is harmless, but a user doing a slow drag-with-pauses could inadvertently merge distant clicks into one group. One mitigation is to compute the centroid only from the **first** click in the group (i.e. the "anchor"), rather than from the running average of all members.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Adds dead zone filtering to auto-zoom segment generation (refs #1646, builds on #1663):
dead_zone_radiusconfig field (default 0.1 = 10% of screen) — if cursor stays within this radius of a previous click group's centroid, clicks merge into the same group regardless of time gapHow it works
Before: Two clicks 4s apart at (0.5, 0.5) and (0.55, 0.55) → 2 separate zoom segments → jittery zoom-in/out
After: Same clicks with
dead_zone_radius: 0.1→ grouped into 1 segment → smooth experienceBackward compatibility
dead_zone_radiusdefaults to 0.1 via#[serde(default)]. Existing recordings are unaffected since auto-zoom segments are regenerated from click data, not stored with this parameter.Test plan
dead_zone_merges_nearby_clicks— clicks within radius merge to 1 segmentdead_zone_allows_distant_clicks— clicks outside radius stay separatedead_zone_zero_disables— radius 0.0 produces same behavior as beforeGreptile Summary
This PR adds a configurable
dead_zone_radiusto the auto-zoom click-grouping algorithm so that spatially close clicks are merged into a single zoom segment even when the time gap between them exceedsclick_group_time_threshold, preventing jittery zoom-in/out on repeated interactions with the same UI element. It also lifts all previously hard-coded zoom constants into a newAutoZoomConfigstruct and exposes three of them (zoom amount, sensitivity, smoothing) as UI sliders in the Experimental settings page.Key observations:
dead_zone_merges_nearby_clickstest assertionsegments.len() <= 1passes even when zero segments are produced, which would indicate the feature is broken rather than working; it should assert== 1.dead_zone_radiusis included inAutoZoomConfigand its TypeScript defaults, but no UI slider is exposed for it, so users cannot tune the feature that this PR is specifically introducing.Confidence Score: 3/5
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[New click event] --> B{Has position?} B -- No --> C[time_and_spatial check only] B -- Yes --> D{dead_zone_radius > 0?} D -- No --> C D -- Yes --> E[Iterate existing click_groups] C --> E E --> F{time_and_spatial:\ntime_close AND spatial_close?} F -- Yes --> G[Merge into group\nbreak] F -- No --> H{in_dead_zone:\ndist to running centroid < radius?} H -- Yes --> G H -- No --> I{More groups?} I -- Yes --> E I -- No --> J[Create new group] G --> K[Recompute centroid\ncentroid drifts toward new click] J --> L[Centroid = single click position] K --> M[Generate interval:\ngroup_start − pre_padding .. group_end + post_padding] L --> M M --> N{duration ≥ min_segment_duration?} N -- Yes --> O[Add ZoomSegment] N -- No --> P[Discard]Comments Outside Diff (2)
apps/desktop/src-tauri/src/recording.rs, line 2571-2580 (link)The assertion
segments.len() <= 1also passes whensegmentsis empty (0 segments). This means if the dead zone merge logic silently failed to group the two nearby clicks but still produced zero segments (e.g. the merged group's interval was filtered out), the test would still pass, giving a false green. Since the two merged clicks form an interval of[0.6s, 6.8s](duration 6.2s), which comfortably exceedsmin_segment_duration = 1.0s, exactly 1 segment should always be produced. The assertion should be tightened to== 1.Prompt To Fix With AI
apps/desktop/src-tauri/src/recording.rs, line 2093-2115 (link)The
in_dead_zonecheck iterates over all existingclick_groupswith no temporal filter. This means that if a user clicks at position (0.5, 0.5) at t=0s, clicks somewhere else for a long stretch, and then clicks at (0.51, 0.51) at t=120s, the second click will merge into the very first group — producing a single zoom segment spanning over 2 minutes once the post-padding is applied.The PR description calls this intentional ("regardless of time gap"), but in a long recording this silently creates a single enormous zoom segment from the union of
[group_start − click_pre_padding, last_click + click_post_padding], which is almost certainly not what the user expects.A practical guard would be to bound the dead zone merge by the same
click_group_time_thresholdused for normal spatial grouping, or by some separate configurable dead zone time limit (e.g.dead_zone_time_limit_secs). At minimum the behaviour should be documented in a comment so future reviewers understand the intentional "any time gap" design.Prompt To Fix With AI
Prompt To Fix All With AI
Last reviewed commit: "feat(recording): add..."
(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!