Waveform mark fixes by daschuer · Pull Request #15968 · mixxxdj/mixxx

daschuer · 2026-02-10T22:39:52Z

This fixed [Skin],show_intro_outro_cues for scrolling waveforms #15967
The issue was that init() was used for connecting controls before the waveform marks have been found.

This also removes a double marks updates. The "position" and "end_positions" were directly connected in case of WOverview and it is updated by Cue::updated(). In case of scrolling waveform this connection was broken anyway.

It is also involved in #15954 so the fix here might have an positive effecrt

These were the call stacks showing the redundant updates.:

WaveformMarkSet::update() at waveformmarkset.cpp:124 0x55555662f820	
WOverview::updateCues() at woverview.cpp:508 0x555556733997	
WOverview::onMarkChanged() at woverview.cpp:423 0x555556737262	
WOverview::receiveCuesUpdated() at woverview.cpp:514 0x555556737262	
QtPrivate::QSlotObjectBase::call() at qobjectdefs_impl.h:375 0x7ffff27ac023	
doActivate<false>() at qobject.cpp:3.912 0x7ffff27ac023	
QtPrivate::QSlotObjectBase::call() at qobjectdefs_impl.h:375 0x7ffff27ac023	
doActivate<false>() at qobject.cpp:3.912 0x7ffff27ac023	
QMetaObject::activate() at qobject.cpp:3.972 0x7ffff27a6ac7	
Cue::updated() at moc_cue.cpp:134 0x55555651e82a	
Cue::setStartPosition() at cue.cpp:166 0x55555651e82a	
CueControl::hotcuePositionChanged() at cuecontrol.cpp:1.407 0x555555edfec6

WaveformMarkSet::update() at waveformmarkset.cpp:124 0x55555662f820	
WaveformRenderMarkBase::updateMarks() at waveformrendermarkbase.cpp:98 0x555556647ab2	
WaveformRenderMarkBase::updateMarksFromCues() at waveformrendermarkbase.cpp:93 0x555556649c38	
QtPrivate::QSlotObjectBase::call() at qobjectdefs_impl.h:375 0x7ffff27ac023	
doActivate<false>() at qobject.cpp:3.912 0x7ffff27ac023	
QtPrivate::QSlotObjectBase::call() at qobjectdefs_impl.h:375 0x7ffff27ac023	
doActivate<false>() at qobject.cpp:3.912 0x7ffff27ac023	
QMetaObject::activate() at qobject.cpp:3.972 0x7ffff27a6ac7	
Cue::updated() at moc_cue.cpp:134 0x55555651e82a	
Cue::setStartPosition() at cue.cpp:166 0x55555651e82a	
CueControl::hotcuePositionChanged() at cuecontrol.cpp:1.407 0x555555edfec6

WaveformMarkSet::update() at waveformmarkset.cpp:124 0x55555662f820	
WOverview::updateCues() at woverview.cpp:508 0x555556733997	
WOverview::onMarkChanged() at woverview.cpp:423 0x555556736cd2

…endered->init()

…ve an update via Cue::updated()

…eceive an update via Cue::updated()

…_outro_cues mixxxdj#15967

acolombier

Thanks for looking into this.
I haven't yet completely understood how removing the connectSample*Changed impact the waveform rendering. I need to dig deeper to understand how the scene invalidation would work, especially of the context of the rendergraph with the scenegraph (though not important for 2.6, but important to know if we pull this fix forward)

daschuer · 2026-02-11T14:19:02Z

The waveform marks have the "....position" co. If you change position let's say for '[Channel1],hotcue_1_position' form the developers window, you see the three backtracks from above.

This PR remove the redundant last one, which is from the connection to WOverview::onMarkChanged().
Originally there was also the similar connection in WaveformRenderMarkBase(), but this does not connect because the connect is called before the marks are created. This wrong order cause also the missing connection to [Skin],show_intro_outro_cues.

This is all is unrelated to the rendering itself. It is just a question about the way position changes are causing a visual update.

ywwg · 2026-02-11T14:37:25Z

Code looks great and uncontroversial, I'll do some quick hand QA to make sure there aren't any immediate issues

ywwg

looks great, good cleanup

ywwg · 2026-02-11T15:08:48Z

Let's just make sure Antoine is ok with the changse as well

acolombier

Unfortunately, this appears to be breaking marker in QML entirely

Here is how it use to work before in 2.6

Here is with this PR

daschuer · 2026-02-11T22:25:30Z

Did you merge this PR to main?

acolombier · 2026-02-11T22:29:22Z

No, compared with 2.6

daschuer · 2026-02-12T07:04:02Z

I cannot confirm. I have tested this branch with option --qml and I see:

Can you think of any reason it is broken for you?

ywwg · 2026-02-12T22:05:47Z

I am seeing this too, if I open mixxx --qml and load a track with marks, the marks don't show up the first time. they do show up on subsequent track loads (including the same track). I suspect the connections are not being made in the right order

daschuer · 2026-02-13T07:37:57Z

Ah I see, only the very first track is affected.

…f hot cue markers is not supported.

…f showing none or old hot cues.

daschuer · 2026-02-13T13:08:58Z

I have found the issue. It was related to using an half baked track object in case of QML.

I think we have a conceptual issue here. In case of QML rendering is done by the QSGRenderThread concurrently accessing the main thread resources without synchronisation. Not sure if this is actually an issue, but we should consider a clear separation to not suffer random crashes with the QML GUI. Currently the variable "m_updateImagesImmediately" is used to control the differenc between QML Multithreading and Widgets single thread.

ywwg · 2026-02-13T16:06:12Z

would it help to have a short meeting to discuss the issue? it sounds like we have uncovered a design flaw that we will need to reconcile to prevent issues like this in the future.

ywwg · 2026-02-13T16:06:39Z

we can still merge this cleanup once I confirm the cue issue has been fixed

ywwg · 2026-02-13T16:07:21Z

can confirm, marks appear on first load now

acolombier · 2026-02-13T16:49:06Z

Note that QML should manage this synchronisation implicitly AFAIU, the only case we need to manage it explicitelt is for multi threaded rendering engine.

I will have a look at this PR, but remove the change request in case I take too long.

Removing as Owen as confirmed the issue is fixed!

acolombier · 2026-02-13T22:24:05Z

    Q_UNUSED(pOldTrack); // only used in DEBUG_ASSERT
    DEBUG_ASSERT(getTrackInfo() == pOldTrack);
-    setCurrentTrack(pNewTrack);
+    setCurrentTrack({});


Could you clarify why this is needed? As I understand, setCurrentTrack will still be called in QmlWaveformDisplay::slotTrackLoaded, which was previously discarded as the current track would already be set. Was that causing trouble? Might be good to capture the explanation in a comment so we don't accidentally add this back in the future.

During loading a new Track all data is populated one by one. We want to remove the old track during that time and pass the new one once it is finished and ready for use in slotTrackLoaded() above.
Maybe this helps also to fix @ywwg race condition.

Thanks for the explanation.
From my discovery, this won't help with the race condition.

acolombier · 2026-02-13T22:52:37Z

Just to confirm my message above, I can confirm that Qt guaranties synchronisation between the GUI thread and QSGRenderThread

In this implementation, the render thread is never blocked and the GUI thread will initiate a polishAndSync which will block and wait for the render thread to pick it up and release the block only after the render thread is done syncing.

acolombier · 2026-02-17T00:20:02Z

I can confirm the issue with marker not displaying is fixed. Thank you.

ronso0 · 2026-03-01T16:37:30Z

Unfortunately this introduces some update regressions in the overview.
Hotcues:

set hotcue1 -> no update in overview
set hotcue2 -> hotcue1 pops up in overview

Loop cues:

updated/shown after seek in the overview (repaint?)

Same delay after clearing hotcues: they're removed only after forced repaint (hover overview)

mixxx-overview-hotcues-delayed-update.webm

daschuer mentioned this pull request Feb 10, 2026

[Skin],show_intro_outro_cues does not work for scrolling waveforms #15967

Open

github-actions Bot added ui waveform labels Feb 10, 2026

daschuer added 6 commits February 11, 2026 07:29

Use VERIFY_OR_DEBUG_ASSERT() in WaveformWidgetRenderer::init() for pR…

4f8e71e

…endered->init()

Remove redundant connection of cue position control. We already recei…

1adf980

…ve an update via Cue::updated()

Remove redundant connection of cue end position control. We already r…

831aff0

…eceive an update via Cue::updated()

WaeformRedndere: Avoid plain new() by using std::unique_ptr

d127514

Initalize uninitalized variables

2d5d4e6

Move connectVisibleChanged() to setup() this fixes the CO: show_intro…

de51b2d

…_outro_cues mixxxdj#15967

daschuer force-pushed the gh15954 branch from 600ca31 to de51b2d Compare February 11, 2026 06:29

acolombier reviewed Feb 11, 2026

View reviewed changes

ywwg approved these changes Feb 11, 2026

View reviewed changes

ywwg requested a review from acolombier February 11, 2026 15:08

acolombier previously requested changes Feb 11, 2026

View reviewed changes

daschuer added 2 commits February 13, 2026 13:59

Remove unused clearMarks(). Dynamically changing the maximum number o…

0f14054

…f hot cue markers is not supported.

Don't make an unfinished loaded track current. This fixes the issue o…

fc248ef

…f showing none or old hot cues.

github-actions Bot added the qml label Feb 13, 2026

ywwg requested a review from acolombier February 13, 2026 16:07

acolombier reviewed Feb 13, 2026

View reviewed changes

ronso0 added this to the 2.6.0 milestone Feb 15, 2026

acolombier merged commit 5e9cffa into mixxxdj:2.6 Feb 17, 2026
14 checks passed

ronso0 mentioned this pull request Mar 1, 2026

Overview cue upate regression after #15968 #16075

Closed

Uh oh!

Uh oh!

Conversation

daschuer commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acolombier left a comment

Choose a reason for hiding this comment

Uh oh!

daschuer commented Feb 11, 2026

Uh oh!

ywwg commented Feb 11, 2026

Uh oh!

ywwg left a comment

Choose a reason for hiding this comment

Uh oh!

ywwg commented Feb 11, 2026

Uh oh!

acolombier left a comment

Choose a reason for hiding this comment

Uh oh!

daschuer commented Feb 11, 2026

Uh oh!

acolombier commented Feb 11, 2026

Uh oh!

daschuer commented Feb 12, 2026

Uh oh!

ywwg commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented Feb 13, 2026

Uh oh!

daschuer commented Feb 13, 2026

Uh oh!

ywwg commented Feb 13, 2026

Uh oh!

ywwg commented Feb 13, 2026

Uh oh!

ywwg commented Feb 13, 2026

Uh oh!

acolombier commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acolombier Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

daschuer Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

acolombier Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

acolombier commented Feb 13, 2026

Uh oh!

acolombier commented Feb 17, 2026

Uh oh!

Uh oh!

ronso0 commented Mar 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

daschuer commented Feb 10, 2026 •

edited

Loading

ywwg commented Feb 12, 2026 •

edited

Loading

acolombier commented Feb 13, 2026 •

edited

Loading