Fix Show Manager edge cases around timing and previews#2025
Open
cmuellner wants to merge 5 commits into
Open
Conversation
Origin: reported in PR mcallegari#1955. Reproduce in either the v4 or QML Show Manager by adding a timed item to a track, then placing a second item on the same track with its start time exactly equal to the first item's end time. The old inclusive end checks treated the shared endpoint as overlap and rejected the item. Use half-open time ranges, `[start, end)`, in both Show Managers so items only overlap when they share actual time. No automated regression test exists for this Show Manager GUI path. Signed-off-by: Christoph Müllner <christophm30@gmail.com>
Origin: reported in PR mcallegari#1955. Reproduce in the v4 Show Manager by moving the play cursor to a visible time on the timeline, then changing the time scale. The cursor was recomputed from its logical time, but the scale-change path added two pixels that normal cursor movement does not use. Use the same coordinate convention as moveCursor() when recomputing the cursor position after a time-scale change. No automated regression test exists for this Show Manager GUI path. Signed-off-by: Christoph Müllner <christophm30@gmail.com>
Origin: reported in PR mcallegari#1955. Reproduce by painting a v4 Show item for a zero-duration EFX or RGB Matrix function, or by painting a Sequence whose last step has a long fade-out marker. The loop marker code divided by the underlying function duration, and the sequence fade-out line could extend past the item width. In the QML Show Manager, Audio and Video preview data also emitted fade markers for zero-speed fades, which produced visible edge markers. Skip loop markers when the underlying duration is zero, clamp sequence fade-out markers to the item width, and only emit QML audio/video fade markers when the fade speed is non-zero. No automated regression test exists for this Show Manager GUI path. Signed-off-by: Christoph Müllner <christophm30@gmail.com>
Origin: found while reviewing the PR mcallegari#1955 bug list; not reported in PR mcallegari#1955. Reproduce in the v4 Show Manager by adding an Audio item, switching the waveform preview mode while preview generation is active, and repainting the timeline at the same time. The paint path read m_preview while the preview thread could delete and replace the same pointer. Protect preview replacement and paint access with a mutex. The paint path copies the current pixmap while holding the lock, then scales and draws the local copy after releasing it. No automated regression test exists for this GUI threading path. Signed-off-by: Christoph Müllner <christophm30@gmail.com>
Origin: independent finding while reviewing the v4 Show Manager audio preview lifetime. Reproduce in the v4 Show Manager by adding an Audio item with waveform preview enabled, switching preview modes to start preview generation, then deleting the item or reloading the show while generation is still active. PreviewThread kept a raw AudioItem pointer and could continue dereferencing it after the item was destroyed. Track running preview threads on AudioItem, request interruption for obsolete preview work, and wait for any remaining threads before the item and preview pixmap are destroyed. No automated regression test exists for this GUI lifetime path. Signed-off-by: Christoph Müllner <christophm30@gmail.com>
|
coverage: 35.004% (-0.03%) from 35.037% — cmuellner:pr-1955-fixes-p2 into mcallegari:master |
Owner
|
I take this is AI generated code, but are you actually reproducing each issue and testing if the fix resolves it? I'm not against AI code contributions as long as they are supervised and tested by humans |
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
3 participants
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.
Description
Summary of Changes:
This PR fixes Show Manager reliability issues found while reviewing stale
PR #1955.
The changes allow adjacent timeline items, keep the cursor stable after
time-scale changes, avoid invalid preview marker calculations, and
synchronize audio waveform preview access between painting and preview
generation, and stop preview threads before Audio items are destroyed.
Normal timeline editing and preview rendering behavior is unchanged for
valid items.
Commit Guide
Testing
Automated Coverage:
There is no dedicated Show Manager unit test target in the current tree.
Manual Testing Gaps
Reviewers can smoke-test the affected paths by placing adjacent items on
the same track, changing the v4 timeline scale with a visible cursor,
opening items with zero-duration or long fade markers, and deleting or
reloading an Audio item while waveform preview generation is active.
Additional Notes
The fixes are grouped because they all affect Show Manager timeline or
preview behavior. They are still kept as separate commits for review.
Each commit body contains the full rationale and reproduction details.
Checklist
{on a new line for functions and class definitions.