Fix OCR capturing VoiceInk status overlay instead of frontmost app window#429
Conversation
…ndow The screen capture service was selecting the first layer-0 window, which during recording was VoiceInk's own status indicator overlay. This caused OCR to always return 'No text detected' since the overlay has no readable content. Changes: - Filter out windows owned by VoiceInk's process - Prioritize windows belonging to NSWorkspace.frontmostApplication - Filter out tiny windows (<120x120) to avoid tooltips/overlays - Move CGWindowListCopyWindowInfo off main thread for better UI responsiveness - Refactor WindowCandidate struct to class scope
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="VoiceInk/Services/ScreenCaptureService.swift">
<violation number="1" location="VoiceInk/Services/ScreenCaptureService.swift:68">
P2: Rule violated: **Backward compatibility**
Removing the minimum window size check (120x120) breaks backward compatibility and contradicts the PR description which claims to exclude tiny windows. Previously, small windows like tooltips, notification badges, or tiny status indicators were filtered out. Now they can be selected for OCR capture, potentially degrading OCR results.
Consider restoring the size filter to maintain the previous behavior while still fixing the original bug.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| func isEligible(_ candidate: WindowCandidate) -> Bool { | ||
| guard candidate.layer == 0 else { return false } | ||
| guard candidate.ownerPID != currentPID else { return false } | ||
| return true |
There was a problem hiding this comment.
P2: Rule violated: Backward compatibility
Removing the minimum window size check (120x120) breaks backward compatibility and contradicts the PR description which claims to exclude tiny windows. Previously, small windows like tooltips, notification badges, or tiny status indicators were filtered out. Now they can be selected for OCR capture, potentially degrading OCR results.
Consider restoring the size filter to maintain the previous behavior while still fixing the original bug.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Services/ScreenCaptureService.swift, line 68:
<comment>Removing the minimum window size check (120x120) breaks backward compatibility and contradicts the PR description which claims to exclude tiny windows. Previously, small windows like tooltips, notification badges, or tiny status indicators were filtered out. Now they can be selected for OCR capture, potentially degrading OCR results.
Consider restoring the size filter to maintain the previous behavior while still fixing the original bug.</comment>
<file context>
@@ -65,7 +65,7 @@ class ScreenCaptureService: ObservableObject {
guard candidate.layer == 0 else { return false }
guard candidate.ownerPID != currentPID else { return false }
- return candidate.bounds.width >= 120 && candidate.bounds.height >= 120
+ return true
}
</file context>
| return true | |
| return candidate.bounds.width >= 120 && candidate.bounds.height >= 120 |
There was a problem hiding this comment.
This is a misleading comment. It refers to a code change from my first commit. Not sure why the AI complains about backwards compatibility to my own prior commit in the PR.
Anyways, I first tried to filter out all small windows from the candidate list for OCR. However, in my tests with small popups e.g. from this app https://dropoverapp.com/ the implemented changes were sufficient. There may be cases where it is required to filter out small overlay windows but I'd rather not add the change unless it's confirmed that we need this. Also 120x120px was kinda arbitrary.
…ndow The screen capture service was selecting the first layer-0 window, which during recording was VoiceInk's own status indicator overlay. This caused OCR to always return 'No text detected' since the overlay has no readable content. Changes: - Filter out windows owned by VoiceInk's process - Prioritize windows belonging to NSWorkspace.frontmostApplication - Refactor WindowCandidate struct to class scope
Fix OCR capturing VoiceInk status overlay instead of frontmost app window
2 participants
Summary
Fixes #424
Problem
During recording, the screen capture service was selecting the first layer-0 window, which was VoiceInk's own status indicator overlay. Since this overlay contains no readable text, OCR always returned "No text detected via OCR."
Solution
ProcessInfo.processInfo.processIdentifier)Validation
I'm running the new version locally and OCR works now.
Summary by cubic
Prevent OCR from capturing the VoiceInk status overlay by filtering our own windows and selecting the frontmost app’s window instead. Restores context-aware OCR so it reads actual on-screen text. Fixes #424.
Bug Fixes
Refactors
Written for commit d25ae52. Summary will update automatically on new commits.