Undo volume

CMakeLists.txt

@@ -33,7 +33,7 @@ pkg_check_modules(SPA REQUIRED libspa-0.2>=0.2)
 add_compile_options(
     -Wall
     -Wextra
-    -Wpedantic
+    -Wno-pedantic
     -Werror=return-type
 )
 

PROJECT_PLAN.md

@@ -1233,7 +1233,7 @@ private:
 - [x] Implement volume slider with PipeWire parameter sync
 - [x] Add mute buttons and solo functionality
 - [x] Create stereo/multi-channel level meters
-- [ ] Implement undo/redo for volume changes
+- [x] Implement undo/redo for volume changes
 - [ ] **Acceptance Criteria:** Mixer panel controls node volumes, changes persist in presets
 
 ### Milestone 6: Undo/Redo & Polish

src/gui/ClickSlider.h

@@ -0,0 +1,26 @@
+#pragma once
+
+#include <QMouseEvent>
+#include <QSlider>
+#include <QStyle>
+
+class ClickSlider : public QSlider
+{
+public:
+    using QSlider::QSlider;
+
+protected:
+    void mousePressEvent(QMouseEvent *event) override
+    {
+        if (event->button() == Qt::LeftButton) {
+            const int span = (orientation() == Qt::Horizontal) ? width() : height();
+            const int pos = (orientation() == Qt::Horizontal)
+                ? static_cast<int>(event->position().x())
+                : static_cast<int>(height() - event->position().y());
+            const int value = QStyle::sliderValueFromPosition(minimum(), maximum(), pos, span);
+            setValue(value);
+            event->accept();
+        }
+        QSlider::mousePressEvent(event);
+    }
+};

src/gui/GraphEditorWidget.cpp

@@ -15,16 +15,65 @@
 #include <QSlider>
 #include <QToolButton>
 #include <QTimer>
+#include <QUndoCommand>
+#include <QUndoStack>
 #include <QEvent>
 #include <QVBoxLayout>
 #include <QScrollArea>
 #include <QSizePolicy>
 #include <QElapsedTimer>
 #include <algorithm>
+#include <cmath>
 
 #include <QtNodes/GraphicsViewStyle>
 #include <QtNodes/NodeStyle>
 
+#include "gui/ClickSlider.h"
+
+class VolumeChangeCommand : public QUndoCommand
+{
+public:
+    VolumeChangeCommand(GraphEditorWidget *widget,
+                        uint32_t nodeId,
+                        const NodeVolumeState &previous,
+                        const NodeVolumeState &next)
+        : m_widget(widget)
+        , m_nodeId(nodeId)
+        , m_previous(previous)
+        , m_next(next)
+    {
+        setText(QString("Volume Change"));
+    }
+
+    void undo() override
+    {
+        if (m_widget) {
+            m_widget->applyVolumeState(m_nodeId, m_previous, true);
+            if (m_widget->m_volumeUndoStack) {
+                m_widget->m_undoVolumeAction->setEnabled(m_widget->m_volumeUndoStack->canUndo());
+                m_widget->m_redoVolumeAction->setEnabled(m_widget->m_volumeUndoStack->canRedo());
+            }
+        }
+    }
+
+    void redo() override
+    {
+        if (m_widget) {
+            m_widget->applyVolumeState(m_nodeId, m_next, true);
+            if (m_widget->m_volumeUndoStack) {
+                m_widget->m_undoVolumeAction->setEnabled(m_widget->m_volumeUndoStack->canUndo());
+                m_widget->m_redoVolumeAction->setEnabled(m_widget->m_volumeUndoStack->canRedo());
+            }
+        }
+    }
+
+private:
+    GraphEditorWidget *m_widget = nullptr;
+    uint32_t m_nodeId = 0;
+    NodeVolumeState m_previous{};
+    NodeVolumeState m_next{};
+};
+
 GraphEditorWidget::GraphEditorWidget(Potato::PipeWireController *controller, QWidget *parent)
     : QWidget(parent)
     , m_controller(controller)
@@ -154,6 +203,16 @@ GraphEditorWidget::GraphEditorWidget(Potato::PipeWireController *controller, QWi
             this, &GraphEditorWidget::onConnectionCreated);
     connect(m_model, &PipeWireGraphModel::connectionDeleted,
             this, &GraphEditorWidget::onConnectionDeleted);
+    connect(m_model, &PipeWireGraphModel::nodeVolumeChanged,
+            this, [this](uint32_t nodeId, const NodeVolumeState &previous, const NodeVolumeState &current) {
+                if (m_ignoreVolumeUndo) {
+                    return;
+                }
+                pushVolumeCommand(nodeId, previous, current);
+                if (m_mixerFaders.contains(nodeId)) {
+                    m_mixerLastState.insert(nodeId, current);
+                }
+            });
 
     connect(m_controller, &Potato::PipeWireController::nodeAdded,
             this, &GraphEditorWidget::onNodeAdded);
@@ -211,6 +270,28 @@ GraphEditorWidget::GraphEditorWidget(Potato::PipeWireController *controller, QWi
     });
     m_view->addAction(resetLayoutAction);
 
+    m_volumeUndoStack = new QUndoStack(this);
+    m_undoVolumeAction = new QAction(QString("Undo Volume"), m_view);
+    m_redoVolumeAction = new QAction(QString("Redo Volume"), m_view);
+    m_undoVolumeAction->setShortcut(QKeySequence::Undo);
+    m_redoVolumeAction->setShortcut(QKeySequence::Redo);
+    m_undoVolumeAction->setEnabled(false);
+    m_redoVolumeAction->setEnabled(false);
+    connect(m_undoVolumeAction, &QAction::triggered, m_volumeUndoStack, &QUndoStack::undo);
+    connect(m_redoVolumeAction, &QAction::triggered, m_volumeUndoStack, &QUndoStack::redo);
+    m_view->addAction(m_undoVolumeAction);
+    m_view->addAction(m_redoVolumeAction);
+    connect(m_volumeUndoStack, &QUndoStack::canUndoChanged, this, [this](bool canUndo) {
+        if (m_undoVolumeAction) {
+            m_undoVolumeAction->setEnabled(canUndo);
+        }
+    });
+    connect(m_volumeUndoStack, &QUndoStack::canRedoChanged, this, [this](bool canRedo) {
+        if (m_redoVolumeAction) {
+            m_redoVolumeAction->setEnabled(canRedo);
+        }
+    });
+
     auto *savePresetAction = new QAction(QString("Save Preset..."), m_view);
     connect(savePresetAction, &QAction::triggered, [this]() {
         const QString filePath = QFileDialog::getSaveFileName(this,
@@ -786,7 +867,7 @@ void GraphEditorWidget::refreshMixerStrip(uint32_t nodeId, const Potato::NodeInf
     auto *meter = new AudioLevelMeter(strip);
     meter->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
     
-    auto *fader = new QSlider(Qt::Vertical, strip);
+    auto *fader = new ClickSlider(Qt::Vertical, strip);
     fader->setRange(0, 100);
     fader->setValue(100);
     fader->setToolTip("Volume");
@@ -845,24 +926,63 @@ void GraphEditorWidget::refreshMixerStrip(uint32_t nodeId, const Potato::NodeInf
     m_mixerMutes.insert(nodeId, muteBtn);
     m_mixerSolos.insert(nodeId, soloBtn);
     m_mixerUserMute.insert(nodeId, false);
+    m_mixerLastState.insert(nodeId, NodeVolumeState{1.0f, false});
 
     connect(fader, &QSlider::valueChanged, [this, nodeId](int value) {
-        if (!m_mixerMutes.contains(nodeId)) {
+        if (!m_mixerMutes.contains(nodeId) || !m_mixerFaders.contains(nodeId)) {
             return;
         }
         const float volume = static_cast<float>(value) / 100.0f;
         const bool userMute = m_mixerUserMute.value(nodeId, false);
-        const bool soloActive = !m_mixerSoloNodes.isEmpty();
+        if (m_mixerFaders[nodeId]->isSliderDown()) {
-        const bool effectiveMute = userMute || (soloActive && !m_mixerSoloNodes.contains(nodeId));
+            if (!m_mixerStartState.contains(nodeId)) {
-        m_controller->setNodeVolume(nodeId, volume, effectiveMute);
+                const NodeVolumeState previous = m_mixerLastState.value(nodeId, NodeVolumeState{volume, userMute});
-        if (m_model) {
+                m_mixerStartState.insert(nodeId, previous);
-            m_model->setNodeVolumeState(nodeId, NodeVolumeState{volume, userMute});
             }
+        } else {
+            const NodeVolumeState previous = m_mixerLastState.value(nodeId, NodeVolumeState{volume, userMute});
+            const NodeVolumeState next{volume, userMute};
+            pushVolumeCommand(nodeId, previous, next);
+            m_mixerLastState.insert(nodeId, next);
+        }
+        applySoloState();
+    });
+
+    connect(fader, &QSlider::sliderPressed, [this, nodeId]() {
+        if (!m_mixerFaders.contains(nodeId)) {
+            return;
+        }
+        const float volume = static_cast<float>(m_mixerFaders[nodeId]->value()) / 100.0f;
+        const bool userMute = m_mixerUserMute.value(nodeId, false);
+        const NodeVolumeState previous = m_mixerLastState.value(nodeId, NodeVolumeState{volume, userMute});
+        m_mixerStartState.insert(nodeId, previous);
+    });
+
+    connect(fader, &QSlider::sliderReleased, [this, nodeId]() {
+        if (!m_mixerFaders.contains(nodeId)) {
+            return;
+        }
+        const float volume = static_cast<float>(m_mixerFaders[nodeId]->value()) / 100.0f;
+        const bool userMute = m_mixerUserMute.value(nodeId, false);
+        const NodeVolumeState previous = m_mixerStartState.value(nodeId, m_mixerLastState.value(nodeId, NodeVolumeState{volume, userMute}));
+        const NodeVolumeState next{volume, userMute};
+        m_mixerStartState.remove(nodeId);
+        pushVolumeCommand(nodeId, previous, next);
+        m_mixerLastState.insert(nodeId, next);
     });
 
     connect(muteBtn, &QToolButton::toggled, [this, nodeId](bool checked) {
+        if (!m_mixerFaders.contains(nodeId)) {
+            return;
+        }
+        const float volume = static_cast<float>(m_mixerFaders[nodeId]->value()) / 100.0f;
+        const bool previousMute = m_mixerUserMute.value(nodeId, false);
+        const NodeVolumeState previous{volume, previousMute};
+        const NodeVolumeState next{volume, checked};
         m_mixerUserMute[nodeId] = checked;
         applySoloState();
+        pushVolumeCommand(nodeId, previous, next);
+        m_mixerLastState.insert(nodeId, next);
     });
 
     connect(soloBtn, &QToolButton::toggled, [this, nodeId](bool checked) {
@@ -900,6 +1020,7 @@ void GraphEditorWidget::updateMixerState(uint32_t nodeId, const Potato::NodeInfo
     fader->blockSignals(false);
     muteBtn->blockSignals(false);
     m_mixerUserMute[nodeId] = state.mute;
+    m_mixerLastState.insert(nodeId, state);
     applySoloState();
 }
 
@@ -912,6 +1033,8 @@ void GraphEditorWidget::removeMixerStrip(uint32_t nodeId)
         m_mixerMutes.remove(nodeId);
         m_mixerSolos.remove(nodeId);
     m_mixerUserMute.remove(nodeId);
+    m_mixerStartState.remove(nodeId);
+    m_mixerLastState.remove(nodeId);
     m_mixerSoloNodes.remove(nodeId);
         strip->deleteLater();
     }
@@ -927,7 +1050,49 @@ void GraphEditorWidget::applySoloState()
         const bool effectiveMute = userMute || (soloActive && !m_mixerSoloNodes.contains(nodeId));
         m_controller->setNodeVolume(nodeId, volume, effectiveMute);
         if (m_model) {
-            m_model->setNodeVolumeState(nodeId, NodeVolumeState{volume, userMute});
+            m_model->setNodeVolumeState(nodeId, NodeVolumeState{volume, userMute}, false);
         }
     }
 }
+
+void GraphEditorWidget::applyVolumeState(uint32_t nodeId, const NodeVolumeState &state, bool updateMixer)
+{
+    Q_UNUSED(updateMixer)
+    m_ignoreVolumeUndo = true;
+    if (m_model) {
+        m_model->setNodeVolumeState(nodeId, state, false);
+    }
+    m_mixerUserMute[nodeId] = state.mute;
+    if (m_mixerFaders.contains(nodeId)) {
+        m_mixerFaders[nodeId]->blockSignals(true);
+        m_mixerFaders[nodeId]->setValue(static_cast<int>(state.volume * 100.0f));
+        m_mixerFaders[nodeId]->blockSignals(false);
+    }
+    if (m_mixerMutes.contains(nodeId)) {
+        m_mixerMutes[nodeId]->blockSignals(true);
+        m_mixerMutes[nodeId]->setChecked(state.mute);
+        m_mixerMutes[nodeId]->blockSignals(false);
+    }
+    m_mixerLastState.insert(nodeId, state);
+    m_mixerStartState.remove(nodeId);
+    applySoloState();
+    m_ignoreVolumeUndo = false;
+}
+
+void GraphEditorWidget::pushVolumeCommand(uint32_t nodeId, const NodeVolumeState &previous, const NodeVolumeState &next)
+{
+    if (!m_volumeUndoStack || m_ignoreVolumeUndo) {
+        return;
+    }
+    const bool changedVolume = qAbs(previous.volume - next.volume) > 0.0001f;
+    if (!changedVolume && previous.mute == next.mute) {
+        return;
+    }
+    m_volumeUndoStack->push(new VolumeChangeCommand(this, nodeId, previous, next));
+    if (m_undoVolumeAction) {
+        m_undoVolumeAction->setEnabled(m_volumeUndoStack->canUndo());
+    }
+    if (m_redoVolumeAction) {
+        m_redoVolumeAction->setEnabled(m_volumeUndoStack->canRedo());
+    }
+}

src/gui/GraphEditorWidget.h

@@ -23,11 +23,16 @@ class QToolButton;
 class QSplitter;
 class QTabWidget;
 class PresetManager;
+class QUndoStack;
+class VolumeChangeCommand;
+class QAction;
 
 class GraphEditorWidget : public QWidget
 {
     Q_OBJECT
 
+    friend class VolumeChangeCommand;
+
 public:
     explicit GraphEditorWidget(Potato::PipeWireController *controller, QWidget *parent = nullptr);
 
@@ -52,6 +57,8 @@ private:
     void updateMixerState(uint32_t nodeId, const Potato::NodeInfo &node);
     void removeMixerStrip(uint32_t nodeId);
     void applySoloState();
+    void applyVolumeState(uint32_t nodeId, const NodeVolumeState &state, bool updateMixer);
+    void pushVolumeCommand(uint32_t nodeId, const NodeVolumeState &previous, const NodeVolumeState &next);
     void handleLinkRemoved(uint32_t linkId);
     bool isMeterNode(uint32_t nodeId) const;
     int activeLinkCount(uint32_t nodeId) const;
@@ -75,6 +82,8 @@ private:
     QMap<uint32_t, QToolButton*> m_mixerMutes;
     QMap<uint32_t, QToolButton*> m_mixerSolos;
     QMap<uint32_t, bool> m_mixerUserMute;
+    QMap<uint32_t, NodeVolumeState> m_mixerStartState;
+    QMap<uint32_t, NodeVolumeState> m_mixerLastState;
     QSet<uint32_t> m_mixerSoloNodes;
 
     QSet<QString> m_ignoreCreate;
@@ -100,4 +109,8 @@ private:
     qint64 m_meterProfileMax = 0;
     int m_meterProfileFrames = 0;
     PresetManager *m_presetManager = nullptr;
+    QUndoStack *m_volumeUndoStack = nullptr;
+    QAction *m_undoVolumeAction = nullptr;
+    QAction *m_redoVolumeAction = nullptr;
+    bool m_ignoreVolumeUndo = false;
 };

src/gui/PipeWireGraphModel.cpp

@@ -18,6 +18,7 @@
 #include <QFileInfo>
 #include <QHBoxLayout>
 #include <QSlider>
+#include "gui/ClickSlider.h"
 #include <QSizePolicy>
 #include <QToolButton>
 #include <QWidget>
@@ -26,6 +27,7 @@
 #include <QtNodes/StyleCollection>
 
 #include <algorithm>
+#include <cmath>
 #include <unordered_set>
 #include <vector>
 
@@ -137,7 +139,7 @@ QWidget *PipeWireGraphModel::nodeWidget(QtNodes::NodeId nodeId) const
     muteButton->setFixedSize(20, 20);
     muteButton->setToolTip(QString("Mute"));
 
-    auto *slider = new QSlider(Qt::Horizontal, widget);
+    auto *slider = new ClickSlider(Qt::Horizontal, widget);
     slider->setRange(0, 100);
     slider->setValue(100);
     slider->setFixedHeight(18);
@@ -155,8 +157,10 @@ QWidget *PipeWireGraphModel::nodeWidget(QtNodes::NodeId nodeId) const
             return;
         }
         const float volume = static_cast<float>(slider->value()) / 100.0f;
-        m_controller->setNodeVolume(pipewireId, volume, muteButton->isChecked());
+        const NodeVolumeState state{volume, muteButton->isChecked()};
-        m_nodeVolumeState.insert(pipewireId, NodeVolumeState{volume, muteButton->isChecked()});
+        m_controller->setNodeVolume(pipewireId, volume, state.mute);
+        auto *self = const_cast<PipeWireGraphModel*>(this);
+        self->setNodeVolumeState(pipewireId, state, true);
     };
 
     QObject::connect(slider, &QSlider::valueChanged, widget, [applyVolume](int) { applyVolume(); });
@@ -802,36 +806,14 @@ void PipeWireGraphModel::applyVolumeStates(const QHash<QString, NodeVolumeState>
             continue;
         }
         const NodeVolumeState state = states.value(node.stableId);
-        m_nodeVolumeState.insert(node.id, state);
+        setNodeVolumeState(node.id, state, false);
         m_controller->setNodeVolume(node.id, state.volume, state.mute);
-
-        auto nodeIt = m_pwToNode.find(node.id);
-        if (nodeIt == m_pwToNode.end()) {
-            continue;
-        }
-        auto widgetIt = m_nodeWidgets.find(nodeIt->second);
-        if (widgetIt == m_nodeWidgets.end()) {
-            continue;
-        }
-        QWidget *widget = widgetIt->second;
-        if (!widget) {
-            continue;
-        }
-        if (auto *slider = widget->findChild<QSlider*>()) {
-            slider->blockSignals(true);
-            slider->setValue(static_cast<int>(state.volume * 100.0f));
-            slider->blockSignals(false);
-        }
-        if (auto *button = widget->findChild<QToolButton*>()) {
-            button->blockSignals(true);
-            button->setChecked(state.mute);
-            button->blockSignals(false);
-        }
     }
 }
 
-void PipeWireGraphModel::setNodeVolumeState(uint32_t nodeId, const NodeVolumeState &state)
+void PipeWireGraphModel::setNodeVolumeState(uint32_t nodeId, const NodeVolumeState &state, bool notify)
 {
+    const NodeVolumeState previous = m_nodeVolumeState.value(nodeId, NodeVolumeState{});
     m_nodeVolumeState.insert(nodeId, state);
 
     auto nodeIt = m_pwToNode.find(nodeId);
@@ -858,6 +840,13 @@ void PipeWireGraphModel::setNodeVolumeState(uint32_t nodeId, const NodeVolumeSta
         button->setChecked(state.mute);
         button->blockSignals(false);
     }
+
+    if (notify) {
+        const bool changedVolume = std::abs(previous.volume - state.volume) > 0.0001f;
+        if (changedVolume || previous.mute != state.mute) {
+            Q_EMIT nodeVolumeChanged(nodeId, previous, state);
+        }
+    }
 }
 
 bool PipeWireGraphModel::nodeVolumeState(uint32_t nodeId, NodeVolumeState &state) const

src/gui/PipeWireGraphModel.h

@@ -81,9 +81,12 @@ public:
     void applyLayoutJson(const QJsonObject &root);
     QHash<QString, NodeVolumeState> volumeStates() const;
     void applyVolumeStates(const QHash<QString, NodeVolumeState> &states);
-    void setNodeVolumeState(uint32_t nodeId, const NodeVolumeState &state);
+    void setNodeVolumeState(uint32_t nodeId, const NodeVolumeState &state, bool notify = true);
     bool nodeVolumeState(uint32_t nodeId, NodeVolumeState &state) const;
 
+signals:
+    void nodeVolumeChanged(uint32_t nodeId, const NodeVolumeState &previous, const NodeVolumeState &current);
+
 private:
     QWidget *nodeWidget(QtNodes::NodeId nodeId) const;
     QtNodes::ConnectionId connectionFromPipeWire(const Potato::LinkInfo &link, bool *ok) const;