fix: Multiple issues with undo/redo stack handling

This commit is contained in:
WerWolv
2026-01-17 21:03:36 +01:00
parent 220b5f9772
commit 4a311ed69f
7 changed files with 129 additions and 100 deletions

View File

@@ -10,6 +10,8 @@
#include <hex/ui/imgui_imhex_extensions.h>
struct ImRect;
EXPORT_MODULE namespace hex {
class TutorialManager {
@@ -172,6 +174,8 @@ EXPORT_MODULE namespace hex {
static void setRenderer(std::function<DrawFunction(const std::string &)> renderer);
static void postElementRendered(ImGuiID id, const ImRect &boundingBox);
private:
TutorialManager() = delete;

View File

@@ -7,6 +7,7 @@
#include <map>
#include <memory>
#include <mutex>
#include <vector>
namespace hex::prv {
@@ -40,6 +41,8 @@ namespace hex::prv::undo {
bool add(std::unique_ptr<Operation> &&operation);
static std::recursive_mutex& getMutex();
const std::vector<std::unique_ptr<Operation>> &getAppliedOperations() const {
return m_undoStack;
}

View File

@@ -96,39 +96,6 @@ namespace hex {
}
void TutorialManager::init() {
EventImGuiElementRendered::subscribe([](ImGuiID id, const std::array<float, 4> bb){
const auto boundingBox = ImRect(bb[0], bb[1], bb[2], bb[3]);
if (!ImGui::IsRectVisible(boundingBox.Min, boundingBox.Max))
return;
{
const auto element = hex::s_highlights->find(id);
if (element != hex::s_highlights->end()) {
hex::s_highlightDisplays->emplace_back(boundingBox, element->second);
const auto window = ImGui::GetCurrentWindow();
if (window != nullptr && window->DockNode != nullptr && window->DockNode->TabBar != nullptr)
window->DockNode->TabBar->NextSelectedTabId = window->TabId;
}
}
{
const auto element = s_interactiveHelpItems->find(id);
if (element != s_interactiveHelpItems->end()) {
(*s_interactiveHelpDisplays)[id] = boundingBox;
}
}
if (id != 0 && boundingBox.Contains(ImGui::GetMousePos())) {
if ((s_hoveredRect.GetArea() == 0 || boundingBox.GetArea() < s_hoveredRect.GetArea()) && s_interactiveHelpItems->contains(id)) {
s_hoveredRect = boundingBox;
s_hoveredId = id;
}
}
});
if (*s_renderer == nullptr) {
*s_renderer = [](const std::string &message) {
return [message] {
@@ -141,6 +108,37 @@ namespace hex {
}
}
void TutorialManager::postElementRendered(ImGuiID id, const ImRect &boundingBox) {
if (!ImGui::IsRectVisible(boundingBox.Min, boundingBox.Max))
return;
{
const auto element = hex::s_highlights->find(id);
if (element != hex::s_highlights->end()) {
hex::s_highlightDisplays->emplace_back(boundingBox, element->second);
const auto window = ImGui::GetCurrentWindow();
if (window != nullptr && window->DockNode != nullptr && window->DockNode->TabBar != nullptr)
window->DockNode->TabBar->NextSelectedTabId = window->TabId;
}
}
{
const auto element = s_interactiveHelpItems->find(id);
if (element != s_interactiveHelpItems->end()) {
(*s_interactiveHelpDisplays)[id] = boundingBox;
}
}
if (id != 0 && boundingBox.Contains(ImGui::GetMousePos())) {
if ((s_hoveredRect.GetArea() == 0 || boundingBox.GetArea() < s_hoveredRect.GetArea()) && s_interactiveHelpItems->contains(id)) {
s_hoveredRect = boundingBox;
s_hoveredId = id;
}
}
}
const std::map<std::string, TutorialManager::Tutorial>& TutorialManager::getTutorials() {
return s_tutorials;
}

View File

@@ -3,13 +3,12 @@
#include <imgui.h>
#include <imgui_internal.h>
#include <array>
#include <hex/api/tutorial_manager.hpp>
#if !defined(IMGUI_TEST_ENGINE)
void ImGuiTestEngineHook_ItemAdd(ImGuiContext*, ImGuiID id, const ImRect& bb, const ImGuiLastItemData*) {
std::array<float, 4> boundingBox = { bb.Min.x, bb.Min.y, bb.Max.x, bb.Max.y };
hex::EventImGuiElementRendered::post(id, boundingBox);
hex::TutorialManager::postElementRendered(id, bb);
}
void ImGuiTestEngineHook_ItemInfo(ImGuiContext*, ImGuiID, const char*, ImGuiItemStatusFlags) {}

View File

@@ -12,8 +12,7 @@ namespace hex::prv::undo {
namespace {
std::atomic<bool> s_locked;
std::mutex s_mutex;
std::recursive_mutex s_mutex;
}
@@ -21,12 +20,13 @@ namespace hex::prv::undo {
}
std::recursive_mutex& Stack::getMutex() {
return s_mutex;
}
void Stack::undo(u32 count) {
std::scoped_lock lock(s_mutex);
s_locked = true;
ON_SCOPE_EXIT { s_locked = false; };
std::lock_guard lock(s_mutex);
// If there are no operations, we can't undo anything.
if (m_undoStack.empty())
@@ -42,14 +42,12 @@ namespace hex::prv::undo {
m_redoStack.emplace_back(std::move(m_undoStack.back()));
m_redoStack.back()->undo(m_provider);
m_undoStack.pop_back();
EventDataChanged::post(m_provider);
}
}
void Stack::redo(u32 count) {
std::scoped_lock lock(s_mutex);
s_locked = true;
ON_SCOPE_EXIT { s_locked = false; };
std::lock_guard lock(s_mutex);
// If there are no operations, we can't redo anything.
if (m_redoStack.empty())
@@ -65,10 +63,13 @@ namespace hex::prv::undo {
m_undoStack.emplace_back(std::move(m_redoStack.back()));
m_undoStack.back()->redo(m_provider);
m_redoStack.pop_back();
EventDataChanged::post(m_provider);
}
}
void Stack::groupOperations(u32 count, const UnlocalizedString &unlocalizedName) {
std::lock_guard lock(s_mutex);
if (count <= 1)
return;
@@ -94,14 +95,19 @@ namespace hex::prv::undo {
}
void Stack::apply(const Stack &otherStack) {
std::lock_guard lock(s_mutex);
for (const auto &operation : otherStack.m_undoStack) {
this->add(operation->clone());
}
}
void Stack::reapply() {
std::lock_guard lock(s_mutex);
for (const auto &operation : m_undoStack) {
operation->redo(m_provider);
EventDataChanged::post(m_provider);
}
}
@@ -109,14 +115,7 @@ namespace hex::prv::undo {
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);
std::lock_guard lock(s_mutex);
// Clear the redo stack
m_redoStack.clear();
@@ -133,10 +132,14 @@ namespace hex::prv::undo {
}
bool Stack::canUndo() const {
std::lock_guard lock(s_mutex);
return !m_undoStack.empty();
}
bool Stack::canRedo() const {
std::lock_guard lock(s_mutex);
return !m_redoStack.empty();
}

View File

@@ -19,6 +19,7 @@ namespace hex::plugin::builtin {
u64 m_selectedPatch = 0x00;
PerProvider<u32> m_numOperations;
PerProvider<u32> m_savedOperations;
PerProvider<std::set<u64>> m_modifiedAddresses;
};
}

View File

@@ -49,6 +49,8 @@ namespace hex::plugin::builtin {
});
ImHexApi::HexEditor::addForegroundHighlightingProvider([this](u64 offset, const u8* buffer, size_t, bool) -> std::optional<color_t> {
std::lock_guard lock(prv::undo::Stack::getMutex());
std::ignore = buffer;
if (!ImHexApi::Provider::isValid())
@@ -60,29 +62,8 @@ namespace hex::plugin::builtin {
offset -= provider->getBaseAddress();
const auto &undoStack = provider->getUndoStack();
const auto stackSize = undoStack.getAppliedOperations().size();
const auto savedStackSize = m_savedOperations.get(provider);
if (stackSize == savedStackSize) {
// Do nothing
} else if (stackSize > savedStackSize) {
for (const auto &operation : undoStack.getAppliedOperations() | std::views::drop(savedStackSize)) {
if (!operation->shouldHighlight())
continue;
if (operation->getRegion().overlaps(Region { .address=offset, .size=1}))
return ImGuiExt::GetCustomColorU32(ImGuiCustomCol_Patches);
}
} else {
for (const auto &operation : undoStack.getUndoneOperations() | std::views::reverse | std::views::take(savedStackSize - stackSize)) {
if (!operation->shouldHighlight())
continue;
if (operation->getRegion().overlaps(Region { .address=offset, .size=1}))
return ImGuiExt::GetCustomColorU32(ImGuiCustomCol_Patches);
}
}
if (m_modifiedAddresses->contains(offset))
return ImGuiExt::GetCustomColorU32(ImGuiCustomCol_Patches);
return std::nullopt;
});
@@ -114,6 +95,35 @@ namespace hex::plugin::builtin {
provider->getUndoStack().add<undo::OperationRemove>(offset, size);
});
EventDataChanged::subscribe(this, [this](prv::Provider *provider) {
std::lock_guard lock(prv::undo::Stack::getMutex());
const auto &undoStack = provider->getUndoStack();
const auto stackSize = undoStack.getAppliedOperations().size();
const auto savedStackSize = m_savedOperations.get(provider);
m_modifiedAddresses->clear();
if (stackSize == savedStackSize) {
// Do nothing
} else if (stackSize > savedStackSize) {
for (const auto &operation : undoStack.getAppliedOperations() | std::views::drop(savedStackSize)) {
if (!operation->shouldHighlight())
continue;
auto region = operation->getRegion();
m_modifiedAddresses->insert_range(std::views::iota(region.getStartAddress(), region.getEndAddress() + 1));
}
} else {
for (const auto &operation : undoStack.getUndoneOperations() | std::views::reverse | std::views::take(savedStackSize - stackSize)) {
if (!operation->shouldHighlight())
continue;
auto region = operation->getRegion();
m_modifiedAddresses->insert_range(std::views::iota(region.getStartAddress(), region.getEndAddress() + 1));
}
}
});
}
ViewPatches::~ViewPatches() {
@@ -138,37 +148,53 @@ namespace hex::plugin::builtin {
ImGui::TableHeadersRow();
const auto &undoRedoStack = provider->getUndoStack();
std::vector<prv::undo::Operation*> operations;
for (const auto &operation : undoRedoStack.getUndoneOperations())
operations.push_back(operation.get());
for (const auto &operation : undoRedoStack.getAppliedOperations() | std::views::reverse)
operations.push_back(operation.get());
std::lock_guard lock(prv::undo::Stack::getMutex());
u32 index = 0;
const auto &undoRedoStack = provider->getUndoStack();
const auto &undoneOps = undoRedoStack.getUndoneOperations();
const auto &appliedOps = undoRedoStack.getAppliedOperations();
const u32 totalSize = undoneOps.size() + appliedOps.size();
ImGuiListClipper clipper;
clipper.Begin(totalSize);
clipper.Begin(operations.size());
while (clipper.Step()) {
auto iter = operations.begin();
for (auto i = 0; i < clipper.DisplayStart; i++)
++iter;
auto undoneOperationsCount = undoneOps.size();
for (u64 i = clipper.DisplayStart; i < u64(clipper.DisplayEnd); i++) {
prv::undo::Operation* operation;
auto undoneOperationsCount = undoRedoStack.getUndoneOperations().size();
for (auto i = clipper.DisplayStart; i < clipper.DisplayEnd; i++) {
const auto &operation = *iter;
if (i < undoneOps.size()) {
// Element from undone operations
operation = undoneOps[i].get();
} else {
// Element from applied operations (reversed)
u32 appliedIndex = appliedOps.size() - 1 - (i - undoneOps.size());
operation = appliedOps[appliedIndex].get();
}
const auto [address, size] = operation->getRegion();
ImGui::TableNextRow();
ImGui::TableNextColumn();
ImGui::BeginDisabled(size_t(i) < undoneOperationsCount);
if (ImGui::Selectable(fmt::format("{} {}", index == undoneOperationsCount ? ICON_VS_ARROW_SMALL_RIGHT : " ", index).c_str(), false, ImGuiSelectableFlags_SpanAllColumns)) {
ImHexApi::HexEditor::setSelection(address, size);
bool isUndone = size_t(i) < undoneOperationsCount;
ImGui::PushStyleColor(ImGuiCol_Text, ImGui::GetColorU32(isUndone ? ImGuiCol_TextDisabled : ImGuiCol_Text));
if (ImGui::Selectable(fmt::format("{} {}", i == undoneOperationsCount ? ICON_VS_ARROW_SMALL_RIGHT : " ", i).c_str(), false, ImGuiSelectableFlags_SpanAllColumns)) {
if (ImGui::GetIO().KeyShift) {
if (isUndone) {
const u32 count = undoneOperationsCount - u32(i);
provider->getUndoStack().redo(count);
} else {
const u32 count = u32(i) - undoneOperationsCount;
provider->getUndoStack().undo(count);
}
} else {
ImHexApi::HexEditor::setSelection(address, size);
}
}
ImGui::PopStyleColor();
if (ImGui::IsItemHovered()) {
const auto content = operation->formatContent();
if (!content.empty()) {
@@ -196,11 +222,6 @@ namespace hex::plugin::builtin {
ImGui::TableNextColumn();
ImGuiExt::TextFormatted("{}", operation->format());
index += 1;
++iter;
ImGui::EndDisabled();
}
}