From 88e0e94dcdb3503d8b4a9dbcd2e10c871b1f3cba Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 24 Dec 2023 22:49:39 +0100 Subject: [PATCH 1/8] Simplify TrackLabelButton Remove the dependency to `Instrument` and `InstrumentTrack` from `TrackLabelButton`. The icon of an `InstrumentTrackView` is now determined in the class `InstrumentTrackView` itself using the new method `determinePixmap`. This enables the removal of the overridden `paintEvent` method from `TrackLabelButton`. --- include/InstrumentTrackView.h | 2 ++ include/TrackLabelButton.h | 1 - src/gui/tracks/InstrumentTrackView.cpp | 26 ++++++++++++++++++++- src/gui/tracks/TrackLabelButton.cpp | 31 -------------------------- 4 files changed, 27 insertions(+), 33 deletions(-) diff --git a/include/InstrumentTrackView.h b/include/InstrumentTrackView.h index d7d5fb83a39..ef3d7f36073 100644 --- a/include/InstrumentTrackView.h +++ b/include/InstrumentTrackView.h @@ -93,6 +93,8 @@ private slots: void handleConfigChange(QString cls, QString attr, QString value); +private: + QPixmap determinePixmap(InstrumentTrack * instrumentTrack); private: InstrumentTrackWindow * m_window; diff --git a/include/TrackLabelButton.h b/include/TrackLabelButton.h index 0d1c6e163f4..af50b078b2e 100644 --- a/include/TrackLabelButton.h +++ b/include/TrackLabelButton.h @@ -55,7 +55,6 @@ public slots: 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; diff --git a/src/gui/tracks/InstrumentTrackView.cpp b/src/gui/tracks/InstrumentTrackView.cpp index 8087af42335..d72f276fa14 100644 --- a/src/gui/tracks/InstrumentTrackView.cpp +++ b/src/gui/tracks/InstrumentTrackView.cpp @@ -40,6 +40,7 @@ #include "Mixer.h" #include "MixerView.h" #include "GuiApplication.h" +#include "Instrument.h" #include "InstrumentTrack.h" #include "InstrumentTrackWindow.h" #include "MainWindow.h" @@ -62,7 +63,7 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV m_tlb = new TrackLabelButton( this, getTrackSettingsWidget() ); m_tlb->setCheckable( true ); - m_tlb->setIcon( embed::getIconPixmap( "instrument_track" ) ); + m_tlb->setIcon(determinePixmap(_it)); m_tlb->show(); connect( m_tlb, SIGNAL(toggled(bool)), @@ -393,4 +394,27 @@ QMenu * InstrumentTrackView::createMixerMenu(QString title, QString newMixerLabe } +QPixmap InstrumentTrackView::determinePixmap(InstrumentTrack * instrumentTrack) +{ + if (instrumentTrack) + { + Instrument * instrument = instrumentTrack->instrument(); + + if (instrument && instrument->descriptor()) + { + const PixmapLoader * pl = instrument->key().isValid() ? + instrument->key().logo() : + instrument->descriptor()->logo; + + if(pl) + { + return pl->pixmap(); + } + } + } + + return embed::getIconPixmap("instrument_track"); +} + + } // namespace lmms::gui diff --git a/src/gui/tracks/TrackLabelButton.cpp b/src/gui/tracks/TrackLabelButton.cpp index 2a50a4aa254..247f9ee52fc 100644 --- a/src/gui/tracks/TrackLabelButton.cpp +++ b/src/gui/tracks/TrackLabelButton.cpp @@ -31,8 +31,6 @@ #include "ConfigManager.h" #include "embed.h" #include "Engine.h" -#include "Instrument.h" -#include "InstrumentTrack.h" #include "RenameDialog.h" #include "Song.h" #include "TrackRenameLineEdit.h" @@ -185,35 +183,6 @@ void TrackLabelButton::mouseReleaseEvent( QMouseEvent *_me ) -void TrackLabelButton::paintEvent( QPaintEvent * _pe ) -{ - if( m_trackView->getTrack()->type() == Track::Type::Instrument ) - { - auto it = dynamic_cast(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 ); -} - - - - void TrackLabelButton::resizeEvent(QResizeEvent *_re) { setText( elideName( m_trackView->getTrack()->displayName() ) ); From 1903a30c58010d6af1de944646de91a691fb301c Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 24 Dec 2023 22:56:26 +0100 Subject: [PATCH 2/8] Add helper method isInCompactMode Add the helper method `isInCompactMode` which knows how to use the `ConfigManager` to determine if the option for compact track buttons is enabled. This removes duplicate code with intricate knowledge of the keys under which compact mode is stored. --- include/TrackLabelButton.h | 2 ++ src/gui/tracks/TrackLabelButton.cpp | 10 +++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/TrackLabelButton.h b/include/TrackLabelButton.h index af50b078b2e..e19fc6be9bc 100644 --- a/include/TrackLabelButton.h +++ b/include/TrackLabelButton.h @@ -57,6 +57,8 @@ public slots: void mouseReleaseEvent( QMouseEvent * _me ) override; void resizeEvent( QResizeEvent * _re ) override; +private: + bool isInCompactMode() const; private: TrackView * m_trackView; diff --git a/src/gui/tracks/TrackLabelButton.cpp b/src/gui/tracks/TrackLabelButton.cpp index 247f9ee52fc..77ab6a5b800 100644 --- a/src/gui/tracks/TrackLabelButton.cpp +++ b/src/gui/tracks/TrackLabelButton.cpp @@ -51,7 +51,7 @@ TrackLabelButton::TrackLabelButton( TrackView * _tv, QWidget * _parent ) : m_renameLineEdit = new TrackRenameLineEdit( this ); m_renameLineEdit->hide(); - if( ConfigManager::inst()->value( "ui", "compacttrackbuttons" ).toInt() ) + if (isInCompactMode()) { setFixedSize( 32, 29 ); } @@ -75,7 +75,7 @@ TrackLabelButton::TrackLabelButton( TrackView * _tv, QWidget * _parent ) : void TrackLabelButton::rename() { - if( ConfigManager::inst()->value( "ui", "compacttrackbuttons" ).toInt() ) + if(isInCompactMode()) { QString txt = m_trackView->getTrack()->name(); RenameDialog renameDlg( txt ); @@ -101,7 +101,7 @@ void TrackLabelButton::rename() void TrackLabelButton::renameFinished() { - if( !( ConfigManager::inst()->value( "ui", "compacttrackbuttons" ).toInt() ) ) + if(!isInCompactMode()) { m_renameLineEdit->clearFocus(); m_renameLineEdit->hide(); @@ -206,5 +206,9 @@ QString TrackLabelButton::elideName( const QString &name ) return elidedName; } +bool TrackLabelButton::isInCompactMode() const +{ + return ConfigManager::inst()->value("ui", "compacttrackbuttons").toInt(); +} } // namespace lmms::gui From 44175e3e32d78a98bf2c6d1f339528d1eb56b2eb Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 24 Dec 2023 23:20:45 +0100 Subject: [PATCH 3/8] Set song as modified when track name changes Extend `Track::setName` to set the song as modified whenever the track name changes. This moves the update into a core class and enables the removal of the dependencies to `Engine` and `Song` from the GUI class `TrackLabelButton`. Also add a check if the name is really changed and only perform the actions if that's the case. To make this work the implementation of `setName` had to be moved from the header into the implementation file. --- include/Track.h | 6 +----- src/core/Track.cpp | 16 ++++++++++++++++ src/gui/tracks/TrackLabelButton.cpp | 5 +---- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/include/Track.h b/include/Track.h index 248d56b0471..c814208cb10 100644 --- a/include/Track.h +++ b/include/Track.h @@ -201,11 +201,7 @@ class LMMS_EXPORT Track : public Model, public JournallingObject BoolModel* getMutedModel(); public slots: - virtual void setName( const QString & newName ) - { - m_name = newName; - emit nameChanged(); - } + virtual void setName( const QString & newName ); void setMutedBeforeSolo(const bool muted) { diff --git a/src/core/Track.cpp b/src/core/Track.cpp index 7a664a11e63..1530d991e6c 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -638,4 +638,20 @@ BoolModel *Track::getMutedModel() return &m_mutedModel; } +void Track::setName( const QString & newName ) + { + if (m_name != newName) + { + m_name = newName; + + lmms::Song * song = Engine::getSong(); + if (song) + { + song->setModified(); + } + + emit nameChanged(); + } + } + } // namespace lmms diff --git a/src/gui/tracks/TrackLabelButton.cpp b/src/gui/tracks/TrackLabelButton.cpp index 77ab6a5b800..1ed456bf478 100644 --- a/src/gui/tracks/TrackLabelButton.cpp +++ b/src/gui/tracks/TrackLabelButton.cpp @@ -30,11 +30,10 @@ #include "ConfigManager.h" #include "embed.h" -#include "Engine.h" #include "RenameDialog.h" -#include "Song.h" #include "TrackRenameLineEdit.h" #include "TrackView.h" +#include "Track.h" namespace lmms::gui { @@ -83,7 +82,6 @@ void TrackLabelButton::rename() if( txt != text() ) { m_trackView->getTrack()->setName( txt ); - Engine::getSong()->setModified(); } } else @@ -111,7 +109,6 @@ void TrackLabelButton::renameFinished() { setText( elideName( m_renameLineEdit->text() ) ); m_trackView->getTrack()->setName( m_renameLineEdit->text() ); - Engine::getSong()->setModified(); } } } From 666c91a77d34be33cdfc7b5fe44c65347bb38611 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Mon, 25 Dec 2023 19:48:02 +0100 Subject: [PATCH 4/8] Keep instrument and sample content at top on resize Keep the content of the instrument and sample track at the top of the widget if the widget is resized. This is also fixes a bug where the `TrackLabelButton` does not react anymore when the size is increased. Technically the layout with the actual widgets is put into another vertical layout with a spacer that consumes all remaining space at the bottom. --- src/gui/tracks/InstrumentTrackView.cpp | 6 +++++- src/gui/tracks/SampleTrackView.cpp | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/gui/tracks/InstrumentTrackView.cpp b/src/gui/tracks/InstrumentTrackView.cpp index d72f276fa14..00e3cfe8779 100644 --- a/src/gui/tracks/InstrumentTrackView.cpp +++ b/src/gui/tracks/InstrumentTrackView.cpp @@ -143,7 +143,9 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV m_activityIndicator->setFixedSize(8, 28); m_activityIndicator->show(); - auto layout = new QHBoxLayout(getTrackSettingsWidget()); + auto masterLayout = new QVBoxLayout(getTrackSettingsWidget()); + masterLayout->setContentsMargins(0, 1, 0, 0); + auto layout = new QHBoxLayout(); layout->setContentsMargins(0, 0, 0, 0); layout->setSpacing(0); layout->addWidget(m_tlb); @@ -151,6 +153,8 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV layout->addWidget(m_activityIndicator); layout->addWidget(m_volumeKnob); layout->addWidget(m_panningKnob); + masterLayout->addLayout(layout); + masterLayout->addSpacerItem(new QSpacerItem(0, 0, QSizePolicy::Minimum, QSizePolicy::Expanding)); connect( m_activityIndicator, SIGNAL(pressed()), this, SLOT(activityIndicatorPressed())); diff --git a/src/gui/tracks/SampleTrackView.cpp b/src/gui/tracks/SampleTrackView.cpp index ddb68ee998e..62ddcab83f6 100644 --- a/src/gui/tracks/SampleTrackView.cpp +++ b/src/gui/tracks/SampleTrackView.cpp @@ -86,7 +86,9 @@ SampleTrackView::SampleTrackView( SampleTrack * _t, TrackContainerView* tcv ) : m_activityIndicator->setFixedSize(8, 28); m_activityIndicator->show(); - auto layout = new QHBoxLayout(getTrackSettingsWidget()); + auto masterLayout = new QVBoxLayout(getTrackSettingsWidget()); + masterLayout->setContentsMargins(0, 1, 0, 0); + auto layout = new QHBoxLayout(); layout->setContentsMargins(0, 0, 0, 0); layout->setSpacing(0); layout->addWidget(m_tlb); @@ -94,6 +96,8 @@ SampleTrackView::SampleTrackView( SampleTrack * _t, TrackContainerView* tcv ) : layout->addWidget(m_activityIndicator); layout->addWidget(m_volumeKnob); layout->addWidget(m_panningKnob); + masterLayout->addLayout(layout); + masterLayout->addSpacerItem(new QSpacerItem(0, 0, QSizePolicy::Minimum, QSizePolicy::Expanding)); connect(_t, SIGNAL(playingChanged()), this, SLOT(updateIndicator())); From 2f29f9bbf38d7a3e8b26d4b8f01df281b6d5986c Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Mon, 25 Dec 2023 20:13:37 +0100 Subject: [PATCH 5/8] Vertical track resizing via mouse wheel Enable to vertically resize tracks using the mouse wheel. Scrolling the mouse wheel over the track view with the control key pressed will increase/decrease the height by one pixel per wheel event. Pressing the shift key will increase/decrease in steps of five pixels. Extract code that can be shared between the existing and the new way to resize the track into the private helper method `resizeToHeight`. --- include/TrackView.h | 3 +++ src/gui/tracks/TrackView.cpp | 28 +++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/include/TrackView.h b/include/TrackView.h index f697d9ea86a..7e1db2e7683 100644 --- a/include/TrackView.h +++ b/include/TrackView.h @@ -134,9 +134,12 @@ public slots: void mousePressEvent( QMouseEvent * me ) override; void mouseMoveEvent( QMouseEvent * me ) override; void mouseReleaseEvent( QMouseEvent * me ) override; + void wheelEvent(QWheelEvent * me) override; void paintEvent( QPaintEvent * pe ) override; void resizeEvent( QResizeEvent * re ) override; +private: + void resizeToHeight(int height); private: enum class Action diff --git a/src/gui/tracks/TrackView.cpp b/src/gui/tracks/TrackView.cpp index 426be7e3626..89639bf9419 100644 --- a/src/gui/tracks/TrackView.cpp +++ b/src/gui/tracks/TrackView.cpp @@ -364,9 +364,7 @@ void TrackView::mouseMoveEvent( QMouseEvent * me ) } else if( m_action == Action::Resize ) { - setFixedHeight( qMax( me->y(), MINIMAL_TRACK_HEIGHT ) ); - m_trackContainerView->realignTracks(); - m_track->setHeight( height() ); + resizeToHeight(me->y()); } if( height() < DEFAULT_TRACK_HEIGHT ) @@ -393,6 +391,22 @@ void TrackView::mouseReleaseEvent( QMouseEvent * me ) QWidget::mouseReleaseEvent( me ); } +void TrackView::wheelEvent(QWheelEvent * we) +{ + we->accept(); + + const int deltaY = we->angleDelta().y(); + int const direction = deltaY < 0 ? -1 : 1; + + auto const modKeys = we->modifiers(); + int stepSize = modKeys == Qt::ControlModifier ? 1 : modKeys == Qt::ShiftModifier ? 5 : 0; + + if (stepSize != 0) + { + resizeToHeight(height() + stepSize * direction); + } +} + @@ -445,4 +459,12 @@ void TrackView::setIndicatorMute(FadeButton* indicator, bool muted) } +void TrackView::resizeToHeight(int h) +{ + setFixedHeight(qMax(h, MINIMAL_TRACK_HEIGHT)); + m_trackContainerView->realignTracks(); + m_track->setHeight(height()); +} + + } // namespace lmms::gui From 2a6c540dc7dfd5971b723476ee99bd5a90b21d95 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sat, 17 Feb 2024 19:13:59 +0100 Subject: [PATCH 6/8] Render beat pattern step buttons at the top Render the step buttons of beat patterns at the top instead of at the bottom so that they stay aligned with the other elements to the left of them (buttons, knobs, etc). Set the y offset to 4 so that the step buttons are vertically aligned with the other elements. The previous calculation lead to a minimum offset of 6 which always made the step buttons look misaligned. Introduce the new static variable `s_beatStepButtonOffset` which ensures that the rendering and the evaluation of mouse clicks do not go out of sync. --- src/gui/clips/MidiClipView.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/gui/clips/MidiClipView.cpp b/src/gui/clips/MidiClipView.cpp index a9ee1cb4008..ef13d76b1ab 100644 --- a/src/gui/clips/MidiClipView.cpp +++ b/src/gui/clips/MidiClipView.cpp @@ -46,6 +46,7 @@ namespace lmms::gui { +static int const s_beatStepButtonOffset = 4; MidiClipView::MidiClipView( MidiClip* clip, TrackView* parent ) : ClipView( clip, parent ), @@ -246,7 +247,7 @@ void MidiClipView::mousePressEvent( QMouseEvent * _me ) { bool displayPattern = fixedClips() || (pixelsPerBar() >= 96 && m_legacySEPattern); if (_me->button() == Qt::LeftButton && m_clip->m_clipType == MidiClip::Type::BeatClip && displayPattern - && _me->y() > height() - m_stepBtnOff.height()) + && _me->y() > s_beatStepButtonOffset && _me->y() < s_beatStepButtonOffset + m_stepBtnOff.height()) // when mouse button is pressed in pattern mode @@ -478,7 +479,7 @@ void MidiClipView::paintEvent( QPaintEvent * ) // figure out x and y coordinates for step graphic const int x = BORDER_WIDTH + static_cast(it * w / steps); - const int y = height() - m_stepBtnOff.height() - 1; + const int y = s_beatStepButtonOffset; if (n) { From 1811425650db3a20b6f36e4f93520e38d514d4e5 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sat, 17 Feb 2024 21:23:18 +0100 Subject: [PATCH 7/8] Style changes from code review Incorporate some style changes proposed in a code review. --- include/InstrumentTrackView.h | 2 +- include/Track.h | 2 +- include/TrackView.h | 2 +- src/core/Track.cpp | 19 +++++++++---------- src/gui/tracks/InstrumentTrackView.cpp | 12 ++++++------ src/gui/tracks/TrackLabelButton.cpp | 4 ++-- src/gui/tracks/TrackView.cpp | 2 +- 7 files changed, 21 insertions(+), 22 deletions(-) diff --git a/include/InstrumentTrackView.h b/include/InstrumentTrackView.h index 23fa307d1ea..a7fa554b49e 100644 --- a/include/InstrumentTrackView.h +++ b/include/InstrumentTrackView.h @@ -94,7 +94,7 @@ private slots: void handleConfigChange(QString cls, QString attr, QString value); private: - QPixmap determinePixmap(InstrumentTrack * instrumentTrack); + QPixmap determinePixmap(InstrumentTrack* instrumentTrack); private: InstrumentTrackWindow * m_window; diff --git a/include/Track.h b/include/Track.h index c814208cb10..1c161984f00 100644 --- a/include/Track.h +++ b/include/Track.h @@ -201,7 +201,7 @@ class LMMS_EXPORT Track : public Model, public JournallingObject BoolModel* getMutedModel(); public slots: - virtual void setName( const QString & newName ); + virtual void setName(const QString& newName); void setMutedBeforeSolo(const bool muted) { diff --git a/include/TrackView.h b/include/TrackView.h index 7e1db2e7683..b2654202b9a 100644 --- a/include/TrackView.h +++ b/include/TrackView.h @@ -134,7 +134,7 @@ public slots: void mousePressEvent( QMouseEvent * me ) override; void mouseMoveEvent( QMouseEvent * me ) override; void mouseReleaseEvent( QMouseEvent * me ) override; - void wheelEvent(QWheelEvent * me) override; + void wheelEvent(QWheelEvent* we) override; void paintEvent( QPaintEvent * pe ) override; void resizeEvent( QResizeEvent * re ) override; diff --git a/src/core/Track.cpp b/src/core/Track.cpp index 4390d8314b5..1ea1e4d7307 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -638,19 +638,18 @@ BoolModel *Track::getMutedModel() } void Track::setName( const QString & newName ) +{ + if (m_name != newName) { - if (m_name != newName) - { - m_name = newName; + m_name = newName; - lmms::Song * song = Engine::getSong(); - if (song) - { - song->setModified(); - } - - emit nameChanged(); + if (auto song = Engine::getSong()) + { + song->setModified(); } + + emit nameChanged(); } +} } // namespace lmms diff --git a/src/gui/tracks/InstrumentTrackView.cpp b/src/gui/tracks/InstrumentTrackView.cpp index 04ec6ba15cd..788991ed0f8 100644 --- a/src/gui/tracks/InstrumentTrackView.cpp +++ b/src/gui/tracks/InstrumentTrackView.cpp @@ -398,19 +398,19 @@ QMenu * InstrumentTrackView::createMixerMenu(QString title, QString newMixerLabe } -QPixmap InstrumentTrackView::determinePixmap(InstrumentTrack * instrumentTrack) +QPixmap InstrumentTrackView::determinePixmap(InstrumentTrack* instrumentTrack) { if (instrumentTrack) { - Instrument * instrument = instrumentTrack->instrument(); + Instrument* instrument = instrumentTrack->instrument(); if (instrument && instrument->descriptor()) { - const PixmapLoader * pl = instrument->key().isValid() ? - instrument->key().logo() : - instrument->descriptor()->logo; + const PixmapLoader* pl = instrument->key().isValid() + ? instrument->key().logo() + : instrument->descriptor()->logo; - if(pl) + if (pl) { return pl->pixmap(); } diff --git a/src/gui/tracks/TrackLabelButton.cpp b/src/gui/tracks/TrackLabelButton.cpp index 1ed456bf478..087edba3d74 100644 --- a/src/gui/tracks/TrackLabelButton.cpp +++ b/src/gui/tracks/TrackLabelButton.cpp @@ -74,7 +74,7 @@ TrackLabelButton::TrackLabelButton( TrackView * _tv, QWidget * _parent ) : void TrackLabelButton::rename() { - if(isInCompactMode()) + if (isInCompactMode()) { QString txt = m_trackView->getTrack()->name(); RenameDialog renameDlg( txt ); @@ -99,7 +99,7 @@ void TrackLabelButton::rename() void TrackLabelButton::renameFinished() { - if(!isInCompactMode()) + if (!isInCompactMode()) { m_renameLineEdit->clearFocus(); m_renameLineEdit->hide(); diff --git a/src/gui/tracks/TrackView.cpp b/src/gui/tracks/TrackView.cpp index 89639bf9419..e7f0bcf9f74 100644 --- a/src/gui/tracks/TrackView.cpp +++ b/src/gui/tracks/TrackView.cpp @@ -391,7 +391,7 @@ void TrackView::mouseReleaseEvent( QMouseEvent * me ) QWidget::mouseReleaseEvent( me ); } -void TrackView::wheelEvent(QWheelEvent * we) +void TrackView::wheelEvent(QWheelEvent* we) { we->accept(); From ffb2a66996e05bb64f04ab19be372f3d9e479abc Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 18 Feb 2024 09:56:29 +0100 Subject: [PATCH 8/8] More code review changes Make `InstrumentTrackView::determinePixmap` static. I have also attempted to keep the non-static member function version and to use the `InstrumentTrackView`'s model with a cast. However, this did not work because 'setModel' is executed as the very last step in the constructor, i.e. the model is not set when `determinePixmap` is called. Pulling it to the top is too risky right now. It's a little bit strange anyway that the `InstrumentTrackView` immediately "forgets" that it's working on an `InstrumentTrack` and that casts would be necessary although it receives a "pristine" `InstrumentTrack` in it's constructor. Rename `s_beatStepButtonOffset` to `BeatStepButtonOffset` and define it as a constexpr. More corrections of whitespace and indentation. --- include/InstrumentTrackView.h | 2 +- src/core/Track.cpp | 2 +- src/gui/clips/MidiClipView.cpp | 6 +++--- src/gui/tracks/TrackView.cpp | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/InstrumentTrackView.h b/include/InstrumentTrackView.h index a7fa554b49e..c7d524b36cf 100644 --- a/include/InstrumentTrackView.h +++ b/include/InstrumentTrackView.h @@ -94,7 +94,7 @@ private slots: void handleConfigChange(QString cls, QString attr, QString value); private: - QPixmap determinePixmap(InstrumentTrack* instrumentTrack); + static QPixmap determinePixmap(InstrumentTrack* instrumentTrack); private: InstrumentTrackWindow * m_window; diff --git a/src/core/Track.cpp b/src/core/Track.cpp index 1ea1e4d7307..6c4ba465e4b 100644 --- a/src/core/Track.cpp +++ b/src/core/Track.cpp @@ -637,7 +637,7 @@ BoolModel *Track::getMutedModel() return &m_mutedModel; } -void Track::setName( const QString & newName ) +void Track::setName(const QString& newName) { if (m_name != newName) { diff --git a/src/gui/clips/MidiClipView.cpp b/src/gui/clips/MidiClipView.cpp index ef13d76b1ab..0a6fece3168 100644 --- a/src/gui/clips/MidiClipView.cpp +++ b/src/gui/clips/MidiClipView.cpp @@ -46,7 +46,7 @@ namespace lmms::gui { -static int const s_beatStepButtonOffset = 4; +constexpr int BeatStepButtonOffset = 4; MidiClipView::MidiClipView( MidiClip* clip, TrackView* parent ) : ClipView( clip, parent ), @@ -247,7 +247,7 @@ void MidiClipView::mousePressEvent( QMouseEvent * _me ) { bool displayPattern = fixedClips() || (pixelsPerBar() >= 96 && m_legacySEPattern); if (_me->button() == Qt::LeftButton && m_clip->m_clipType == MidiClip::Type::BeatClip && displayPattern - && _me->y() > s_beatStepButtonOffset && _me->y() < s_beatStepButtonOffset + m_stepBtnOff.height()) + && _me->y() > BeatStepButtonOffset && _me->y() < BeatStepButtonOffset + m_stepBtnOff.height()) // when mouse button is pressed in pattern mode @@ -479,7 +479,7 @@ void MidiClipView::paintEvent( QPaintEvent * ) // figure out x and y coordinates for step graphic const int x = BORDER_WIDTH + static_cast(it * w / steps); - const int y = s_beatStepButtonOffset; + const int y = BeatStepButtonOffset; if (n) { diff --git a/src/gui/tracks/TrackView.cpp b/src/gui/tracks/TrackView.cpp index e7f0bcf9f74..08ad01c9c36 100644 --- a/src/gui/tracks/TrackView.cpp +++ b/src/gui/tracks/TrackView.cpp @@ -462,8 +462,8 @@ void TrackView::setIndicatorMute(FadeButton* indicator, bool muted) void TrackView::resizeToHeight(int h) { setFixedHeight(qMax(h, MINIMAL_TRACK_HEIGHT)); - m_trackContainerView->realignTracks(); - m_track->setHeight(height()); + m_trackContainerView->realignTracks(); + m_track->setHeight(height()); }