feat: restore cursor and click highlight feature#638
Open
kaili-yang wants to merge 2 commits into
Open
Conversation
…design Completed the cursor + click highlight features following the original author's design intent. Key changes: - Created cursorHighlight.ts: implemented CursorHighlightConfig defaults, rendering helpers, and opacity timing. - Updated cursorRenderer.ts: replaced the no-op drawClickRing() with real PixiJS ring/dot drawing logic. - Managed EditorState & projectPersistence.ts: added cursorHighlight settings to undoable history and serialization paths. - Added SettingsPanel.tsx UI: full configuration options (style, size, opacity, color, offset X/Y, only-on-clicks). - Updated frameRenderer.ts, gifExporter.ts, videoExporter.ts: rendered the highlights during MP4 and GIF exports.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (6)
📝 WalkthroughWalkthroughAdds end-to-end cursor highlight support: defines config and helpers, integrates highlight into PIXI overlay and VideoPlayback, persists/normalizes settings, exposes UI controls in SettingsPanel, and forwards the config to frame rendering and exporters. Translations for highlight UI were added across locales. ChangesCursor Highlight Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c8c9421ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+2292
to
+2293
| showCursorHighlightSettings={showCursorSettings} | ||
| cursorHighlightSupportsClicks={showCursorSettings} |
There was a problem hiding this comment.
These props expose cursor-highlight controls whenever showCursorSettings is true, including native-cursor recordings, but the preview highlight is rendered only through cursorOverlay and that overlay is explicitly hidden when native cursor data exists (VideoPlayback.tsx calls cursorOverlay.update(..., showCursor && !hasNativeCursorRecordingRef.current, ...)). In that common native-cursor case, users can edit highlight settings but see no preview effect, so they cannot reliably tune the feature before export.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/video-editor/VideoEditor.tsx (1)
419-465:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
webcamSizePresetincurrentProjectSnapshot(lowkey risky as-is).Line 419 builds the live snapshot without
webcamSizePreset, but Line 567 includes it during save. That mismatch can make unsaved-state checks stale when webcam size is the only change.nit: cleaner/fix diff
return createProjectSnapshot(currentProjectMedia, { wallpaper, shadowIntensity, showBlur, motionBlurAmount, borderRadius, padding, cropRegion, zoomRegions, trimRegions, speedRegions, annotationRegions, aspectRatio, webcamLayoutPreset, webcamMaskShape, + webcamSizePreset, webcamPosition, exportQuality, exportFormat, gifFrameRate, gifLoop, gifSizePreset, cursorHighlight, }); }, [ currentProjectMedia, wallpaper, shadowIntensity, showBlur, motionBlurAmount, borderRadius, padding, cropRegion, zoomRegions, trimRegions, speedRegions, annotationRegions, aspectRatio, webcamLayoutPreset, webcamMaskShape, + webcamSizePreset, webcamPosition, exportQuality, exportFormat, gifFrameRate, gifLoop, gifSizePreset, cursorHighlight, ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/VideoEditor.tsx` around lines 419 - 465, The live snapshot created by createProjectSnapshot is missing webcamSizePreset which causes unsaved-state mismatches when only webcam size changes; update the createProjectSnapshot call inside the memoized builder (the function that returns createProjectSnapshot(currentProjectMedia, { ... })) to include webcamSizePreset in the passed properties and add webcamSizePreset to the dependency array alongside currentProjectMedia, webcamLayoutPreset, webcamMaskShape, webcamPosition, etc., so the snapshot and the saved snapshot used on line ~567 are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 329-332: The cursor highlight controls can be hidden when
showCursorSettings is false but showCursorHighlightSettings is true because
hasCursorPanel is only computed from showCursorSettings; update the logic that
determines whether the cursor tab/panel is rendered (the hasCursorPanel
variable/condition in SettingsPanel.tsx) to include showCursorHighlightSettings
(and/or cursorHighlightSupportsClicks if gating by capability), e.g. compute
hasCursorPanel = showCursorSettings || showCursorHighlightSettings so the cursor
highlight settings become reachable even when the main cursor settings tab is
hidden.
In `@src/components/video-editor/videoPlayback/cursorRenderer.ts`:
- Around line 506-529: The highlight radius isn't scaled by the viewport like
the cursor is; update drawHighlight to accept a viewportScale parameter (pass
getCursorViewportScale(viewport) from the place where drawHighlight is called)
and compute radius as Math.max(1, (config.sizePx / 2) * viewportScale) so the
Pixi highlight matches cursor sizing; update the drawHighlight signature and all
its callers to pass the viewportScale accordingly and leave export code
untouched since it already applies pixelScale.
In `@src/i18n/locales/ar/settings.json`:
- Around line 203-212: The Arabic locale contains English text for the
cursor.highlight group (keys like "cursor.highlight.title",
"cursor.highlight.style", "cursor.highlight.dot", "cursor.highlight.ring",
"cursor.highlight.size", "cursor.highlight.opacity",
"cursor.highlight.onlyOnClicks", "cursor.highlight.color",
"cursor.highlight.offsetX", "cursor.highlight.offsetY"); replace these inline
English values with their Arabic translations or remove the entries to
intentionally fall back to the base locale, ensuring the UI strings for
cursor.highlight are fully localized in the Arabic settings.json.
In `@src/i18n/locales/es/settings.json`:
- Around line 203-212: The cursor.highlight keys are still in English; update
the Spanish locale values for cursor.highlight.title and its children
(cursor.highlight.style, .dot, .ring, .size, .opacity, .onlyOnClicks, .color,
.offsetX, .offsetY) to Spanish equivalents—for example: title->"Resaltado del
cursor", style->"Estilo", dot->"Punto", ring->"Anillo", size->"Tamaño",
opacity->"Opacidad", onlyOnClicks->"Solo al hacer clic", color->"Color",
offsetX->"Desplazamiento X", offsetY->"Desplazamiento Y"—replace the English
strings with these Spanish translations in the JSON entries.
In `@src/i18n/locales/fr/settings.json`:
- Around line 204-213: The French locale block for cursor.highlight is still in
English; update the values under "cursor.highlight" (keys: "title", "style",
"dot", "ring", "size", "opacity", "onlyOnClicks", "color", "offsetX", "offsetY")
to French equivalents—for example replace with: "Titre: Curseur — Mise en
évidence", "Style", "Point", "Anneau", "Taille", "Opacité", "Uniquement sur
clics", "Couleur", "Décalage X", "Décalage Y"—so all labels in the fr
settings.json are fully localized.
In `@src/i18n/locales/it/settings.json`:
- Around line 202-211: The cursor.highlight keys in the Italian locale (strings
named "title", "style", "dot", "ring", "size", "opacity", "onlyOnClicks",
"color", "offsetX", "offsetY" under the cursor.highlight object) are still
English; replace each English literal with the appropriate Italian translation
(e.g., translate "Cursor Highlight" -> "Evidenziazione cursore", "Style" ->
"Stile", "Dot" -> "Punto", "Ring" -> "Anello", "Size" -> "Dimensione", "Opacity"
-> "Opacità", "Only on clicks" -> "Solo sui clic", "Color" -> "Colore", "Offset
X" -> "Offset X" or "Offset X" localized if preferred, "Offset Y" -> "Offset
Y"/localized) so the it locale contains all Italian strings for
cursor.highlight.
In `@src/i18n/locales/ja-JP/settings.json`:
- Around line 203-212: The cursor.highlight labels in the ja-JP locale are still
English; update the values for the keys "title", "style", "dot", "ring", "size",
"opacity", "onlyOnClicks", "color", "offsetX", and "offsetY" in the existing
cursor.highlight block to Japanese (for example: "title" -> "カーソルのハイライト",
"style" -> "スタイル", "dot" -> "ドット", "ring" -> "リング", "size" -> "サイズ", "opacity"
-> "不透明度", "onlyOnClicks" -> "クリック時のみ", "color" -> "色", "offsetX" -> "Xオフセット",
"offsetY" -> "Yオフセット") so the UI is fully localized for ja-JP.
In `@src/i18n/locales/ko-KR/settings.json`:
- Around line 203-212: The Korean locale entries under cursor.highlight are
still in English; translate the values for the keys cursor.highlight.title,
cursor.highlight.style, cursor.highlight.dot, cursor.highlight.ring,
cursor.highlight.size, cursor.highlight.opacity, cursor.highlight.onlyOnClicks,
cursor.highlight.color, cursor.highlight.offsetX, and cursor.highlight.offsetY
into Korean so the settings panel is fully localized (replace the current
English strings with appropriate Korean translations).
In `@src/i18n/locales/ru/settings.json`:
- Around line 203-212: The Russian locale file contains English labels for the
cursor.highlight section (keys: "title", "style", "dot", "ring", "size",
"opacity", "onlyOnClicks", "color", "offsetX", "offsetY") causing mixed-language
UI; replace each English string value under the cursor.highlight block in
src/i18n/locales/ru/settings.json with the correct Russian translations for
those keys so the entire cursor.highlight section is localized (ensure key names
remain unchanged).
In `@src/i18n/locales/tr/settings.json`:
- Around line 203-212: The JSON object under the cursor.highlight keys contains
English strings that must be translated to Turkish; update the values for
"title", "style", "dot", "ring", "size", "opacity", "onlyOnClicks", "color",
"offsetX", and "offsetY" in the tr locale so they are proper Turkish
translations (e.g., translate "Cursor Highlight", "Style", "Dot", "Ring",
"Size", "Opacity", "Only on clicks", "Color", "Offset X", "Offset Y" to their
Turkish equivalents) while keeping the keys unchanged.
In `@src/i18n/locales/vi/settings.json`:
- Around line 203-212: The cursor highlight locale entries (keys like "title",
"style", "dot", "ring", "size", "opacity", "onlyOnClicks", "color", "offsetX",
"offsetY") are still in English; replace their string values with proper
Vietnamese translations so the vi locale is fully localized for the
cursor-highlight controls (update the values for "Cursor Highlight", "Style",
"Dot", "Ring", "Size", "Opacity", "Only on clicks", "Color", "Offset X", "Offset
Y" to Vietnamese equivalents).
In `@src/i18n/locales/zh-CN/settings.json`:
- Around line 203-212: The JSON entries for the cursor highlight UI are using
English values for the keys "title", "style", "dot", "ring", "size", "opacity",
"onlyOnClicks", "color", "offsetX", and "offsetY"; update those string values to
their Chinese translations so the zh-CN locale renders correctly (e.g., replace
"Cursor Highlight" and the other English labels with appropriate Chinese
equivalents) and keep the keys unchanged.
In `@src/i18n/locales/zh-TW/settings.json`:
- Around line 204-213: The JSON values for the cursor highlight section are
still in English; replace the English strings for the keys "title", "style",
"dot", "ring", "size", "opacity", "onlyOnClicks", "color", "offsetX", and
"offsetY" with Traditional Chinese equivalents (e.g., "游標高亮" for "Cursor
Highlight", "樣式" for "Style", "點" for "Dot", "環" for "Ring", "大小" for "Size",
"不透明度" for "Opacity", "僅在點擊時" for "Only on clicks", "顏色" for "Color", "X 偏移" for
"Offset X", "Y 偏移" for "Offset Y") so the zh-TW locale is fully translated.
In `@src/lib/exporter/frameRenderer.ts`:
- Around line 564-570: The current reverse-scan in frameRenderer (the for loop
over samples checking samples[i].timeMs <= timeMs) is O(n) per frame and must be
replaced with a logarithmic or linear-amortized approach; implement a
binary-search helper (e.g., findLatestSampleAtOrBefore(samples, timeMs)) that
returns the index of the newest sample whose timeMs <= timeMs and use that to
set cx/cy, or alternatively maintain a moving index (lastSampleIndex) that only
advances forward between frames, update it in the frame rendering function
(frameRenderer.renderFrame or the routine around the loop) and reuse it per
frame to avoid re-scanning from the end. Ensure the helper works with empty
arrays and boundary conditions (time before first sample -> handle
appropriately) and replace references to the old loop with a call to the new
helper or the moving-index logic.
In `@src/lib/exporter/videoExporter.ts`:
- Line 53: The fast-path export eligibility check currently ignores the new
cursorHighlight config, allowing exports to skip the FrameRenderer and drop
overlays; update the fast-path/eligibility guard used in videoExporter.ts to
treat any non-empty cursorHighlight as ineligible for the fast path (i.e., if
config.cursorHighlight is defined/true then disable fast-path), so exports will
always go through FrameRenderer when cursor highlights are configured and the
overlay is preserved.
---
Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 419-465: The live snapshot created by createProjectSnapshot is
missing webcamSizePreset which causes unsaved-state mismatches when only webcam
size changes; update the createProjectSnapshot call inside the memoized builder
(the function that returns createProjectSnapshot(currentProjectMedia, { ... }))
to include webcamSizePreset in the passed properties and add webcamSizePreset to
the dependency array alongside currentProjectMedia, webcamLayoutPreset,
webcamMaskShape, webcamPosition, etc., so the snapshot and the saved snapshot
used on line ~567 are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a954989-b391-4db4-b2ea-494134c39de8
📒 Files selected for processing (22)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.tssrc/components/video-editor/videoPlayback/cursorHighlight.tssrc/components/video-editor/videoPlayback/cursorRenderer.tssrc/hooks/useEditorHistory.tssrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/it/settings.jsonsrc/i18n/locales/ja-JP/settings.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/vi/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/settings.jsonsrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/videoExporter.ts
kaili-yang
added a commit
to kaili-yang/openscreen
that referenced
this pull request
May 22, 2026
- Video Editor: track webcamSizePreset in snapshot and fix cursor settings gating\n- Video Playback & Export: scale cursor highlight relative to player viewport size, use binary search for telemetry samples lookup, block exporter fast-path when highlight enabled\n- Localization: add translations for cursor.highlight object in all 11 non-English locales
kaili-yang
force-pushed
the
feat/cursor-highlight
branch
from
213f4b6 to
4c8c942
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-implements cursor highlight (ring/dot overlay at cursor position) following the original author's design intent. The feature was removed when uiohook-napi was dropped; now restored using the existing CGEventTap-based cursor telemetry pipeline on macOS and WGC on Windows.
Pull Request Template
Description
Motivation
Type of Change
Related Issue(s)
Screenshots / Video
Screenshot (if applicable):
Video (if applicable):
Regression Testing
✅ Pass
Summary by CodeRabbit
New Features
Documentation / Localization