feat: User-adjustable playback speed for slider play buttons (IMA-190) - #5
Merged
Merged
Conversation
Pin current playback behavior (both play buttons start their QTimer at 100 ms, looping step callbacks, iconify text fallback, shared stop helper) so the upcoming playback-speed refactor and feature land on a green baseline. Zero playback coverage existed before this. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Structure-only change: both play-button handlers were line-for-line twins. The helper mirrors the existing _stop_play_animation and RETURNS the (possibly newly created) timer so the thin wrappers can rebind their instance attribute. Behavior pinned by the characterization tests in the previous commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the hardcoded 10 fps animation interval with a QSpinBox speed control (1-10 fps, default 5) in a fixed right-aligned slot below the sliders, shared by both the time and FOV play buttons: - DEFAULT_PLAYBACK_FPS/PLAYBACK_FPS_MIN/MAX replace SLIDER_PLAY_INTERVAL_MS; _fps_to_interval_ms converts (0-clamped) - play handlers read the stored _playback_fps int - changing the spinbox mid-playback retimes every active timer live via QTimer.setInterval - control is visible whenever either slider row is, via _update_speed_control_visibility hooked into the three row-visibility sites (FOV helper + both live-acquisition T-row reveals) Default drops 10 -> 5 fps per Nick's report that slider playback was way too fast; 10 fps remains reachable via the control. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two items deferred from the IMA-190 review with full context: fps persistence across sessions (QSettings) and the pre-existing playback-tick-outruns-synchronous-loads issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
SLIDER_PLAY_INTERVAL_MS = 100) with a QSpinBox speed control: 1–10 fps, default 5, shared by the time and FOV play buttonsQTimer.setInterval)_toggle_play_animationextraction (deduplicates the twin play handlers) → featuretests/test_playback_speed.py: 35 tests in the house MagicMock pattern (no pytest-qt), incl. regression tests for the 10→5 fps default changeTest plan
pytest tests/— 271 passed locally (3 pre-existing local-env failures in test_push_api unrelated to this change; pass in CI)black<26 --checkclean on changed files (matches CI lint)Closes IMA-190.
🤖 Generated with Claude Code