Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add vertical zoom controls to SongEditor #6776

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add vertical zoom controls to SongEditor #6776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c53f3d9
Bump cmt submodule (#6151)
tresf Jul 11, 2023
10d9512
allow y-resizing of song editor
enp2s0 Jul 18, 2023
9002145
respect manually resized tracks
enp2s0 Jul 18, 2023
94361c6
rename existing zoom variables for consistency
enp2s0 Jul 19, 2023
fb4f762
bugfix: make new tracks respect scaling
enp2s0 Jul 19, 2023
8d6d042
rename for consistency
enp2s0 Jul 20, 2023
ee05592
style/formatting fix
enp2s0 Jul 20, 2023
cdf0ac4
one more fix
enp2s0 Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions include/SongEditor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ class SongEditor : public TrackContainerView
void saveSettings( QDomDocument& doc, QDomElement& element ) override;
void loadSettings( const QDomElement& element ) override;

ComboBoxModel *zoomingModel() const;
ComboBoxModel* zoomingXModel() const;
ComboBoxModel* zoomingYModel() const;
ComboBoxModel *snappingModel() const;
float getSnapSize() const;
QString getSnapSizeString() const;
Expand Down Expand Up @@ -112,6 +113,7 @@ private slots:
void updateScrollBar(int len);

void zoomingChanged();
void zoomingYChanged();

private:
void keyPressEvent( QKeyEvent * ke ) override;
Expand Down Expand Up @@ -141,11 +143,13 @@ private slots:

PositionLine * m_positionLine;

ComboBoxModel* m_zoomingModel;
ComboBoxModel* m_zoomingXModel;
ComboBoxModel* m_zoomingYModel;
ComboBoxModel* m_snappingModel;
bool m_proportionalSnap;

static const QVector<float> m_zoomLevels;
static const QVector<float> m_zoomXLevels;
static const QVector<float> m_zoomYLevels;
Comment on lines +151 to +152
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static const QVector<float> m_zoomXLevels;
static const QVector<float> m_zoomYLevels;
static constexpr std::array m_zoomXLevels = { 0.125f, 0.25f, 0.5f, 1.0f, 2.0f, 4.0f, 8.0f, 16.0f };
static constexpr std::array m_zoomYLevels = { 1.0f, 1.25f, 1.5f, 1.75f, 2.0f, 2.5f, 3.0f, 4.0f, 8.0f, 16.0f };

Can use constexpr arrays instead.


bool m_scrollBack;
bool m_smoothScroll;
Expand Down Expand Up @@ -213,7 +217,8 @@ protected slots:
QAction* m_selectModeAction;
QAction* m_crtlAction;

ComboBox * m_zoomingComboBox;
ComboBox* m_zoomingXComboBox;
ComboBox* m_zoomingYComboBox;
ComboBox * m_snappingComboBox;
QLabel* m_snapSizeLabel;

Expand Down
3 changes: 3 additions & 0 deletions include/TrackContainerView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class TrackContainerView : public QWidget, public ModelView,
}

void setPixelsPerBar( int ppb );
void setVerticalScale(double h);

const TrackView * trackViewAt( const int _y ) const;

Expand Down Expand Up @@ -167,6 +168,7 @@ public slots:

protected:
static const int DEFAULT_PIXELS_PER_BAR = 16;
static const int DEFAULT_TRACK_HEIGHT = 32;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static const int DEFAULT_TRACK_HEIGHT = 32;
static constexpr int DEFAULT_TRACK_HEIGHT = 32;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this constant entirely - there are already MINIMAL_TRACK_HEIGHT and DEFAULT_TRACK_HEIGHT constants defined in include/Track.h.


void resizeEvent( QResizeEvent * ) override;

Expand Down Expand Up @@ -203,6 +205,7 @@ public slots:
QVBoxLayout * m_scrollLayout;

float m_ppb;
double m_trackHeightScale;

RubberBand * m_rubberBand;

Expand Down
2 changes: 1 addition & 1 deletion plugins/LadspaEffect/cmt/cmt
Open in desktop
Submodule cmt updated 44 files
+2 −1 README
+16 −0 doc/changes.html
+1 −1 doc/overview.html
+1 −2 src/am.cpp
+5 −8 src/ambisonic.cpp
+1 −2 src/amp.cpp
+1 −2 src/analogue.cpp
+1 −2 src/canyondelay.cpp
+1 −2 src/cmt.cpp
+5 −2 src/cmt.h
+20 −15 src/delay.cpp
+1 −2 src/descriptor.cpp
+4 −2 src/disintegrator.cpp
+1 −2 src/dynamic.cpp
+1 −2 src/filter.cpp
+18 −18 src/freeverb/Components/denormals.h
+122 −26 src/freeverb/Components/revmodel.cpp
+5 −31 src/freeverb/Components/revmodel.h
+60 −60 src/freeverb/Components/tuning.h
+3 −4 src/freeverb/freeverb.cpp
+67 −67 src/freeverb/readme.txt
+1 −2 src/grain.cpp
+1 −2 src/hardgate.cpp
+5 −2 src/init.cpp
+1 −2 src/ladspa_types.h
+1 −2 src/lofi.cpp
+1 −2 src/logistic.cpp
+1 −2 src/mixer.cpp
+1 −2 src/noise.cpp
+1 −2 src/null.cpp
+1 −2 src/organ.cpp
+1 −2 src/peak.cpp
+1 −2 src/phasemod.cpp
+1 −2 src/pink.cpp
+3 −5 src/pink_full.cpp
+1 −2 src/pink_sh.cpp
+1 −2 src/pinknoise.h
+1 −2 src/run_adding.h
+1 −2 src/sine.cpp
+1 −2 src/sledgehammer.cpp
+1 −2 src/syndrum.cpp
+1 −2 src/utils.h
+1 −2 src/vcf303.cpp
+1 −2 src/wshape_sine.cpp
2 changes: 1 addition & 1 deletion src/gui/clips/ClipView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ ClipView::ClipView( Clip * clip,

connect( m_clip, SIGNAL(lengthChanged()),
this, SLOT(updateLength()));
connect( getGUI()->songEditor()->m_editor->zoomingModel(), SIGNAL(dataChanged()), this, SLOT(updateLength()));
connect(getGUI()->songEditor()->m_editor->zoomingXModel(), SIGNAL(dataChanged()), this, SLOT(updateLength()));
connect( m_clip, SIGNAL(positionChanged()),
this, SLOT(updatePosition()));
connect( m_clip, SIGNAL(destroyedClip()), this, SLOT(close()));
Expand Down
110 changes: 69 additions & 41 deletions src/gui/editors/SongEditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,17 @@ namespace lmms::gui
{


const QVector<float> SongEditor::m_zoomLevels =
const QVector<float> SongEditor::m_zoomXLevels =
{ 0.125f, 0.25f, 0.5f, 1.0f, 2.0f, 4.0f, 8.0f, 16.0f };

const QVector<float> SongEditor::m_zoomYLevels =
{ 1.0f, 1.25f, 1.5f, 1.75f, 2.0f, 2.5f, 3.0f, 4.0f, 8.0f, 16.0f };
Comment on lines +64 to +65
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason for these specific values? You may also want to look at #6664, which is changing the horizontal zoom control to a continuous slider.


Comment on lines +61 to +66
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be removed after making them constexpr.

SongEditor::SongEditor( Song * song ) :
TrackContainerView( song ),
m_song( song ),
m_zoomingModel(new ComboBoxModel()),
m_zoomingXModel(new ComboBoxModel()),
m_zoomingYModel(new ComboBoxModel()),
m_snappingModel(new ComboBoxModel()),
m_proportionalSnap( false ),
m_scrollBack( false ),
Expand All @@ -75,13 +79,14 @@ SongEditor::SongEditor( Song * song ) :
m_mousePos(),
m_rubberBandStartTrackview(0),
m_rubberbandStartTimePos(0),
m_currentZoomingValue(m_zoomingModel->value()),
m_currentZoomingValue(m_zoomingXModel->value()),
m_trackHeadWidth(ConfigManager::inst()->value("ui", "compacttrackbuttons").toInt()==1
? DEFAULT_SETTINGS_WIDGET_WIDTH_COMPACT + TRACK_OP_WIDTH_COMPACT
: DEFAULT_SETTINGS_WIDGET_WIDTH + TRACK_OP_WIDTH),
m_selectRegion(false)
{
m_zoomingModel->setParent(this);
m_zoomingXModel->setParent(this);
m_zoomingYModel->setParent(this);
m_snappingModel->setParent(this);
m_timeLine = new TimeLineWidget( m_trackHeadWidth, 32,
pixelsPerBar(),
Expand Down Expand Up @@ -242,16 +247,22 @@ SongEditor::SongEditor( Song * song ) :


//Set up zooming model
for( float const & zoomLevel : m_zoomLevels )
for(const auto& zoomLevel : m_zoomXLevels)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for(const auto& zoomLevel : m_zoomXLevels)
for (const auto& zoomLevel : m_zoomXLevels)

style

{
m_zoomingXModel->addItem(QString("%1\%").arg(zoomLevel * 100));
}
m_zoomingXModel->setInitValue(m_zoomingXModel->findText("100%"));
connect(m_zoomingXModel, SIGNAL(dataChanged()), this, SLOT(zoomingChanged()));
connect(m_zoomingXModel, SIGNAL(dataChanged()), m_positionLine, SLOT(update()));
Comment on lines +255 to +256
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be better to avoid the SIGNAL and SLOT macros and use the newer way of calling connect.
See: https://doc.qt.io/qt-5/signalsandslots.html


// Set up Y-axis zooming model
for(const auto& zoomLevel : m_zoomYLevels)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for(const auto& zoomLevel : m_zoomYLevels)
for (const auto& zoomLevel : m_zoomYLevels)

style

{
m_zoomingModel->addItem( QString( "%1\%" ).arg( zoomLevel * 100 ) );
m_zoomingYModel->addItem(QString("%1\%").arg(zoomLevel * 100));
}
m_zoomingModel->setInitValue(
m_zoomingModel->findText( "100%" ) );
connect( m_zoomingModel, SIGNAL(dataChanged()),
this, SLOT(zoomingChanged()));
connect( m_zoomingModel, SIGNAL(dataChanged()),
m_positionLine, SLOT(update()));
m_zoomingYModel->setInitValue(m_zoomingYModel->findText("100%"));
connect(m_zoomingYModel, SIGNAL(dataChanged()), this, SLOT(zoomingYChanged()));


//Set up snapping model, 2^i
for ( int i = 3; i >= -4; i-- )
Expand Down Expand Up @@ -298,7 +309,7 @@ float SongEditor::getSnapSize() const
// If proportional snap is on, we snap to finer values when zoomed in
if (m_proportionalSnap)
{
val = val - m_zoomingModel->value() + 3;
val = val - m_zoomingXModel->value() + 3;
}
val = std::max(val, -6); // -6 gives 1/64th bar snapping. Lower values cause crashing.

Expand All @@ -313,7 +324,7 @@ float SongEditor::getSnapSize() const
QString SongEditor::getSnapSizeString() const
{
int val = -m_snappingModel->value() + 3;
val = val - m_zoomingModel->value() + 3;
val = val - m_zoomingXModel->value() + 3;
val = std::max(val, -6); // -6 gives 1/64th bar snapping. Lower values cause crashing.

if ( val >= 0 ){
Expand Down Expand Up @@ -367,7 +378,7 @@ void SongEditor::selectRegionFromPixels(int xStart, int xEnd)
//we save the position of scrollbars, mouse position and zooming level
m_origin = QPoint(xStart, 0);
m_scrollPos = QPoint(m_leftRightScroll->value(), contentWidget()->verticalScrollBar()->value());
m_currentZoomingValue = zoomingModel()->value();
m_currentZoomingValue = zoomingXModel()->value();

//calculate the song position where the mouse was clicked
m_rubberbandStartTimePos = TimePos((xStart - m_trackHeadWidth)
Expand Down Expand Up @@ -399,10 +410,10 @@ void SongEditor::updateRubberband()
int originX = m_origin.x();

//take care of the zooming
if (m_currentZoomingValue != m_zoomingModel->value())
if (m_currentZoomingValue != m_zoomingXModel->value())
{
originX = m_trackHeadWidth + (originX - m_trackHeadWidth)
* m_zoomLevels[m_zoomingModel->value()] / m_zoomLevels[m_currentZoomingValue];
* m_zoomXLevels[m_zoomingXModel->value()] / m_zoomXLevels[m_currentZoomingValue];
}

//take care of the scrollbar position
Expand Down Expand Up @@ -537,7 +548,7 @@ void SongEditor::wheelEvent( QWheelEvent * we )
{
if( we->modifiers() & Qt::ControlModifier )
{
int z = m_zoomingModel->value();
int z = m_zoomingXModel->value();

if(we->angleDelta().y() > 0)
{
Expand All @@ -547,19 +558,19 @@ void SongEditor::wheelEvent( QWheelEvent * we )
{
z--;
}
z = qBound( 0, z, m_zoomingModel->size() - 1 );
z = qBound(0, z, m_zoomingXModel->size() - 1);
Copy link
Member

@messmerd messmerd Jul 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
z = qBound(0, z, m_zoomingXModel->size() - 1);
z = std::clamp(z, 0, m_zoomingXModel->size() - 1);

We're trying to get away from Qt-specific functions where we can. You may need to include <algorithm> for this.



int x = position(we).x() - m_trackHeadWidth;
// bar based on the mouse x-position where the scroll wheel was used
int bar = x / pixelsPerBar();
// what would be the bar in the new zoom level on the very same mouse x
int newBar = x / DEFAULT_PIXELS_PER_BAR / m_zoomLevels[z];
int newBar = x / DEFAULT_PIXELS_PER_BAR / m_zoomXLevels[z];
// scroll so the bar "selected" by the mouse x doesn't move on the screen
m_leftRightScroll->setValue(m_leftRightScroll->value() + bar - newBar);

// update combobox with zooming-factor
m_zoomingModel->setValue( z );
m_zoomingXModel->setValue(z);

// update timeline
m_song->m_playPos[Song::Mode_PlaySong].m_timeLine->
Expand Down Expand Up @@ -612,7 +623,7 @@ void SongEditor::mousePressEvent(QMouseEvent *me)
//we save the position of scrollbars, mouse position and zooming level
m_scrollPos = QPoint(m_leftRightScroll->value(), contentWidget()->verticalScrollBar()->value());
m_origin = contentWidget()->mapFromParent(QPoint(me->pos().x(), me->pos().y()));
m_currentZoomingValue = zoomingModel()->value();
m_currentZoomingValue = zoomingXModel()->value();

//paint the rubberband
rubberBand()->setEnabled(true);
Expand Down Expand Up @@ -839,17 +850,21 @@ void SongEditor::updatePositionLine()

void SongEditor::zoomingChanged()
{
setPixelsPerBar( m_zoomLevels[m_zoomingModel->value()] * DEFAULT_PIXELS_PER_BAR );

m_song->m_playPos[Song::Mode_PlaySong].m_timeLine->
setPixelsPerBar( pixelsPerBar() );
setPixelsPerBar(m_zoomXLevels[m_zoomingXModel->value()] * DEFAULT_PIXELS_PER_BAR);

m_song->m_playPos[Song::Mode_PlaySong].m_timeLine->setPixelsPerBar(pixelsPerBar());
realignTracks();
updateRubberband();
m_timeLine->setSnapSize(getSnapSize());

emit zoomingValueChanged( m_zoomLevels[m_zoomingModel->value()] );
emit zoomingValueChanged(m_zoomXLevels[m_zoomingXModel->value()]);
}

void SongEditor::zoomingYChanged()
{
setVerticalScale(m_zoomYLevels[m_zoomingYModel->value()]);
updatePositionLine();
}

void SongEditor::selectAllClips( bool select )
{
Expand Down Expand Up @@ -899,12 +914,15 @@ int SongEditor::indexOfTrackView(const TrackView *tv)



ComboBoxModel *SongEditor::zoomingModel() const
ComboBoxModel* SongEditor::zoomingXModel() const
{
return m_zoomingModel;
return m_zoomingXModel;
}


ComboBoxModel* SongEditor::zoomingYModel() const
{
return m_zoomingYModel;
}


ComboBoxModel *SongEditor::snappingModel() const
Expand Down Expand Up @@ -988,19 +1006,29 @@ SongEditorWindow::SongEditorWindow(Song* song) :

DropToolBar *zoomToolBar = addDropToolBarToTop(tr("Zoom controls"));

auto zoom_lbl = new QLabel(m_toolBar);
zoom_lbl->setPixmap( embed::getIconPixmap( "zoom" ) );
auto zoomx_lbl = new QLabel(m_toolBar);
zoomx_lbl->setPixmap(embed::getIconPixmap("zoom_x"));
auto zoomy_lbl = new QLabel(m_toolBar);
Comment on lines +1009 to +1011
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zoomx_lbl and zoomy_lbl don't follow our naming conventions. Maybe zoomXLabel and zoomYLabel could be used.

zoomy_lbl->setPixmap(embed::getIconPixmap("zoom_y"));

//Set up zooming-stuff
m_zoomingComboBox = new ComboBox( m_toolBar );
m_zoomingComboBox->setFixedSize( 80, ComboBox::DEFAULT_HEIGHT );
m_zoomingComboBox->move( 580, 4 );
m_zoomingComboBox->setModel(m_editor->m_zoomingModel);
m_zoomingComboBox->setToolTip(tr("Horizontal zooming"));
connect(m_editor->zoomingModel(), SIGNAL(dataChanged()), this, SLOT(updateSnapLabel()));

zoomToolBar->addWidget( zoom_lbl );
zoomToolBar->addWidget( m_zoomingComboBox );
m_zoomingXComboBox = new ComboBox(m_toolBar);
m_zoomingXComboBox->setFixedSize(80, ComboBox::DEFAULT_HEIGHT);
m_zoomingXComboBox->move(580, 4);
m_zoomingXComboBox->setModel(m_editor->m_zoomingXModel);
m_zoomingXComboBox->setToolTip(tr("Horizontal zooming"));
connect(m_editor->zoomingXModel(), SIGNAL(dataChanged()), this, SLOT(updateSnapLabel()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SIGNAL and SLOT macros can be removed here too


m_zoomingYComboBox = new ComboBox(m_toolBar);
m_zoomingYComboBox->setFixedSize(80, ComboBox::DEFAULT_HEIGHT);
m_zoomingYComboBox->move(580, 4);
m_zoomingYComboBox->setModel(m_editor->m_zoomingYModel);
m_zoomingYComboBox->setToolTip(tr("Vertical zooming"));

zoomToolBar->addWidget(zoomx_lbl);
zoomToolBar->addWidget(m_zoomingXComboBox);
zoomToolBar->addWidget(zoomy_lbl);
zoomToolBar->addWidget(m_zoomingYComboBox);

DropToolBar *snapToolBar = addDropToolBarToTop(tr("Snap controls"));
auto snap_lbl = new QLabel(m_toolBar);
Expand Down
35 changes: 34 additions & 1 deletion src/gui/editors/TrackContainerView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ TrackContainerView::TrackContainerView( TrackContainer * _tc ) :
m_trackViews(),
m_scrollArea( new scrollArea( this ) ),
m_ppb( DEFAULT_PIXELS_PER_BAR ),
m_trackHeightScale(1),
m_rubberBand( new RubberBand( m_scrollArea ) )
{
m_tc->setHook( this );
Expand Down Expand Up @@ -280,7 +281,13 @@ TrackView * TrackContainerView::createTrackView( Track * _t )
if (trackView->getTrack() == _t) { return trackView; }
}

return _t->createView( this );
auto trackView = _t->createView(this);

int height = lround(m_trackHeightScale * DEFAULT_TRACK_HEIGHT);
trackView->setFixedHeight(height);
trackView->getTrack()->setHeight(height);
Comment on lines +286 to +288
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will overwrite any custom height when creating the view for a track, which will affect loading tracks from files and cloning tracks.


return trackView;
}


Expand Down Expand Up @@ -349,6 +356,32 @@ void TrackContainerView::setPixelsPerBar( int ppb )
}
}

void TrackContainerView::setVerticalScale(double h)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely there's a better name than h for this parameter?

{
for (const auto& trackView : m_trackViews)
{
int new_height = lround(h * DEFAULT_TRACK_HEIGHT);
int cur_height = trackView->getTrack()->getHeight();

// If a track has been manually resized, calculate the new height by scaling the old height rather than just setting it.
if(cur_height != lround((m_trackHeightScale * DEFAULT_TRACK_HEIGHT)))
{
new_height = lround(cur_height * (h / m_trackHeightScale));
}

// clamp the value to prevent it from becoming smaller than the zoom level.
if(new_height < lround(h * DEFAULT_TRACK_HEIGHT))
{
new_height = lround(h * DEFAULT_TRACK_HEIGHT);
}
Comment on lines +372 to +376
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use std::max here. Also, why restrict to the zoomed default track height, rather than the absolute minimum track height? Users can still shrink tracks once zoomed in, so this will reset the height of such tracks whenever the user changes the zoom level, which sounds annoying.


trackView->setFixedHeight(new_height);
trackView->getTrack()->setHeight(new_height);
}

m_trackHeightScale = h;
}




Expand Down