From 04ecf733951d2aba5bc8dad08c50f189979c8737 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Wed, 13 Mar 2024 20:00:58 +0100 Subject: [PATCH] Fix missing icons for some instruments (#7132) Revert some of the changes made in commit 88e0e94dcdb. The underlying idea was that the `InstrumentTrackView` should be responsible for assigning the icon that's shown by its `TrackLabelButton`. However, this does not work because in some cases the `InstrumentTrack` that's passed into `InstrumentTrackView::determinePixmap` does not have an `Instrument` assigned. This in turn seems to be caused due to initalizations that are running in parallel in different threads. Here are the steps to reproduce the threading problem (line numbers refer to commit 360254f): 1. Set a break point in line 1054 of `InstrumentTrack`, i.e. the line in `InstrumentTrack::loadInstrument` where `m_instrument` is being assigned to. 2. Set a break point in `InstrumentTrackView::determinePixmap`, e.g. inside of the first if statement. 3. Drop an instance of "Sf2 Player" onto the Song Editor. 4. The first break point in `InstrumentTrack` is hit in a thread called "lmms::Instrumen" (shown like that in my debugger). Try to step over it. 5. The second break point in `InstrumentTrackView` now gets hit before the code is stepped over. This time we are in the thread called "lmms". I guess this is the GUI main thread. 6. Continue execution. If you now switch to the application then the icon is shown. I guess the debugger is halted long enough in the main thread so that the InstrumentTrack gets an instrument assigned in another thread. If you delete/disable the break point in `InstrumentTrack::determinePixmap` and follow the coarse steps above then the icon is not shown because the track has no instrument. The current fix still delegates to the `InstrumentTrackView` to determine the pixmap in hopes that one day there will be a better solution where the parent component can be fully responsible for its child component. Fixes #7116. --- include/InstrumentTrackView.h | 2 ++ include/TrackLabelButton.h | 1 + src/gui/tracks/InstrumentTrackView.cpp | 5 +++++ src/gui/tracks/TrackLabelButton.cpp | 11 +++++++++++ 4 files changed, 19 insertions(+) diff --git a/include/InstrumentTrackView.h b/include/InstrumentTrackView.h index c7d524b36..cfde89bde 100644 --- a/include/InstrumentTrackView.h +++ b/include/InstrumentTrackView.h @@ -71,6 +71,8 @@ public: // Create a menu for assigning/creating channels for this track QMenu * createMixerMenu( QString title, QString newMixerLabel ) override; + QPixmap determinePixmap(); + protected: void modelChanged() override; diff --git a/include/TrackLabelButton.h b/include/TrackLabelButton.h index e19fc6be9..1d3620d12 100644 --- a/include/TrackLabelButton.h +++ b/include/TrackLabelButton.h @@ -55,6 +55,7 @@ protected: void mousePressEvent( QMouseEvent * _me ) override; void mouseDoubleClickEvent( QMouseEvent * _me ) override; void mouseReleaseEvent( QMouseEvent * _me ) override; + void paintEvent(QPaintEvent* pe) override; void resizeEvent( QResizeEvent * _re ) override; private: diff --git a/src/gui/tracks/InstrumentTrackView.cpp b/src/gui/tracks/InstrumentTrackView.cpp index 788991ed0..c812999fd 100644 --- a/src/gui/tracks/InstrumentTrackView.cpp +++ b/src/gui/tracks/InstrumentTrackView.cpp @@ -397,6 +397,11 @@ QMenu * InstrumentTrackView::createMixerMenu(QString title, QString newMixerLabe return mixerMenu; } +QPixmap InstrumentTrackView::determinePixmap() +{ + return determinePixmap(dynamic_cast(getTrack())); +} + QPixmap InstrumentTrackView::determinePixmap(InstrumentTrack* instrumentTrack) { diff --git a/src/gui/tracks/TrackLabelButton.cpp b/src/gui/tracks/TrackLabelButton.cpp index 087edba3d..c164b780e 100644 --- a/src/gui/tracks/TrackLabelButton.cpp +++ b/src/gui/tracks/TrackLabelButton.cpp @@ -30,6 +30,7 @@ #include "ConfigManager.h" #include "embed.h" +#include "InstrumentTrackView.h" #include "RenameDialog.h" #include "TrackRenameLineEdit.h" #include "TrackView.h" @@ -178,6 +179,16 @@ void TrackLabelButton::mouseReleaseEvent( QMouseEvent *_me ) } +void TrackLabelButton::paintEvent(QPaintEvent* pe) +{ + InstrumentTrackView* instrumentTrackView = dynamic_cast(m_trackView); + if (instrumentTrackView) + { + setIcon(instrumentTrackView->determinePixmap()); + } + + QToolButton::paintEvent(pe); +} void TrackLabelButton::resizeEvent(QResizeEvent *_re)