Zooming in waveform using scrollwheel is broken #11626
Labels
Milestone
Comments
|
I cannot confirm this with Ubuntu Focal. Is this a windows only thing? |
|
Can confirm this issue with 2.4.0 (592c0c7) using Gentoo Linux. Scrollwheel doesn't zoom in or out when mouse pointer is positioned inside the waveform. So it seems that not only Windows is affected. |
|
The issue is also there on openSUSE Tumbleweed. |
|
agree that seems a likely candidate |
|
Yeah, just something I missed. I expect this will be easy to implement. I'll have a look (not today though). |
|
No worries, take your time. Thank you for taking care of it. |
|
On macOS it works fine (tried both with two-fingers on the trackpad and with an external mouse). And looking at the code, I don't see anything that explains this. I forward all events from the openglwindow to the widget... Maybe I can figure this out if someone with a system where it doesn't work can add some logging and send me the results. In src/widget/openglwindow.cpp add the following: At the top of the file: and as first line of bool OpenGLWindow::event(QEvent* pEv) { and run with ./mixxx --logLevel debug |
|
Ah, and in src/mixxxmainwindow.cpp: |
|
Nevermind I think I built the wrong commit Log output with your patch
|
|
You only added logging in mixxxmainwindow.cpp, right? Not in openglwindow.cpp The wheel events are there alright, and with an angleDelta that should definitely have effect here: Can you uncomment the qDebug() statements in WWaveformViewer::onZoomChange, WWaveformViewer::setZoom and WaveformWidgetRenderer::setZoom as well? (and while at it, change the text of the first two, they refer to the wrong class). |
Actually I did, but I encountered some other issues that resulted in all non-cpu waveforms to be disabled so I don't think I was able to test properly. Also the issue did not occur, with those waveforms. I'll try investigate whats wrong. |
|
Ok, I fixed the issue with the non-cpu waveforms. Forgot to Patch used to get debug log
diff --git a/src/mixxxmainwindow.cpp b/src/mixxxmainwindow.cpp
index bfaf258bcf..225dde139b 100644
--- a/src/mixxxmainwindow.cpp
+++ b/src/mixxxmainwindow.cpp
@@ -1129,6 +1129,9 @@ bool MixxxMainWindow::loadConfiguredSkin() {
}
bool MixxxMainWindow::eventFilter(QObject* obj, QEvent* event) {
+ if (event->type() == QEvent::Wheel) {
+ qDebug() << "MixxxMainWindow::eventFilter" << obj << event;
+ }
if (event->type() == QEvent::ToolTip) {
// always show tooltips in the preferences window
QWidget* activeWindow = QApplication::activeWindow();
diff --git a/src/waveform/renderers/waveformwidgetrenderer.cpp b/src/waveform/renderers/waveformwidgetrenderer.cpp
index 1b3d618cba..95849730c1 100644
--- a/src/waveform/renderers/waveformwidgetrenderer.cpp
+++ b/src/waveform/renderers/waveformwidgetrenderer.cpp
@@ -387,7 +387,7 @@ void WaveformWidgetRenderer::setup(
}
void WaveformWidgetRenderer::setZoom(double zoom) {
- //qDebug() << "WaveformWidgetRenderer::setZoom" << zoom;
+ qDebug() << "WaveformWidgetRenderer::setZoom" << zoom;
m_zoomFactor = math_clamp<double>(zoom, s_waveformMinZoom, s_waveformMaxZoom);
}
diff --git a/src/widget/openglwindow.cpp b/src/widget/openglwindow.cpp
index 74d40eeb4b..92175579e6 100644
--- a/src/widget/openglwindow.cpp
+++ b/src/widget/openglwindow.cpp
@@ -2,6 +2,7 @@
#include <QApplication>
#include <QResizeEvent>
+#include <QDebug>
#include "moc_openglwindow.cpp"
#include "widget/tooltipqopengl.h"
@@ -62,6 +63,7 @@ bool OpenGLWindow::event(QEvent* pEv) {
// gets called recursive, potentially resulting in infinite recursion
// and a stack overflow. The boolean m_handlingEvent protects against
// this recursion.
+ qDebug() << pEv;
const auto t = pEv->type();
bool result = QOpenGLWindow::event(pEv);
diff --git a/src/widget/wwaveformviewer.cpp b/src/widget/wwaveformviewer.cpp
index 93c5f9d6e9..090bcb00a5 100644
--- a/src/widget/wwaveformviewer.cpp
+++ b/src/widget/wwaveformviewer.cpp
@@ -227,14 +227,14 @@ void WWaveformViewer::slotLoadingTrack(TrackPointer pNewTrack, TrackPointer pOld
}
void WWaveformViewer::onZoomChange(double zoom) {
- //qDebug() << "WaveformWidgetRenderer::onZoomChange" << this << zoom;
+ qDebug() << "WWaveformViewer::onZoomChange" << this << zoom;
setZoom(zoom);
// notify back the factory to sync zoom if needed
WaveformWidgetFactory::instance()->notifyZoomChange(this);
}
void WWaveformViewer::setZoom(double zoom) {
- //qDebug() << "WaveformWidgetRenderer::setZoom" << zoom;
+ qDebug() << "WWaveformViewer::setZoom" << zoom;
if (m_waveformWidget) {
m_waveformWidget->setZoom(zoom);
}
Debug log
|
|
Thanks @Swiftb0y |
|
@Swiftb0y (or anyone with the issue), can you try this patch: diff --git a/src/widget/wglwidgetqopengl.cpp b/src/widget/wglwidgetqopengl.cpp
index 25a4a6ef2a..2a9bb736df 100644
--- a/src/widget/wglwidgetqopengl.cpp
+++ b/src/widget/wglwidgetqopengl.cpp
@@ -54,6 +54,10 @@ void WGLWidget::resizeEvent(QResizeEvent* event) {
QWidget::resizeEvent(event);
}
+void WGLWidget::wheelEvent(QWheelEvent* event) {
+ event->ignore();
+}
+
bool WGLWidget::isContextValid() const {
return m_pOpenGLWindow && m_pOpenGLWindow->context() && m_pOpenGLWindow->context()->isValid();
}
diff --git a/src/widget/wglwidgetqopengl.h b/src/widget/wglwidgetqopengl.h
index 78b4cd48e8..2dec72c7c5 100644
--- a/src/widget/wglwidgetqopengl.h
+++ b/src/widget/wglwidgetqopengl.h
@@ -41,6 +41,7 @@ class WGLWidget : public QWidget {
protected:
void showEvent(QShowEvent* event) override;
void resizeEvent(QResizeEvent* event) override;
+ void wheelEvent(QWheelEvent* event) override;
QPaintDevice* paintDevice();
|
|
Did a quick test with this patch applied to 5e06563 (2.4 branch), unfortunately with no luck. It looks like only GL/GLSL based waveforms are affected... if I try a different one without GL (like RGB or HSV for example), the scrollwheel works as expected (with significant more CPU consumption, of course). |
|
yep, can confirm the same. |
|
Urgh... Ok, final attempt. Please test with any of the allshader waveform types. If this still doesn't work, I will install one of the reported linux distros to debug this. diff --git a/src/waveform/widgets/allshader/waveformwidget.cpp b/src/waveform/widgets/allshader/waveformwidget.cpp
index 37d67addaa..76e38834a5 100644
--- a/src/waveform/widgets/allshader/waveformwidget.cpp
+++ b/src/waveform/widgets/allshader/waveformwidget.cpp
@@ -1,5 +1,7 @@
#include "waveform/widgets/allshader/waveformwidget.h"
+#include <QApplication>
+
#include "moc_waveformwidget.cpp"
#include "waveform/renderers/allshader/waveformrendererabstract.h"
@@ -52,3 +54,8 @@ void WaveformWidget::resizeGL(int w, int h) {
renderer->allshaderWaveformRenderer()->resizeGL(w, h);
}
}
+
+void WaveformWidget::wheelEvent(QWheelEvent* event) {
+ QApplication::sendEvent(parentWidget(), event);
+ event->accept();
+}
diff --git a/src/waveform/widgets/allshader/waveformwidget.h b/src/waveform/widgets/allshader/waveformwidget.h
index 656df91998..b55ef35cd9 100644
--- a/src/waveform/widgets/allshader/waveformwidget.h
+++ b/src/waveform/widgets/allshader/waveformwidget.h
@@ -24,6 +24,6 @@ class allshader::WaveformWidget : public ::WGLWidget,
virtual WGLWidget* getGLWidget() override {
return this;
}
-
private:
+ void wheelEvent(QWheelEvent* event) override;
};
diff --git a/src/widget/wwaveformviewer.cpp b/src/widget/wwaveformviewer.cpp
index 93c5f9d6e9..a9ae7a65ba 100644
--- a/src/widget/wwaveformviewer.cpp
+++ b/src/widget/wwaveformviewer.cpp
@@ -187,7 +187,7 @@ void WWaveformViewer::wheelEvent(QWheelEvent* event) {
if (m_waveformWidget) {
if (event->angleDelta().y() > 0) {
onZoomChange(m_waveformWidget->getZoomFactor() / 1.05);
- } else {
+ } else if (event->angleDelta().y() < 0) {
onZoomChange(m_waveformWidget->getZoomFactor() * 1.05);
}
} |
|
That diff does not fix the issue for me on Ubuntu :/ |
|
If I add a wheelevent handler to glsl waveform widget directly it works. I will work up a patch |
ywwg
added a commit
to ywwg/mixxx
that referenced
this issue
Jun 11, 2023
weird, this change definitely fixed it for me... I tried all of the waveform types. @MarcPlace @Swiftb0y Can you add a qDebug() statement to WGLWidget::wheelEvent to see if it's getting called? |
|
@ywwg I don't see how adding the wheelEvent handler to WGLWidget (your fix 11645) instead of to the derived allshader::WaveformWidget::wheelEvent(QWheelEvent* event) (my "final attempt" patch) can make a difference. Are you 100% sure you applied the patch and that you tried with one of the allshader waveform types? The problem I have with your fix 11645 is that it adds code that is very specific for the waveform widgets to the generic WGLWidget. I would really prefer to see this handled in the waveform widget class instead (in which case it should also be added to GLWaveformWidgetAbstract btw) |
Yes, its definitely getting called |
|
I missed the final attempt one. Sorry for the confusion. Give me a second. |
|
Good news, your "final attempt" patch also works. #11645 also works, but I can't judge the quality of either patches. |
|
Great. @ywwg can you please try again with the wheelEvent override in the waveform widget class? |
|
adding it to the allshaders/waveformwidget file fixes the issue for those, but zoom is not working for the waveforms I use, rgb (glsl). That's why I thought it better to apply the fix farther upstream. |
ah yes I see what you mean -- we don't want to zoom in on spinnies :) |
|
yeah putting the fix in glwaveformwidgetabstract fixes it for the glsl waveforms, but not the allshaders ones -- so probably this fix needs to go in more than one place, which seems fine. |
ywwg
added a commit
to ywwg/mixxx
that referenced
this issue
Jun 12, 2023
Merged
|
sorry, had to create a new PR. I am getting a little lost with all the different objects! please let me know if I've got it wrong again :) |
|
Yes, this looks good, thanks! On a sidenote and out of curiosity, any particular reason you prefer RGB (glsl) over RGB (allshaders) (glsl) or RGB L/R (allshaders) (glsl) ? |
|
Thank you very much for digging into this 👍 |
|
I just like the way it looks more haha |
|
Fair enough :-) I would make sense (and be trivial) to add an allshaders version of the original RGB (glsl) (which is not so much about using glsl but more about using a texture to transfer the waveform data and do the conversion of waveform data to colored line values in the shader instead of in the CPU), which would combine the original RGB (glsl) code for the actual waveform with the allshaders code for all the overlays. |
|
Can confirm that #11650 did the trick also for me. Works perfect now on all waveforms. Thank you. 👍 |
|
Also for me it's fixed now on Windows 10. Thanks all of you in this thread for the help! 😄 |
ywwg
added a commit
to ywwg/mixxx
that referenced
this issue
Jul 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug Description
When using 2.3.5 it was possible to use the scrollwheel to zoom the waveform in and out. I'm unable to do the same in 2.4.0 (the scrollwheel simply doesn't do anything when the mouse is positioned over the waveform). When reinstalling 2.3.5 it was working fine again.
Version
2.4.0
OS
Windows 10
The text was updated successfully, but these errors were encountered: