feat: Implement better and more complete undo/redo stack (#1433)

This PR aims to implement a more complete undo/redo stack that, unlike
the old one, also supports undoing insertions, deletions and resize
operations

lib/libimhex/source/api/imhex_api.cpp

@@ -205,12 +205,19 @@ namespace hex {
 
     namespace ImHexApi::Bookmarks {
 
-        void add(Region region, const std::string &name, const std::string &comment, u32 color) {
-            EventManager::post<RequestAddBookmark>(region, name, comment, color);
+        u64 add(Region region, const std::string &name, const std::string &comment, u32 color) {
+            u64 id = 0;
+            EventManager::post<RequestAddBookmark>(region, name, comment, color, &id);
+
+            return id;
         }
 
-        void add(u64 address, size_t size, const std::string &name, const std::string &comment, u32 color) {
-            add(Region { address, size }, name, comment, color);
+        u64 add(u64 address, size_t size, const std::string &name, const std::string &comment, u32 color) {
+            return add(Region { address, size }, name, comment, color);
+        }
+
+        void remove(u64 id) {
+            EventManager::post<RequestRemoveBookmark>(id);
         }
 
     }

lib/libimhex/source/helpers/patches.cpp

@@ -2,22 +2,108 @@
 
 #include <hex/helpers/utils.hpp>
 
+#include <hex/providers/provider.hpp>
+
 #include <cstring>
 #include <string_view>
 
+
 namespace hex {
 
-    static void pushStringBack(std::vector<u8> &buffer, const std::string &string) {
-        std::copy(string.begin(), string.end(), std::back_inserter(buffer));
+    namespace {
+
+        class PatchesGenerator : public hex::prv::Provider {
+        public:
+            explicit PatchesGenerator() = default;
+            ~PatchesGenerator() override = default;
+
+            [[nodiscard]] bool isAvailable() const override { return true; }
+            [[nodiscard]] bool isReadable()  const override { return true; }
+            [[nodiscard]] bool isWritable()  const override { return true; }
+            [[nodiscard]] bool isResizable() const override { return true; }
+            [[nodiscard]] bool isSavable()   const override { return false; }
+            [[nodiscard]] bool isSavableAsRecent() const override { return false; }
+
+            [[nodiscard]] bool open() override { return true; }
+            void close() override { }
+
+            void readRaw(u64 offset, void *buffer, size_t size) override {
+                hex::unused(offset, buffer, size);
+            }
+
+            void writeRaw(u64 offset, const void *buffer, size_t size) override {
+                for (u64 i = 0; i < size; i += 1)
+                    this->m_patches[offset] = static_cast<const u8*>(buffer)[i];
+            }
+
+            [[nodiscard]] size_t getActualSize() const override {
+                if (this->m_patches.empty())
+                    return 0;
+                else
+                    return this->m_patches.rbegin()->first;
+            }
+
+            void resizeRaw(size_t newSize) override {
+                hex::unused(newSize);
+            }
+
+            void insertRaw(u64 offset, size_t size) override {
+                std::vector<std::pair<u64, u8>> patchesToMove;
+
+                for (auto &[address, value] : this->m_patches) {
+                    if (address > offset)
+                        patchesToMove.emplace_back(address, value);
+                }
+
+                for (const auto &[address, value] : patchesToMove)
+                    this->m_patches.erase(address);
+                for (const auto &[address, value] : patchesToMove)
+                    this->m_patches.insert({ address + size, value });
+            }
+
+            void removeRaw(u64 offset, size_t size) override {
+                std::vector<std::pair<u64, u8>> patchesToMove;
+
+                for (auto &[address, value] : this->m_patches) {
+                    if (address > offset)
+                        patchesToMove.emplace_back(address, value);
+                }
+
+                for (const auto &[address, value] : patchesToMove)
+                    this->m_patches.erase(address);
+                for (const auto &[address, value] : patchesToMove)
+                    this->m_patches.insert({ address - size, value });
+            }
+
+            [[nodiscard]] std::string getName() const override {
+                return "";
+            }
+
+            [[nodiscard]] std::string getTypeName() const override { return ""; }
+
+            const std::map<u64, u8>& getPatches() const {
+                return this->m_patches;
+            }
+        private:
+            std::map<u64, u8> m_patches;
+        };
+
+
+        void pushStringBack(std::vector<u8> &buffer, const std::string &string) {
+            std::copy(string.begin(), string.end(), std::back_inserter(buffer));
+        }
+
+        template<typename T>
+        void pushBytesBack(std::vector<u8> &buffer, T bytes) {
+            buffer.resize(buffer.size() + sizeof(T));
+            std::memcpy((&buffer.back() - sizeof(T)) + 1, &bytes, sizeof(T));
+        }
+
     }
 
-    template<typename T>
-    static void pushBytesBack(std::vector<u8> &buffer, T bytes) {
-        buffer.resize(buffer.size() + sizeof(T));
-        std::memcpy((&buffer.back() - sizeof(T)) + 1, &bytes, sizeof(T));
-    }
 
-    wolv::util::Expected<std::vector<u8>, IPSError> generateIPSPatch(const Patches &patches) {
+
+    wolv::util::Expected<std::vector<u8>, IPSError> Patches::toIPSPatch() const {
         std::vector<u8> result;
 
         pushStringBack(result, "PATCH");
@@ -25,7 +111,7 @@ namespace hex {
         std::vector<u64> addresses;
         std::vector<u8> values;
 
-        for (const auto &[address, value] : patches) {
+        for (const auto &[address, value] : this->m_patches) {
             addresses.push_back(address);
             values.push_back(value);
         }
@@ -67,7 +153,7 @@ namespace hex {
         return result;
     }
 
-    wolv::util::Expected<std::vector<u8>, IPSError> generateIPS32Patch(const Patches &patches) {
+    wolv::util::Expected<std::vector<u8>, IPSError> Patches::toIPS32Patch() const {
         std::vector<u8> result;
 
         pushStringBack(result, "IPS32");
@@ -75,7 +161,7 @@ namespace hex {
         std::vector<u64> addresses;
         std::vector<u8> values;
 
-        for (const auto &[address, value] : patches) {
+        for (const auto &[address, value] : this->m_patches) {
             addresses.push_back(address);
             values.push_back(value);
         }
@@ -118,7 +204,21 @@ namespace hex {
         return result;
     }
 
-    wolv::util::Expected<Patches, IPSError> loadIPSPatch(const std::vector<u8> &ipsPatch) {
+    wolv::util::Expected<Patches, IPSError> Patches::fromProvider(hex::prv::Provider* provider) {
+        PatchesGenerator generator;
+
+        generator.getUndoStack().apply(provider->getUndoStack());
+
+        if (generator.getActualSize() > 0xFFFF'FFFF)
+            return wolv::util::Unexpected(IPSError::PatchTooLarge);
+
+        auto patches = generator.getPatches();
+
+        return Patches(std::move(patches));
+    }
+
+
+    wolv::util::Expected<Patches, IPSError> Patches::fromIPSPatch(const std::vector<u8> &ipsPatch) {
         if (ipsPatch.size() < (5 + 3))
             return wolv::util::Unexpected(IPSError::InvalidPatchHeader);
 
@@ -142,7 +242,7 @@ namespace hex {
                     return wolv::util::Unexpected(IPSError::InvalidPatchFormat);
 
                 for (u16 i = 0; i < size; i++)
-                    result[offset + i] = ipsPatch[ipsOffset + i];
+                    result.get()[offset + i] = ipsPatch[ipsOffset + i];
                 ipsOffset += size;
             }
             // Handle RLE record
@@ -155,7 +255,7 @@ namespace hex {
                 ipsOffset += 2;
 
                 for (u16 i = 0; i < rleSize; i++)
-                    result[offset + i] = ipsPatch[ipsOffset + 0];
+                    result.get()[offset + i] = ipsPatch[ipsOffset + 0];
 
                 ipsOffset += 1;
             }
@@ -171,7 +271,7 @@ namespace hex {
             return wolv::util::Unexpected(IPSError::MissingEOF);
     }
 
-    wolv::util::Expected<Patches, IPSError> loadIPS32Patch(const std::vector<u8> &ipsPatch) {
+    wolv::util::Expected<Patches, IPSError> Patches::fromIPS32Patch(const std::vector<u8> &ipsPatch) {
         if (ipsPatch.size() < (5 + 4))
             return wolv::util::Unexpected(IPSError::InvalidPatchHeader);
 
@@ -195,7 +295,7 @@ namespace hex {
                     return wolv::util::Unexpected(IPSError::InvalidPatchFormat);
 
                 for (u16 i = 0; i < size; i++)
-                    result[offset + i] = ipsPatch[ipsOffset + i];
+                    result.get()[offset + i] = ipsPatch[ipsOffset + i];
                 ipsOffset += size;
             }
             // Handle RLE record
@@ -208,7 +308,7 @@ namespace hex {
                 ipsOffset += 2;
 
                 for (u16 i = 0; i < rleSize; i++)
-                    result[offset + i] = ipsPatch[ipsOffset + 0];
+                    result.get()[offset + i] = ipsPatch[ipsOffset + 0];
 
                 ipsOffset += 1;
             }

lib/libimhex/source/providers/provider.cpp

@@ -5,7 +5,6 @@
 
 #include <cmath>
 #include <cstring>
-#include <map>
 #include <optional>
 
 #include <hex/helpers/magic.hpp>
@@ -20,9 +19,8 @@ namespace hex::prv {
     }
 
 
-    Provider::Provider() : m_id(s_idCounter++) {
-        this->m_patches.emplace_back();
-        this->m_currPatches = this->m_patches.begin();
+    Provider::Provider() : m_undoRedoStack(this), m_id(s_idCounter++) {
+
     }
 
     Provider::~Provider() {
@@ -39,7 +37,7 @@ namespace hex::prv {
     }
 
     void Provider::write(u64 offset, const void *buffer, size_t size) {
-        this->writeRaw(offset - this->getBaseAddress(), buffer, size);
+        EventManager::post<EventProviderDataModified>(this, offset, size, static_cast<const u8*>(buffer));
         this->markDirty();
     }
 
@@ -68,53 +66,29 @@ namespace hex::prv {
                 file.writeBuffer(buffer.data(), bufferSize);
             }
 
-            for (auto &[patchAddress, patch] : getPatches()) {
-                file.seek(patchAddress - this->getBaseAddress());
-                file.writeBuffer(&patch, 1);
-            }
-
             EventManager::post<EventProviderSaved>(this);
         }
     }
 
     void Provider::resize(size_t newSize) {
-        hex::unused(newSize);
+        i64 difference = newSize - this->getActualSize();
+
+        if (difference > 0)
+            EventManager::post<EventProviderDataInserted>(this, this->getActualSize(), difference);
+        else if (difference < 0)
+            EventManager::post<EventProviderDataRemoved>(this, this->getActualSize(), -difference);
 
         this->markDirty();
     }
 
     void Provider::insert(u64 offset, size_t size) {
-        auto &patches = getPatches();
-
-        std::vector<std::pair<u64, u8>> patchesToMove;
-
-        for (auto &[address, value] : patches) {
-            if (address > offset)
-                patchesToMove.emplace_back(address, value);
-        }
-
-        for (const auto &[address, value] : patchesToMove)
-            patches.erase(address);
-        for (const auto &[address, value] : patchesToMove)
-            patches.insert({ address + size, value });
+        EventManager::post<EventProviderDataInserted>(this, offset, size);
 
         this->markDirty();
     }
 
     void Provider::remove(u64 offset, size_t size) {
-        auto &patches = getPatches();
-
-        std::vector<std::pair<u64, u8>> patchesToMove;
-
-        for (auto &[address, value] : patches) {
-            if (address > offset)
-                patchesToMove.emplace_back(address, value);
-        }
-
-        for (const auto &[address, value] : patchesToMove)
-            patches.erase(address);
-        for (const auto &[address, value] : patchesToMove)
-            patches.insert({ address - size, value });
+        EventManager::post<EventProviderDataRemoved>(this, offset, size);
 
         this->markDirty();
     }
@@ -131,38 +105,6 @@ namespace hex::prv {
         }
     }
 
-
-    std::map<u64, u8> &Provider::getPatches() {
-        return *this->m_currPatches;
-    }
-
-    const std::map<u64, u8> &Provider::getPatches() const {
-        return *this->m_currPatches;
-    }
-
-    void Provider::applyPatches() {
-        if (!this->isWritable())
-            return;
-
-        this->m_patches.emplace_back();
-
-        for (auto &[patchAddress, patch] : getPatches()) {
-            u8 value = 0x00;
-            this->readRaw(patchAddress - this->getBaseAddress(), &value, 1);
-            this->m_patches.back().insert({ patchAddress, value });
-        }
-
-        for (auto &[patchAddress, patch] : getPatches()) {
-            this->writeRaw(patchAddress - this->getBaseAddress(), &patch, 1);
-        }
-
-        this->markDirty();
-
-        this->m_patches.emplace_back();
-        this->m_currPatches = std::prev(this->m_patches.end());
-    }
-
-
     Overlay *Provider::newOverlay() {
         return this->m_overlays.emplace_back(std::make_unique<Overlay>()).get();
     }
@@ -235,54 +177,20 @@ namespace hex::prv {
         return { };
     }
 
-    void Provider::addPatch(u64 offset, const void *buffer, size_t size, bool createUndo) {
-        if (createUndo) {
-            // Delete all patches after the current one if a modification is made while
-            // the current patch list is not at the end of the undo stack
-            if (std::next(this->m_currPatches) != this->m_patches.end())
-                this->m_patches.erase(std::next(this->m_currPatches), this->m_patches.end());
-
-            createUndoPoint();
-        }
-
-        for (u64 i = 0; i < size; i++) {
-            u8 patch         = static_cast<const u8 *>(buffer)[i];
-            u8 originalValue = 0x00;
-            this->readRaw((offset + i) - this->getBaseAddress(), &originalValue, sizeof(u8));
-
-            if (patch == originalValue)
-                getPatches().erase(offset + i);
-            else
-                getPatches()[offset + i] = patch;
-
-            EventManager::post<EventPatchCreated>(offset, originalValue, patch);
-        }
-
-        this->markDirty();
-
-    }
-
-    void Provider::createUndoPoint() {
-        this->m_patches.push_back(getPatches());
-        this->m_currPatches = std::prev(this->m_patches.end());
-    }
-
     void Provider::undo() {
-        if (canUndo())
-            --this->m_currPatches;
+        this->m_undoRedoStack.undo();
     }
 
     void Provider::redo() {
-        if (canRedo())
-            ++this->m_currPatches;
+        this->m_undoRedoStack.redo();
     }
 
     bool Provider::canUndo() const {
-        return this->m_currPatches != this->m_patches.begin();
+        return this->m_undoRedoStack.canUndo();
     }
 
     bool Provider::canRedo() const {
-        return std::next(this->m_currPatches) != this->m_patches.end();
+        return this->m_undoRedoStack.canRedo();
     }
 
     bool Provider::hasFilePicker() const {
@@ -341,14 +249,6 @@ namespace hex::prv {
             }
         }
 
-        for (const auto &[patchAddress, value] : this->m_patches.back()) {
-            if (!nextRegionAddress.has_value() || patchAddress < nextRegionAddress)
-                nextRegionAddress = patchAddress;
-
-            if (address == patchAddress)
-                insideValidRegion = true;
-        }
-
         if (!nextRegionAddress.has_value())
             return { Region::Invalid(), false };
         else

lib/libimhex/source/providers/undo/stack.cpp

@@ -0,0 +1,131 @@
+#include <hex/providers/undo_redo/stack.hpp>
+#include <hex/providers/undo_redo/operations/operation_group.hpp>
+
+#include <hex/providers/provider.hpp>
+
+#include <wolv/utils/guards.hpp>
+
+
+namespace hex::prv::undo {
+
+    namespace {
+
+        std::atomic_bool s_locked;
+        std::mutex s_mutex;
+
+    }
+
+    Stack::Stack(Provider *provider) : m_provider(provider) {
+
+    }
+
+
+    void Stack::undo(u32 count) {
+        std::scoped_lock lock(s_mutex);
+
+        s_locked = true;
+        ON_SCOPE_EXIT { s_locked = false; };
+
+        // If there are no operations, we can't undo anything.
+        if (this->m_undoStack.empty())
+            return;
+
+        for (u32 i = 0; i < count; i += 1) {
+            // If we reached the start of the list, we can't undo anymore.
+            if (!this->canUndo()) {
+                return;
+            }
+
+            // Move last element from the undo stack to the redo stack
+            this->m_redoStack.emplace_back(std::move(this->m_undoStack.back()));
+            this->m_redoStack.back()->undo(this->m_provider);
+            this->m_undoStack.pop_back();
+        }
+    }
+
+    void Stack::redo(u32 count) {
+        std::scoped_lock lock(s_mutex);
+
+        s_locked = true;
+        ON_SCOPE_EXIT { s_locked = false; };
+
+        // If there are no operations, we can't redo anything.
+        if (this->m_redoStack.empty())
+            return;
+
+        for (u32 i = 0; i < count; i += 1) {
+            // If we reached the end of the list, we can't redo anymore.
+            if (!this->canRedo()) {
+                return;
+            }
+
+            // Move last element from the undo stack to the redo stack
+            this->m_undoStack.emplace_back(std::move(this->m_redoStack.back()));
+            this->m_undoStack.back()->redo(this->m_provider);
+            this->m_redoStack.pop_back();
+        }
+    }
+
+    void Stack::groupOperations(u32 count, const std::string &unlocalizedName) {
+        if (count <= 1)
+            return;
+
+        auto operation = std::make_unique<OperationGroup>(unlocalizedName);
+
+        i64 startIndex = std::max<i64>(0, this->m_undoStack.size() - count);
+
+        // Move operations from our stack to the group in the same order they were added
+        for (u32 i = 0; i < count; i += 1) {
+            i64 index = startIndex + i;
+
+            operation->addOperation(std::move(this->m_undoStack[index]));
+        }
+
+        // Remove the empty operations from the stack
+        this->m_undoStack.resize(startIndex);
+        this->add(std::move(operation));
+    }
+
+    void Stack::apply(const Stack &otherStack) {
+        for (const auto &operation : otherStack.m_undoStack) {
+            this->add(operation->clone());
+        }
+    }
+
+
+
+    bool Stack::add(std::unique_ptr<Operation> &&operation) {
+        // If we're already inside of an undo/redo operation, ignore new operations being added
+        if (s_locked)
+            return false;
+
+        s_locked = true;
+        ON_SCOPE_EXIT { s_locked = false; };
+
+        std::scoped_lock lock(s_mutex);
+
+        // Clear the redo stack
+        this->m_redoStack.clear();
+
+        // Insert the new operation at the end of the list
+        this->m_undoStack.emplace_back(std::move(operation));
+
+        // Do the operation
+        this->getLastOperation()->redo(this->m_provider);
+
+        return true;
+    }
+
+    bool Stack::canUndo() const {
+        return !this->m_undoStack.empty();
+    }
+
+    bool Stack::canRedo() const {
+        return !this->m_redoStack.empty();
+    }
+
+
+
+
+
+}