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.

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

Allow waveform zoom levels to be fractional for smoother zooming with mouse wheel #1843

Merged
merged 1 commit into from
Dec 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/preferences/dialog/dlgprefwaveform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ void DlgPrefWaveform::slotUpdate() {
midVisualGain->setValue(factory->getVisualGain(WaveformWidgetFactory::Mid));
highVisualGain->setValue(factory->getVisualGain(WaveformWidgetFactory::High));
normalizeOverviewCheckBox->setChecked(factory->isOverviewNormalized());
defaultZoomComboBox->setCurrentIndex(factory->getDefaultZoom() - 1);
// Round zoom to int to get a default zoom index.
defaultZoomComboBox->setCurrentIndex(static_cast<int>(factory->getDefaultZoom()) - 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this result in a situation where a user accidentally has their zoom level changed slightly by rounding because they press Apply in the preferences?

Copy link
Member Author

@ywwg ywwg Oct 15, 2018

Choose a reason for hiding this comment

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

The preferences only controls the default zoom level, so hitting apply only updates the saved value in the config. It doesn't change the zoom level if the user has used the mouse wheel to change the zoom from the default. Changing the selection in the preferences does update the zoom level to those values. All in all the experience seems ok.

It used to be possible to right click on the waveform to reset to the default zoom level. I don't know why that was removed, would it be ok to add it back in? Right click-and-hold is used for fast track navigation so I guess that's a little overloaded.

Copy link
Contributor

@Be-ing Be-ing Dec 26, 2018

Choose a reason for hiding this comment

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

Okay, thanks for clarifying. Sorry for the delay reviewing this. I'm going over old PRs now that we finally got 2.2.0 released.

It used to be possible to right click on the waveform to reset to the default zoom level. I don't know why that was removed, would it be ok to add it back in? Right click-and-hold is used for fast track navigation so I guess that's a little overloaded.

Seems reasonable to me. I didn't know about the right click and drag action for quick seeking. I don't really understand the use case considering it's much easier to seek where you want by simply clicking on the overview waveforms. If you want to implement this, please open a new PR.

playMarkerPositionSlider->setValue(factory->getPlayMarkerPosition() * 100);
beatGridAlphaSpinBox->setValue(factory->beatGridAlpha());
beatGridAlphaSlider->setValue(factory->beatGridAlpha());
Expand Down
8 changes: 4 additions & 4 deletions src/waveform/renderers/waveformwidgetrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
#include "util/math.h"
#include "util/performancetimer.h"

const int WaveformWidgetRenderer::s_waveformMinZoom = 1;
const int WaveformWidgetRenderer::s_waveformMaxZoom = 10;
const int WaveformWidgetRenderer::s_waveformDefaultZoom = 3;
const double WaveformWidgetRenderer::s_waveformMinZoom = 1.0;
const double WaveformWidgetRenderer::s_waveformMaxZoom = 10.0;
const double WaveformWidgetRenderer::s_waveformDefaultZoom = 3.0;
const double WaveformWidgetRenderer::s_defaultPlayMarkerPosition = 0.5;

WaveformWidgetRenderer::WaveformWidgetRenderer(const char* group)
Expand Down Expand Up @@ -288,7 +288,7 @@ void WaveformWidgetRenderer::setup(
}
}

void WaveformWidgetRenderer::setZoom(int zoom) {
void WaveformWidgetRenderer::setZoom(double zoom) {
//qDebug() << "WaveformWidgetRenderer::setZoom" << zoom;
m_zoomFactor = math_clamp<double>(zoom, s_waveformMinZoom, s_waveformMaxZoom);
}
Expand Down
8 changes: 4 additions & 4 deletions src/waveform/renderers/waveformwidgetrenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ class VSyncThread;

class WaveformWidgetRenderer {
public:
static const int s_waveformMinZoom;
static const int s_waveformMaxZoom;
static const int s_waveformDefaultZoom;
static const double s_waveformMinZoom;
static const double s_waveformMaxZoom;
static const double s_waveformDefaultZoom;
static const double s_defaultPlayMarkerPosition;

public:
Expand All @@ -43,7 +43,7 @@ class WaveformWidgetRenderer {
double getFirstDisplayedPosition() const { return m_firstDisplayedPosition;}
double getLastDisplayedPosition() const { return m_lastDisplayedPosition;}

void setZoom(int zoom);
void setZoom(double zoom);

void setDisplayBeatGrid(bool set);
void setDisplayBeatGridAlpha(int alpha);
Expand Down
10 changes: 5 additions & 5 deletions src/waveform/waveformwidgetfactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ bool WaveformWidgetFactory::setConfig(UserSettingsPointer config) {
int vsync = m_config->getValue(ConfigKey("[Waveform]","VSync"), 0);
setVSyncType(vsync);

int defaultZoom = m_config->getValueString(ConfigKey("[Waveform]","DefaultZoom")).toInt(&ok);
double defaultZoom = m_config->getValueString(ConfigKey("[Waveform]","DefaultZoom")).toDouble(&ok);
if (ok) {
setDefaultZoom(defaultZoom);
} else{
Expand Down Expand Up @@ -399,7 +399,7 @@ bool WaveformWidgetFactory::setWidgetTypeFromHandle(int handleIndex) {
WaveformWidgetAbstract* previousWidget = holder.m_waveformWidget;
TrackPointer pTrack = previousWidget->getTrackInfo();
//previousWidget->hold();
int previousZoom = previousWidget->getZoomFactor();
double previousZoom = previousWidget->getZoomFactor();
double previousPlayMarkerPosition = previousWidget->getPlayMarkerPosition();
delete previousWidget;
WWaveformViewer* viewer = holder.m_waveformViewer;
Expand All @@ -423,7 +423,7 @@ bool WaveformWidgetFactory::setWidgetTypeFromHandle(int handleIndex) {
return true;
}

void WaveformWidgetFactory::setDefaultZoom(int zoom) {
void WaveformWidgetFactory::setDefaultZoom(double zoom) {
m_defaultZoom = math_clamp(zoom, WaveformWidgetRenderer::s_waveformMinZoom,
WaveformWidgetRenderer::s_waveformMaxZoom);
if (m_config) {
Expand All @@ -445,7 +445,7 @@ void WaveformWidgetFactory::setZoomSync(bool sync) {
return;
}

int refZoom = m_waveformWidgetHolders[0].m_waveformWidget->getZoomFactor();
double refZoom = m_waveformWidgetHolders[0].m_waveformWidget->getZoomFactor();
for (int i = 1; i < m_waveformWidgetHolders.size(); i++) {
m_waveformWidgetHolders[i].m_waveformViewer->setZoom(refZoom);
}
Expand Down Expand Up @@ -496,7 +496,7 @@ void WaveformWidgetFactory::notifyZoomChange(WWaveformViewer* viewer) {
WaveformWidgetAbstract* pWaveformWidget = viewer->getWaveformWidget();
if (pWaveformWidget != NULL && isZoomSync()) {
//qDebug() << "WaveformWidgetFactory::notifyZoomChange";
int refZoom = pWaveformWidget->getZoomFactor();
double refZoom = pWaveformWidget->getZoomFactor();

for (int i = 0; i < m_waveformWidgetHolders.size(); ++i) {
WaveformWidgetHolder& holder = m_waveformWidgetHolders[i];
Expand Down
6 changes: 3 additions & 3 deletions src/waveform/waveformwidgetfactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ class WaveformWidgetFactory : public QObject, public Singleton<WaveformWidgetFac
bool setWidgetTypeFromHandle(int handleIndex);
WaveformWidgetType::Type getType() const { return m_type;}

void setDefaultZoom(int zoom);
int getDefaultZoom() const { return m_defaultZoom;}
void setDefaultZoom(double zoom);
double getDefaultZoom() const { return m_defaultZoom;}

void setZoomSync(bool sync);
int isZoomSync() const { return m_zoomSync;}
Expand Down Expand Up @@ -151,7 +151,7 @@ class WaveformWidgetFactory : public QObject, public Singleton<WaveformWidgetFac
bool m_skipRender;
int m_frameRate;
int m_endOfTrackWarningTime;
int m_defaultZoom;
double m_defaultZoom;
bool m_zoomSync;
double m_visualGain[FilterCount];
bool m_overviewNormalized;
Expand Down
17 changes: 6 additions & 11 deletions src/widget/wwaveformviewer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,11 @@ void WWaveformViewer::mouseReleaseEvent(QMouseEvent* /*event*/) {

void WWaveformViewer::wheelEvent(QWheelEvent *event) {
if (m_waveformWidget) {
//NOTE: (vrince) to limit the zoom action area uncomment the following line
//if (event->x() > width() - m_zoomZoneWidth) {
if (event->delta() > 0) {
//qDebug() << "WaveformWidgetRenderer::wheelEvent +1";
onZoomChange(m_waveformWidget->getZoomFactor() + 1);
} else {
//qDebug() << "WaveformWidgetRenderer::wheelEvent -1";
onZoomChange(m_waveformWidget->getZoomFactor() - 1);
}
//}
if (event->delta() > 0) {
onZoomChange(m_waveformWidget->getZoomFactor() * 1.05);
} else {
onZoomChange(m_waveformWidget->getZoomFactor() / 1.05);
}
}
}

Expand Down Expand Up @@ -198,7 +193,7 @@ void WWaveformViewer::onZoomChange(double zoom) {
WaveformWidgetFactory::instance()->notifyZoomChange(this);
}

void WWaveformViewer::setZoom(int zoom) {
void WWaveformViewer::setZoom(double zoom) {
//qDebug() << "WaveformWidgetRenderer::setZoom" << zoom;
if (m_waveformWidget) {
m_waveformWidget->setZoom(zoom);
Expand Down
2 changes: 1 addition & 1 deletion src/widget/wwaveformviewer.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ private slots:
return m_waveformWidget;
}
//direct access to let factory sync/set default zoom
void setZoom(int zoom);
void setZoom(double zoom);
void setDisplayBeatGridAlpha(int alpha);
void setPlayMarkerPosition(double position);

Expand Down