Skip to content

Commit

Permalink
Move icon determination into TrackLabelButton again
Browse files Browse the repository at this point in the history
Fully undo the changes made in commit 88e0e94 because the intermediate revert made in commit 04ecf73 seems to have led to a performance problem due to the icon being set over and over again in `TrackLabelButton::paintEvent`.

The original intention of the changes made in pull request LMMS#7114 was to remove the paining code that dynamically determines the icon over and over again. Ideally the icon that is used by an instrument should be somewhat of a "static" property that should be known very early on when an instrument view is created. There should not be any need to dynamically resolve the icon over and over, especially not in a button class very far down in the widget hierarchy. However, due to technical reasons this is not the case in the current code. See pull request LMMS#7132 for more details.
  • Loading branch information
michaelgregorius committed Apr 16, 2024
1 parent d2c2a80 commit fe5b532
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
2 changes: 0 additions & 2 deletions include/InstrumentTrackView.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ class InstrumentTrackView : public TrackView
// Create a menu for assigning/creating channels for this track
QMenu * createMixerMenu( QString title, QString newMixerLabel ) override;

QPixmap determinePixmap();


protected:
void modelChanged() override;
Expand Down
7 changes: 0 additions & 7 deletions src/gui/tracks/InstrumentTrackView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
#include "MixerView.h"
#include "GuiApplication.h"
#include "Instrument.h"
#include "InstrumentTrack.h"
#include "InstrumentTrackWindow.h"
#include "MainWindow.h"
#include "MidiClient.h"
Expand Down Expand Up @@ -397,12 +396,6 @@ QMenu * InstrumentTrackView::createMixerMenu(QString title, QString newMixerLabe
return mixerMenu;
}

QPixmap InstrumentTrackView::determinePixmap()
{
return determinePixmap(dynamic_cast<InstrumentTrack*>(getTrack()));
}


QPixmap InstrumentTrackView::determinePixmap(InstrumentTrack* instrumentTrack)
{
if (instrumentTrack)
Expand Down
27 changes: 22 additions & 5 deletions src/gui/tracks/TrackLabelButton.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include "ConfigManager.h"
#include "embed.h"
#include "InstrumentTrackView.h"
#include "Instrument.h"
#include "InstrumentTrack.h"
#include "RenameDialog.h"
#include "TrackRenameLineEdit.h"
#include "TrackView.h"
Expand Down Expand Up @@ -181,13 +183,28 @@ void TrackLabelButton::mouseReleaseEvent( QMouseEvent *_me )

void TrackLabelButton::paintEvent(QPaintEvent* pe)
{
InstrumentTrackView* instrumentTrackView = dynamic_cast<InstrumentTrackView*>(m_trackView);
if (instrumentTrackView)
if (m_trackView->getTrack()->type() == Track::Type::Instrument)
{
setIcon(instrumentTrackView->determinePixmap());
auto it = dynamic_cast<InstrumentTrack*>(m_trackView->getTrack());
const PixmapLoader * pl;
auto get_logo = [](InstrumentTrack* it) -> const PixmapLoader*
{
return it->instrument()->key().isValid()
? it->instrument()->key().logo()
: it->instrument()->descriptor()->logo;
};
if( it && it->instrument() &&
it->instrument()->descriptor() &&
( pl = get_logo(it) ) )
{
if( pl->pixmapName() != m_iconName )
{
m_iconName = pl->pixmapName();
setIcon( pl->pixmap() );
}
}
}

QToolButton::paintEvent(pe);
QToolButton::paintEvent( pe );
}


Expand Down

0 comments on commit fe5b532

Please sign in to comment.