Fix region capture, power ownership, click-source reporting, strict timing #24
Assignees
Tyler (Tyler Westbrook)
Clear assignees
Tyler (Tyler Westbrook)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Tyler/StepForge#24
Reference in New Issue
Block a user
Phase 2 of the improvement plan, PR 5. Mirrors GitHub PR Twest2/StepForge#8.
Verified: 230 unit tests; click selftest markers 3/3 + burst 8/8 deterministic; startup+workflow E2E pass. arm/debounce remain the pre-existing Linux failures.
Phase 2 of the improvement plan (PR 5 of the sequence). Several confirmed capture defects from the audit. Region capture: - regionCapture returned { ok, step } wrapping storeFrameAsStep's own { ok, step }, so the real step was at result.step.step — region selection and region auto-doc (which read result.step.stepId) broke. Return the storeFrameAsStep result directly. - pickRegion leaked the region:picked IPC listener (and the overlay/image refs) whenever the overlay was cancelled/closed rather than picked: cleanup only ran on a pick. Cleanup is now idempotent and runs on pick, close, load failure, and settle. The received rectangle is validated and clamped to the image (new overlayRectToImageRect handles negative-size drags and out-of-bounds selections) so image.crop can never read out of bounds. Power blocker ownership: - New single owner: CaptureService.syncPower() holds the blocker iff a session is actively recording, called on start/pause/resume/finish. A new session starts paused and no longer holds the blocker while idle; tray and second-instance pauses that previously bypassed main.js's stop closure now release it correctly. main.js provides the power policy (blocker + EcoQoS opt-out) via dependency injection. Explicit click-trigger source: - startEvdevWatcher never set clickWatcher, so state().clickCapture read false while evdev was actively capturing clicks. Replaced the boolean with an explicit clickSource (windows-hook | x11 | evdev-x11 | evdev-wayland | unavailable); clickCapture is now derived from it. evdev device-stream errors/closes now fall back via handleClickWatcherLoss instead of being swallowed. Strict click timing: - When no pre-click frame qualifies, strict mode previously fell through to a fresh (post-click) shot and stored it — contradicting the strict promise. It now skips with a capture:diagnostic instead. Non-strict (balanced) mode keeps the fresh-shot fallback. Existing tests that exercised the fallback now run in balanced mode; the strict test asserts the skip. Other: - pathToFileURL replaces file://${p} concatenation for step image URLs and export previews (correct for spaces, #, %, drive letters). - Best-effort click-queue drain on app shutdown (before-quit) so a burst just before quit is not lost; bounded so quit never hangs. Tests: region rect clamping/normalization, power-held-only-while-recording (incl. finish releases), click-source reporting incl. evdev, drain deadline. 230 unit tests pass. Click self-test: markers 3/3 (strict) and burst 8/8 deterministic across runs; the burst scenario runs in balanced mode because it tests the drain, not strict timing. (arm/debounce remain the pre-existing Linux capture failures untouched by this PR.) Co-Authored-By: Claude Fable 5 <[email protected]>Merged into main as
dd71cff(same-SHA merge to both forges; GitHub mirror PR Twest2/StepForge#8 MERGED, CI green ubuntu+windows). Closing.Pull request closed