From 7bd7e7a2dbf6e896cb971b414c72c5f7050cb3c1 Mon Sep 17 00:00:00 2001 From: Bernardo Malheiro Date: Sat, 21 Mar 2026 19:02:46 +0000 Subject: [PATCH 1/2] Fix #8227: prevent segfault when opening SF2 Player GUI during load Root cause: - clicking the TrackLabelButton while a soundfont was still loading triggered InstrumentTrackWindow::adjustTabSize() with a null widget pointer, causing a segmentation fault Changes: - disable TrackLabelButton in constructor when no instrument is loaded - re-enable TrackLabelButton via onInstrumentChanged() slot connected to InstrumentTrack::instrumentChanged() signal - add null guard in adjustTabSize() as an additional safety measure --- include/InstrumentTrackView.h | 3 +++ src/gui/instrument/InstrumentTrackWindow.cpp | 5 +++++ src/gui/tracks/InstrumentTrackView.cpp | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/include/InstrumentTrackView.h b/include/InstrumentTrackView.h index 0c645926ca3..79996491305 100644 --- a/include/InstrumentTrackView.h +++ b/include/InstrumentTrackView.h @@ -79,6 +79,9 @@ class InstrumentTrackView : public TrackView private slots: + //! Slot to update the state of the track label button when the instrument changes. + //! An instrument may be loaded or removed, so the button should be enabled or disabled accordingly. + void onInstrumentChanged(); void toggleInstrumentWindow( bool _on ); void toggleMidiCCRack(); void activityIndicatorPressed(); diff --git a/src/gui/instrument/InstrumentTrackWindow.cpp b/src/gui/instrument/InstrumentTrackWindow.cpp index 71d2eadd8a2..52871abfe0b 100644 --- a/src/gui/instrument/InstrumentTrackWindow.cpp +++ b/src/gui/instrument/InstrumentTrackWindow.cpp @@ -721,6 +721,11 @@ void InstrumentTrackWindow::viewPrevInstrument() void InstrumentTrackWindow::adjustTabSize(QWidget *w) { + // Check if the widget is null, as not all tabs are always present (e.g. the plugin tab is only present if an instrument is loaded) + if( w == nullptr ) + { + return; + } // "-1" : // in "TabWidget::addTab", under "Position tab's window", the widget is // moved up by 1 pixel diff --git a/src/gui/tracks/InstrumentTrackView.cpp b/src/gui/tracks/InstrumentTrackView.cpp index 782b4e6a664..b3be94019b1 100644 --- a/src/gui/tracks/InstrumentTrackView.cpp +++ b/src/gui/tracks/InstrumentTrackView.cpp @@ -71,6 +71,11 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV m_tlb->setIcon(determinePixmap(_it)); m_tlb->show(); + // Check if an instrument has been loaded, if not disable the track label button to prevent opening an empty instrument window. + if( !_it->m_instrument ) { + m_tlb->setEnabled( false ); + } + connect( m_tlb, SIGNAL(toggled(bool)), this, SLOT(toggleInstrumentWindow(bool))); @@ -80,6 +85,10 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV connect(ConfigManager::inst(), SIGNAL(valueChanged(QString,QString,QString)), this, SLOT(handleConfigChange(QString,QString,QString))); + // update the state of the track label button when the instrument changes, as an instrument may be loaded or removed. + connect( _it, SIGNAL(instrumentChanged()), + this, SLOT(onInstrumentChanged())); + m_mixerChannelNumber = new MixerChannelLcdSpinBox(2, getTrackSettingsWidget(), tr("Mixer channel"), this); m_mixerChannelNumber->show(); @@ -300,6 +309,15 @@ void InstrumentTrackView::dropEvent( QDropEvent * _de ) +void InstrumentTrackView::onInstrumentChanged() +{ + // Check if an instrument has been loaded, if not disable the track label button to prevent opening an empty instrument window. + m_tlb->setEnabled( model()->m_instrument != nullptr ); +} + + + + void InstrumentTrackView::toggleInstrumentWindow( bool _on ) { if (_on && ConfigManager::inst()->value("ui", "oneinstrumenttrackwindow").toInt()) From a9f8abb1b58bbb5fc17e0d3457256e9d4fe8c514 Mon Sep 17 00:00:00 2001 From: Bernardo Malheiro Date: Tue, 24 Mar 2026 16:17:22 +0000 Subject: [PATCH 2/2] Address reviewer feedback on SF2 Player GUI fix - Refine button state logic: Updated the implementation to follow the specific architectural pattern suggested during review. - Code style adjustments: Cleaned up spacing to better match the project's guidelines. - Removed null checks that were identified as unnecessary by the maintainers. --- include/InstrumentTrackView.h | 2 -- src/gui/instrument/InstrumentTrackWindow.cpp | 5 ----- src/gui/tracks/InstrumentTrackView.cpp | 13 ++++--------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/include/InstrumentTrackView.h b/include/InstrumentTrackView.h index 79996491305..f7f270cea3a 100644 --- a/include/InstrumentTrackView.h +++ b/include/InstrumentTrackView.h @@ -79,8 +79,6 @@ class InstrumentTrackView : public TrackView private slots: - //! Slot to update the state of the track label button when the instrument changes. - //! An instrument may be loaded or removed, so the button should be enabled or disabled accordingly. void onInstrumentChanged(); void toggleInstrumentWindow( bool _on ); void toggleMidiCCRack(); diff --git a/src/gui/instrument/InstrumentTrackWindow.cpp b/src/gui/instrument/InstrumentTrackWindow.cpp index 52871abfe0b..71d2eadd8a2 100644 --- a/src/gui/instrument/InstrumentTrackWindow.cpp +++ b/src/gui/instrument/InstrumentTrackWindow.cpp @@ -721,11 +721,6 @@ void InstrumentTrackWindow::viewPrevInstrument() void InstrumentTrackWindow::adjustTabSize(QWidget *w) { - // Check if the widget is null, as not all tabs are always present (e.g. the plugin tab is only present if an instrument is loaded) - if( w == nullptr ) - { - return; - } // "-1" : // in "TabWidget::addTab", under "Position tab's window", the widget is // moved up by 1 pixel diff --git a/src/gui/tracks/InstrumentTrackView.cpp b/src/gui/tracks/InstrumentTrackView.cpp index b3be94019b1..7fca5ef9a59 100644 --- a/src/gui/tracks/InstrumentTrackView.cpp +++ b/src/gui/tracks/InstrumentTrackView.cpp @@ -71,11 +71,6 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV m_tlb->setIcon(determinePixmap(_it)); m_tlb->show(); - // Check if an instrument has been loaded, if not disable the track label button to prevent opening an empty instrument window. - if( !_it->m_instrument ) { - m_tlb->setEnabled( false ); - } - connect( m_tlb, SIGNAL(toggled(bool)), this, SLOT(toggleInstrumentWindow(bool))); @@ -85,9 +80,7 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV connect(ConfigManager::inst(), SIGNAL(valueChanged(QString,QString,QString)), this, SLOT(handleConfigChange(QString,QString,QString))); - // update the state of the track label button when the instrument changes, as an instrument may be loaded or removed. - connect( _it, SIGNAL(instrumentChanged()), - this, SLOT(onInstrumentChanged())); + connect(_it, &InstrumentTrack::instrumentChanged, this, &InstrumentTrackView::onInstrumentChanged); m_mixerChannelNumber = new MixerChannelLcdSpinBox(2, getTrackSettingsWidget(), tr("Mixer channel"), this); m_mixerChannelNumber->show(); @@ -176,6 +169,8 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV m_activityIndicator, SLOT(noteEnd())); setModel( _it ); + + onInstrumentChanged(); } @@ -312,7 +307,7 @@ void InstrumentTrackView::dropEvent( QDropEvent * _de ) void InstrumentTrackView::onInstrumentChanged() { // Check if an instrument has been loaded, if not disable the track label button to prevent opening an empty instrument window. - m_tlb->setEnabled( model()->m_instrument != nullptr ); + m_tlb->setEnabled(model()->m_instrument != nullptr); }