Eliminate unnecessary widget renders from control changes.

Instead of re-rendering on every control change (via
WWidget::onConnectedControlChanged), check for changes in what we would render
first.

Updated: WDisplay, WKnob, WKnobComposed, WSliderComposed, WStatusLight
Left unchanged: WWaveformViewer, WPushButton, WOverview, WVuMeter

9 files changed, 82 insertions, 35 deletions

diff --git a/src/widget/wdisplay.cpp b/src/widget/wdisplay.cpp
index 81177b3d62..3942efa2e2 100644
--- a/src/widget/wdisplay.cpp
+++ b/src/widget/wdisplay.cpp
@@ -27,6 +27,7 @@
 
 WDisplay::WDisplay(QWidget * parent)
         : WWidget(parent),
+          m_iCurrentPixmap(0),
           m_pPixmapBack(NULL),
           m_bDisabledLoaded(false) {
     setPositions(0);
@@ -112,7 +113,7 @@ void WDisplay::setPixmap(QVector<PaintablePointer>* pPixmaps, int iPos,
     }
 }
 
-int WDisplay::getActivePixmapIndex() const {
+int WDisplay::getPixmapForParameter(double dParameter) const {
     // When there are an even number of pixmaps by convention we want a value of
     // 0.5 to align to the lower of the two middle pixmaps. In Mixxx < 1.12.0 we
     // accomplished this by the below formula:
@@ -149,7 +150,16 @@ int WDisplay::getActivePixmapIndex() const {
     // Subtracting an epsilon prevents out of bound values at the end of the
     // range and biases the middle value towards the lower of the 2 center
     // pixmaps when there are an even number of pixmaps.
-    return static_cast<int>(getControlParameterDisplay() * numPixmaps() - 0.00001);
+    return static_cast<int>(dParameter * numPixmaps() - 0.00001);
+}
+
+void WDisplay::onConnectedControlChanged(double dParameter, double dValue) {
+    Q_UNUSED(dValue);
+    int pixmap = getPixmapForParameter(dParameter);
+    if (pixmap != m_iCurrentPixmap) {
+        // paintEvent updates m_iCurrentPixmap.
+        update();
+    }
 }
 
 void WDisplay::paintEvent(QPaintEvent* ) {
@@ -171,7 +181,11 @@ void WDisplay::paintEvent(QPaintEvent* ) {
         return;
     }
 
-    int idx = getActivePixmapIndex();
+    int idx = getPixmapForParameter(getControlParameterDisplay());
+
+    // onConnectedControlChanged uses this to detect no-ops but it does not
+    // clamp so don't clamp.
+    m_iCurrentPixmap = idx;
 
     // Clamp active pixmap index to valid ranges.
     if (idx < 0) {
diff --git a/src/widget/wdisplay.h b/src/widget/wdisplay.h
index 5e27f53fa3..cd4a172f0e 100644
--- a/src/widget/wdisplay.h
+++ b/src/widget/wdisplay.h
@@ -35,6 +35,8 @@ class WDisplay : public WWidget {
 
     void setup(QDomNode node, const SkinContext& context);
 
+    void onConnectedControlChanged(double dParameter, double dValue);
+
   protected:
     void paintEvent(QPaintEvent*);
 
@@ -50,7 +52,9 @@ class WDisplay : public WWidget {
 
     void setPositions(int iNoPos);
 
-    int getActivePixmapIndex() const;
+    int getPixmapForParameter(double dParameter) const;
+
+    int m_iCurrentPixmap;
 
     // Free existing pixmaps.
     void resetPositions();
diff --git a/src/widget/wknobcomposed.cpp b/src/widget/wknobcomposed.cpp
index 6a449dd46b..37019757b7 100644
--- a/src/widget/wknobcomposed.cpp
+++ b/src/widget/wknobcomposed.cpp
@@ -5,6 +5,7 @@
 
 WKnobComposed::WKnobComposed(QWidget* pParent)
         : WWidget(pParent),
+          m_dCurrentAngle(140.0),
           m_dMinAngle(-230.0),
           m_dMaxAngle(50.0) {
 }
@@ -59,6 +60,19 @@ void WKnobComposed::setPixmapKnob(const QString& filename) {
     }
 }
 
+void WKnobComposed::onConnectedControlChanged(double dParameter, double dValue) {
+    Q_UNUSED(dValue);
+    // dParameter is in the range [0, 1].
+    double angle = m_dMinAngle + (m_dMaxAngle - m_dMinAngle) * dParameter;
+
+    // TODO(rryan): What's a good epsilon? Should it be dependent on the min/max
+    // angle range? Right now it's just 1/100th of a degree.
+    if (fabs(angle - m_dCurrentAngle) > 0.01) {
+        // paintEvent updates m_dCurrentAngle
+        update();
+    }
+}
+
 void WKnobComposed::paintEvent(QPaintEvent* e) {
     Q_UNUSED(e);
     QStyleOption option;
@@ -75,11 +89,10 @@ void WKnobComposed::paintEvent(QPaintEvent* e) {
     if (!m_pKnob.isNull() && !m_pKnob->isNull()) {
         p.translate(width() / 2.0, height() / 2.0);
 
-        // Value is in the range [0, 1].
-        double value = getControlParameterDisplay();
-
-        double angle = m_dMinAngle + (m_dMaxAngle - m_dMinAngle) * value;
-        p.rotate(angle);
+        // We update m_dCurrentAngle since onConnectedControlChanged uses it for
+        // no-op detection.
+        m_dCurrentAngle = m_dMinAngle + (m_dMaxAngle - m_dMinAngle) * getControlParameterDisplay();
+        p.rotate(m_dCurrentAngle);
 
         m_pKnob->draw(-m_pKnob->width() / 2.0, -m_pKnob->height() / 2.0, &p);
     }
diff --git a/src/widget/wknobcomposed.h b/src/widget/wknobcomposed.h
index e941ad150a..776120ad99 100644
--- a/src/widget/wknobcomposed.h
+++ b/src/widget/wknobcomposed.h
@@ -20,6 +20,8 @@ class WKnobComposed : public WWidget {
 
     void setup(QDomNode node, const SkinContext& context);
 
+    void onConnectedControlChanged(double dParameter, double dValue);
+
   protected:
     void wheelEvent(QWheelEvent *e);
     void mouseMoveEvent(QMouseEvent *e);
@@ -32,6 +34,7 @@ class WKnobComposed : public WWidget {
     void setPixmapBackground(const QString& filename, Paintable::DrawMode mode);
     void setPixmapKnob(const QString& filename);
 
+    double m_dCurrentAngle;
     PaintablePointer m_pKnob;
     PaintablePointer m_pPixmapBack;
     KnobEventHandler<WKnobComposed> m_handler;
diff --git a/src/widget/wpushbutton.cpp b/src/widget/wpushbutton.cpp
index e75f992d61..f8c097525f 100644
--- a/src/widget/wpushbutton.cpp
+++ b/src/widget/wpushbutton.cpp
@@ -227,6 +227,10 @@ void WPushButton::onConnectedControlChanged(double dParameter, double dValue) {
     if (m_iNoStates == 1) {
         m_bPressed = (dValue == 1.0);
     }
+
+    // Since we expect button connections to not change at high frequency we
+    // don't try to detect whether things have changed for WPushButton, we just
+    // re-render.
     update();
 }
 
diff --git a/src/widget/wslidercomposed.cpp b/src/widget/wslidercomposed.cpp
index e509ea04c3..c4ad9b542f 100644
--- a/src/widget/wslidercomposed.cpp
+++ b/src/widget/wslidercomposed.cpp
@@ -221,6 +221,7 @@ void WSliderComposed::paintEvent(QPaintEvent *) {
 void WSliderComposed::resizeEvent(QResizeEvent* pEvent) {
 
     m_dOldValue = -1;
+    m_iPos = -1;
     // Re-calculate m_iPos based on our new width/height.
     onConnectedControlChanged(getControlParameter(), 0);
 }
@@ -229,7 +230,15 @@ void WSliderComposed::onConnectedControlChanged(double dParameter, double) {
     // WARNING: The second parameter to this method is unused and called with
     // invalid values in parts of WSliderComposed. Do not use it unless you fix
     // this.
-    if (!m_bDrag && m_dOldValue != dParameter) {
+
+    // We don't update slider values while you're dragging them. This way you
+    // don't have to "fight" with a controller that is also changing the
+    // control.
+    if (m_bDrag) {
+        return;
+    }
+
+    if (m_dOldValue != dParameter) {
         m_dOldValue = dParameter;
 
         // Calculate handle position
@@ -237,14 +246,22 @@ void WSliderComposed::onConnectedControlChanged(double dParameter, double) {
             dParameter = 1.0 - dParameter;
         }
         int sliderLength = m_bHorizontal ? width() : height();
-        m_iPos = static_cast<int>(dParameter * (sliderLength - m_iHandleLength));
 
-        if (m_iPos > (sliderLength - m_iHandleLength)) {
-            m_iPos = sliderLength - m_iHandleLength;
-        } else if (m_iPos < 0) {
-            m_iPos = 0;
+        int newPos = static_cast<int>(dParameter * (sliderLength - m_iHandleLength));
+        if (newPos > (sliderLength - m_iHandleLength)) {
+            newPos = sliderLength - m_iHandleLength;
+        } else if (newPos < 0) {
+            newPos = 0;
+        }
+
+        // Check a second time for no-ops. It's possible the parameter changed
+        // but the visible pixmap didn't. Only update() the widget if we're
+        // really sure we need to since this involves painting ALL of its
+        // parents.
+        if (newPos != m_iPos) {
+            m_iPos = newPos;
+            update();
         }
-        update();
     }
 }
 
diff --git a/src/widget/wstatuslight.cpp b/src/widget/wstatuslight.cpp
index 32817df906..23ccfdf08f 100644
--- a/src/widget/wstatuslight.cpp
+++ b/src/widget/wstatuslight.cpp
@@ -120,22 +120,22 @@ void WStatusLight::onConnectedControlChanged(double dParameter, double dValue) {
     // Enums are not currently represented using parameter space so it doesn't
     // make sense to use the parameter here yet.
     Q_UNUSED(dParameter);
-    int val = static_cast<int>(dValue);
-    if (m_iPos == val) {
-        return;
-    }
+    int newPos = static_cast<int>(dValue);
 
     if (m_pixmaps.size() == 2) {
         // original behavior for two-state lights: any non-zero value is "on"
-        m_iPos = val > 0 ? 1 : 0;
-        update();
-    } else if (val < m_pixmaps.size() && val >= 0) {
-        // multi-state behavior: values must be correct
-        m_iPos = val;
-        update();
+        newPos = newPos > 0 ? 1 : 0;
+    } else if (newPos < m_pixmaps.size() && newPos >= 0) {
+        // multi-state behavior: values lie within the correct ranges
     } else {
         qDebug() << "Warning: wstatuslight asked for invalid position:"
-                 << val << "max val:" << m_pixmaps.size()-1;
+                 << newPos << "max val:" << m_pixmaps.size()-1;
+        return;
+    }
+
+    if (newPos != m_iPos) {
+        m_iPos = newPos;
+        update();
     }
 }
 
diff --git a/src/widget/wwidget.cpp b/src/widget/wwidget.cpp
index 21e1049861..6afbda6748 100644
--- a/src/widget/wwidget.cpp
+++ b/src/widget/wwidget.cpp
@@ -35,12 +35,6 @@ WWidget::~WWidget() {
     delete m_pTouchShift;
 }
 
-void WWidget::onConnectedControlChanged(double dParameter, double dValue) {
-    Q_UNUSED(dParameter);
-    Q_UNUSED(dValue);
-    update();
-}
-
 bool WWidget::touchIsRightButton() {
     return (m_pTouchShift->get() != 0.0);
 }
diff --git a/src/widget/wwidget.h b/src/widget/wwidget.h
index 46fc969ace..f36dbdbf6a 100644
--- a/src/widget/wwidget.h
+++ b/src/widget/wwidget.h
@@ -45,8 +45,6 @@ class WWidget : public QWidget, public WBaseWidget {
 
     Q_PROPERTY(double value READ getControlParameterDisplay);
 
-    virtual void onConnectedControlChanged(double dParameter, double dValue);
-
   protected:
     bool touchIsRightButton();
     bool event(QEvent* e);