feat: keyboard shortcut overlay pipeline#1636
feat: keyboard shortcut overlay pipeline#1636chindris-mihai-alexandru wants to merge 13 commits intoCapSoftware:mainfrom
Conversation
chindris-mihai-alexandru
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
crates/recording/src/cursor.rs
Outdated
| _ if format!("{key:?}").contains("Meta") | ||
| || format!("{key:?}").contains("Win") | ||
| || format!("{key:?}").contains("Command") => | ||
| { | ||
| "Meta".to_string() | ||
| } |
There was a problem hiding this comment.
using format!("{key:?}").contains("Meta") is fragile and relies on Debug impl details
| _ if format!("{key:?}").contains("Meta") | |
| || format!("{key:?}").contains("Win") | |
| || format!("{key:?}").contains("Command") => | |
| { | |
| "Meta".to_string() | |
| } | |
| Keycode::LMeta | Keycode::RMeta => "Meta".to_string(), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/cursor.rs
Line: 41-46
Comment:
using `format!("{key:?}").contains("Meta")` is fragile and relies on Debug impl details
```suggestion
Keycode::LMeta | Keycode::RMeta => "Meta".to_string(),
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 8dee855. Replaced Debug-string checks with explicit Keycode variant matching for Meta keys in keycode_to_label.
| if keys.iter().any(|key| { | ||
| let label = format!("{key:?}"); | ||
| label.contains("Meta") || label.contains("Win") || label.contains("Command") | ||
| }) { | ||
| modifiers.push("Meta".to_string()); |
There was a problem hiding this comment.
same issue as above - use pattern matching for Meta keys
| if keys.iter().any(|key| { | |
| let label = format!("{key:?}"); | |
| label.contains("Meta") || label.contains("Win") || label.contains("Command") | |
| }) { | |
| modifiers.push("Meta".to_string()); | |
| if keys.iter().any(|key| { | |
| matches!(key, device_query::Keycode::LMeta | device_query::Keycode::RMeta) | |
| }) { |
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/cursor.rs
Line: 73-77
Comment:
same issue as above - use pattern matching for Meta keys
```suggestion
if keys.iter().any(|key| {
matches!(key, device_query::Keycode::LMeta | device_query::Keycode::RMeta)
}) {
```
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Addressed in 8dee855. active_modifiers now uses explicit matches! over Meta/Command variants instead of Debug-string matching.
| fn is_modifier(key: &device_query::Keycode) -> bool { | ||
| let label = format!("{key:?}"); | ||
| label.contains("Shift") | ||
| || label.contains("Control") | ||
| || label.contains("Alt") | ||
| || label.contains("Meta") | ||
| || label.contains("Win") | ||
| || label.contains("Command") | ||
| } |
There was a problem hiding this comment.
using format! with Debug to check modifier keys is inefficient - consider using explicit pattern matching with the device_query Keycode enum variants instead
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/cursor.rs
Line: 60-68
Comment:
using `format!` with Debug to check modifier keys is inefficient - consider using explicit pattern matching with the device_query Keycode enum variants instead
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Updated in 8dee855. is_modifier now relies on explicit Keycode pattern matching (Shift/Ctrl/Alt/Option/Meta/Command) rather than format!("{key:?}") checks.
| "ControlLeft", | ||
| "ControlRight", | ||
| "Alt", | ||
| "Option", | ||
| "Opt", | ||
| "AltLeft", | ||
| "AltRight", | ||
| "Shift", | ||
| "ShiftLeft", | ||
| "ShiftRight", | ||
| ]); | ||
| const modifierOrder = new Map([ | ||
| ["⌃", 0], | ||
| ["⌥", 1], | ||
| ["⇧", 2], | ||
| ["⌘", 3], | ||
| ]); | ||
|
|
||
| const isModifierKey = (key: string) => modifierKeys.has(key); | ||
|
|
||
| const normalizeModifier = ( | ||
| modifier: string, | ||
| ): "⌘" | "⌃" | "⌥" | "⇧" | null => { | ||
| switch (modifier) { | ||
| case "Meta": | ||
| case "Command": | ||
| case "Cmd": | ||
| case "Super": | ||
| case "Win": | ||
| return "⌘"; | ||
| case "Ctrl": | ||
| case "Control": |
There was a problem hiding this comment.
modifier normalization logic is duplicated in three places (TypeScript UI, Rust rendering layer, Rust recording) - consider extracting to a shared module to prevent divergence
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Player.tsx
Line: 563-594
Comment:
modifier normalization logic is duplicated in three places (TypeScript UI, Rust rendering layer, Rust recording) - consider extracting to a shared module to prevent divergence
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Reduced drift by aligning shortcut behavior across paths in 8dee855 (same shortcut-modifier gating in editor preview and renderer). Full cross-language dedupe is larger since this spans TS + Rust, so I kept this PR scoped and consistent.
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| const normalizedModifiers = event.active_modifiers | ||
| .filter((modifier) => modifier !== event.key) | ||
| .map(normalizeModifier) | ||
| .filter( | ||
| (modifier): modifier is "⌘" | "⌃" | "⌥" | "⇧" => modifier !== null, | ||
| ) | ||
| .sort() | ||
| .filter((value, index, values) => values.indexOf(value) === index) | ||
| .sort( | ||
| (a, b) => | ||
| (modifierOrder.get(a) ?? Number.POSITIVE_INFINITY) - | ||
| (modifierOrder.get(b) ?? Number.POSITIVE_INFINITY), | ||
| ); | ||
| const label = [...normalizedModifiers, normalizeKey(event.key)].join( | ||
| " + ", | ||
| ); |
There was a problem hiding this comment.
This currently displays single-key presses too (no Ctrl/Alt/Meta), which can accidentally surface typed content. If the intent is “shortcut overlay”, consider only tracking keydowns when there’s at least one chord modifier, while still processing keyup to clear state.
| const normalizedModifiers = event.active_modifiers | |
| .filter((modifier) => modifier !== event.key) | |
| .map(normalizeModifier) | |
| .filter( | |
| (modifier): modifier is "⌘" | "⌃" | "⌥" | "⇧" => modifier !== null, | |
| ) | |
| .sort() | |
| .filter((value, index, values) => values.indexOf(value) === index) | |
| .sort( | |
| (a, b) => | |
| (modifierOrder.get(a) ?? Number.POSITIVE_INFINITY) - | |
| (modifierOrder.get(b) ?? Number.POSITIVE_INFINITY), | |
| ); | |
| const label = [...normalizedModifiers, normalizeKey(event.key)].join( | |
| " + ", | |
| ); | |
| const normalizedModifiers = event.active_modifiers | |
| .filter((modifier) => modifier !== event.key) | |
| .map(normalizeModifier) | |
| .filter( | |
| (modifier): modifier is "⌘" | "⌃" | "⌥" | "⇧" => modifier !== null, | |
| ) | |
| .sort() | |
| .filter((value, index, values) => values.indexOf(value) === index) | |
| .sort( | |
| (a, b) => | |
| (modifierOrder.get(a) ?? Number.POSITIVE_INFINITY) - | |
| (modifierOrder.get(b) ?? Number.POSITIVE_INFINITY), | |
| ); | |
| const hasShortcutModifier = normalizedModifiers.some( | |
| (modifier) => modifier === "⌘" || modifier === "⌃" || modifier === "⌥", | |
| ); | |
| if (event.down && !hasShortcutModifier) continue; | |
| const label = [...normalizedModifiers, normalizeKey(event.key)].join( | |
| " + ", | |
| ); |
There was a problem hiding this comment.
Implemented in 8dee855. Player.tsx now ignores down events that do not include ⌘/⌃/⌥, while retaining key-up processing for state cleanup.
| let mut parts = mods; | ||
| parts.push(normalize_key_label(&event.key)); | ||
| let label = parts.join(" + "); | ||
|
|
||
| if event.down { | ||
| let state = ShortcutState { | ||
| label, | ||
| down_time: event.time_ms, | ||
| }; | ||
|
|
||
| active.insert(event.key.clone(), state.clone()); | ||
|
|
||
| if let Some(last) = &last_recent { | ||
| if state.down_time > last.down_time { | ||
| last_recent = Some(state); | ||
| } | ||
| } else { | ||
| last_recent = Some(state); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Same concern on the renderer side: as-is, this will happily render plain keys / shift-only chords into the export. If this is meant to be “shortcuts”, filtering to ⌘/⌃/⌥ for down events keeps key-up handling intact and avoids showing typed text.
| let mut parts = mods; | |
| parts.push(normalize_key_label(&event.key)); | |
| let label = parts.join(" + "); | |
| if event.down { | |
| let state = ShortcutState { | |
| label, | |
| down_time: event.time_ms, | |
| }; | |
| active.insert(event.key.clone(), state.clone()); | |
| if let Some(last) = &last_recent { | |
| if state.down_time > last.down_time { | |
| last_recent = Some(state); | |
| } | |
| } else { | |
| last_recent = Some(state); | |
| } | |
| } else { | |
| if is_modifier_key(&event.key) { | |
| continue; | |
| } | |
| let has_shortcut_modifier = mods | |
| .iter() | |
| .any(|symbol| symbol == "⌘" || symbol == "⌃" || symbol == "⌥"); | |
| if event.down && !has_shortcut_modifier { | |
| continue; | |
| } | |
| let mut parts = mods; | |
| parts.push(normalize_key_label(&event.key)); | |
| let label = parts.join(" + "); | |
| if event.down { | |
| let state = ShortcutState { | |
| label, | |
| down_time: event.time_ms, | |
| }; | |
| active.insert(event.key.clone(), state.clone()); | |
| if let Some(last) = &last_recent { | |
| if state.down_time > last.down_time { | |
| last_recent = Some(state); | |
| } | |
| } else { | |
| last_recent = Some(state); | |
| } | |
| } else { | |
| active.remove(&event.key); | |
| } |
There was a problem hiding this comment.
Implemented in 8dee855. Renderer path now applies the same shortcut-modifier gating for down events to avoid plain-key overlays.
| let mut parts = mods; | ||
| parts.push(normalize_key_label(&event.key)); | ||
| let label = parts.join(" + "); |
There was a problem hiding this comment.
(Suggestion block tweak so it applies cleanly without duplicating the existing is_modifier_key check.)
| let mut parts = mods; | |
| parts.push(normalize_key_label(&event.key)); | |
| let label = parts.join(" + "); | |
| let has_shortcut_modifier = mods | |
| .iter() | |
| .any(|symbol| symbol == "⌘" || symbol == "⌃" || symbol == "⌥"); | |
| if event.down && !has_shortcut_modifier { | |
| continue; | |
| } | |
| let mut parts = mods; | |
| parts.push(normalize_key_label(&event.key)); | |
| let label = parts.join(" + "); |
There was a problem hiding this comment.
Applied that exact placement in 8dee855 (shortcut-modifier gate before label assembly, without duplicating the existing is_modifier_key branch).
| let _ = self.text_renderer.prepare( | ||
| device, | ||
| queue, | ||
| &mut self.font_system, | ||
| &mut self.text_atlas, | ||
| &self.viewport, | ||
| text_areas, | ||
| &mut self.swash_cache, | ||
| ); | ||
| } |
There was a problem hiding this comment.
TextRenderer::prepare returning an error here is probably worth handling; right now failures are silently ignored, but visible stays true and we still try to render.
| let _ = self.text_renderer.prepare( | |
| device, | |
| queue, | |
| &mut self.font_system, | |
| &mut self.text_atlas, | |
| &self.viewport, | |
| text_areas, | |
| &mut self.swash_cache, | |
| ); | |
| } | |
| if self | |
| .text_renderer | |
| .prepare( | |
| device, | |
| queue, | |
| &mut self.font_system, | |
| &mut self.text_atlas, | |
| &self.viewport, | |
| text_areas, | |
| &mut self.swash_cache, | |
| ) | |
| .is_err() | |
| { | |
| self.visible = false; | |
| self.current_label = None; | |
| self.overlays.clear(); | |
| return; | |
| } |
There was a problem hiding this comment.
Handled in 8dee855. TextRenderer::prepare errors now clear visibility/state and return early instead of failing silently.
apps/desktop/src-tauri/src/lib.rs
Outdated
| let full_path = meta.path(path); | ||
| CursorEvents::load_from_file(&full_path) | ||
| .map(|events| events.keyboard) | ||
| .unwrap_or_default() |
There was a problem hiding this comment.
Swallowing cursor-event load errors makes it hard to debug corrupted/partial recordings. Consider logging the error (still returning an empty list so the UI keeps working).
| .unwrap_or_default() | |
| CursorEvents::load_from_file(&full_path) | |
| .map(|events| events.keyboard) | |
| .unwrap_or_else(|error| { | |
| tracing::warn!( | |
| path = %full_path.display(), | |
| error = %error, | |
| "Failed to load cursor events" | |
| ); | |
| Vec::new() | |
| }) |
There was a problem hiding this comment.
Implemented in e022a56. get_keyboard_events now logs cursor-event load failures with warn! (including path and error) and still returns an empty vector for resilience.
crates/recording/src/cursor.rs
Outdated
| if keys.iter().any(|key| { | ||
| matches!( | ||
| key, | ||
| device_query::Keycode::LAlt | device_query::Keycode::RAlt |
There was a problem hiding this comment.
On macOS device_query uses LOption/ROption for the Option key; without including those here ⌥ chords won’t be detected.
| device_query::Keycode::LAlt | device_query::Keycode::RAlt | |
| matches!( | |
| key, | |
| device_query::Keycode::LAlt | |
| | device_query::Keycode::RAlt | |
| | device_query::Keycode::LOption | |
| | device_query::Keycode::ROption | |
| ) |
There was a problem hiding this comment.
Implemented in e022a56. Added LOption/ROption alongside LAlt/RAlt in active_modifiers, so macOS Option chords are correctly surfaced as Alt/⌥.
Co-authored-by: tembo[bot] <208362400+tembo[bot]@users.noreply.github.com>
Co-authored-by: tembo[bot] <208362400+tembo[bot]@users.noreply.github.com>
Co-authored-by: tembo[bot] <208362400+tembo[bot]@users.noreply.github.com>
1 participant
Summary
get_keyboard_events) and renders active/recent shortcuts with normalized modifiers and fade-out behavior.Implementation Details
KeyboardEventto cursor event models incrates/project/src/cursor.rs.crates/recording/src/cursor.rsand persisted them in studio recording metadata.get_keyboard_eventscommand inapps/desktop/src-tauri/src/lib.rs.apps/desktop/src/routes/editor/Player.tsx.crates/rendering/src/layers/keyboard.rscrates/rendering/src/shaders/keyboard-overlay.wgslcrates/rendering/src/layers/mod.rsandcrates/rendering/src/lib.rsCompatibility
Greptile Summary
This PR adds end-to-end keyboard shortcut overlay support to the screen recorder by capturing keyboard events during recording and rendering them in both the preview player and final rendered video.
Key changes:
KeyboardEventmodel with backward-compatible serialization (#[serde(default)])device_querycrate with modifier trackingget_keyboard_eventsTauri command to expose events to desktop editorIssues found:
cursor.rsuses fragile Debug format string checking instead of pattern matchingThe implementation is comprehensive with good test coverage in the rendering layer and proper backward compatibility for existing recordings.
Confidence Score: 4/5
crates/recording/src/cursor.rswhich uses fragile Debug format string matching for keycode detectionImportant Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Recording Started] --> B[device_query captures keyboard state] B --> C{Key state changed?} C -->|Yes| D[Create KeyboardEvent with modifiers] C -->|No| B D --> E[Store in CursorActorResponse] E --> F[Flush to cursor data JSON every 5s] F --> G[Recording Complete] G --> H[Desktop Editor Loads Recording] H --> I[get_keyboard_events Tauri command] I --> J[Load KeyboardEvents from cursor JSON] J --> K[Player.tsx Preview] K --> L[Calculate current source time] L --> M{Active shortcut?} M -->|Yes| N[Show overlay with full opacity] M -->|Released recently| O[Show with fade-out 0-850ms] M -->|None| P[Hide overlay] J --> Q[Rendering Pipeline] Q --> R[KeyboardLayer.prepare] R --> S[active_shortcut_label logic] S --> T{Active shortcut?} T -->|Yes| U[Create rounded rect overlays] T -->|No| V[Skip rendering] U --> W[Render with glyphon text] W --> X[Final rendered video]Last reviewed commit: 839612b
(5/5) You can turn off certain types of comments like style here!