From 8e4732c1e0876017ea96b7e31dbd394247e46c1b Mon Sep 17 00:00:00 2001 From: Yousef Khadadeh Date: Thu, 28 May 2026 21:55:30 +0300 Subject: [PATCH] Fix undo/redo problem with erase, added send_multi_update_message blocks --- .../NetworkingObjects/NetObjManager.cpp | 2 +- .../NetworkingObjects/NetObjOrderedList.hpp | 33 +++++++------- src/Bookmarks/BookmarkManager.cpp | 13 +++--- src/CanvasComponents/MeshCanvasComponent.cpp | 8 ++-- .../DrawingProgramSelection.cpp | 4 +- .../Layers/DrawingProgramLayerManager.cpp | 25 ++++++++--- .../Layers/DrawingProgramLayerManager.hpp | 1 + .../Layers/DrawingProgramLayerManagerGUI.cpp | 12 ++--- src/DrawingProgram/Tools/BrushTool.cpp | 6 ++- src/DrawingProgram/Tools/EraserTool.cpp | 45 +++++++++++-------- src/WorldUndoManager.cpp | 25 ++++++----- 11 files changed, 102 insertions(+), 72 deletions(-) diff --git a/include/Helpers/NetworkingObjects/NetObjManager.cpp b/include/Helpers/NetworkingObjects/NetObjManager.cpp index a4d1fde..26aaa8d 100644 --- a/include/Helpers/NetworkingObjects/NetObjManager.cpp +++ b/include/Helpers/NetworkingObjects/NetObjManager.cpp @@ -68,7 +68,7 @@ namespace NetworkingObjects { destroyCallback = newDestroyCallback; } void NetObjManager::send_multi_update_messsage(std::function captureSendBlock, SendUpdateType updateType, const std::shared_ptr& specificClient) { - if(is_connected()) { + if(is_connected() && multiUpdateData.expired()) { // Check for multiUpdateData.expired() to make sure we aren't overriding a previous call to send_multi_update_messsage std::shared_ptr d = std::make_shared(); d->ss = std::make_shared(std::ios::binary | std::ios::out); d->outArchive = std::make_unique(*d->ss); diff --git a/include/Helpers/NetworkingObjects/NetObjOrderedList.hpp b/include/Helpers/NetworkingObjects/NetObjOrderedList.hpp index a7f4447..6ac112d 100644 --- a/include/Helpers/NetworkingObjects/NetObjOrderedList.hpp +++ b/include/Helpers/NetworkingObjects/NetObjOrderedList.hpp @@ -130,20 +130,26 @@ namespace NetworkingObjects { } } - template void sort_netobj_ordered_list_iterator_list(const std::list>& l, std::vector>& itListToSort) { - std::stable_sort(itListToSort.begin(), itListToSort.end(), [&l](auto& a, auto& b) { - return b == l.end() || (a != l.end() && a->pos < b->pos); - }); - } - - template void sort_netobj_ordered_list_iterator_insert_list(const std::list>& l, std::vector, NetObjOwnerPtr>>& itListToSort) { - std::stable_sort(itListToSort.begin(), itListToSort.end(), [&l](auto& a, auto& b) { - return b.first == l.end() || (a.first != l.end() && a.first->pos < b.first->pos); - }); - } - template class NetObjOrderedList { public: + static void sort_netobj_ordered_list_iterator_list(const NetObjTemporaryPtr>& l, std::vector>& itListToSort) { + std::stable_sort(itListToSort.begin(), itListToSort.end(), [&l](auto& a, auto& b) { + return b == l->end() || (a != l->end() && a->pos < b->pos); + }); + } + + static void sort_netobj_ordered_list_iterator_insert_list(const NetObjTemporaryPtr>& l, std::vector, NetObjOwnerPtr>>& itListToSort) { + std::stable_sort(itListToSort.begin(), itListToSort.end(), [&l](auto& a, auto& b) { + return b.first == l->end() || (a.first != l->end() && a.first->pos < b.first->pos); + }); + } + + static void sort_netobj_ordered_list_iterator_insert_list(const NetObjTemporaryPtr>& l, std::vector, T*>>& itListToSort) { + std::stable_sort(itListToSort.begin(), itListToSort.end(), [&l](auto& a, auto& b) { + return b.first == l->end() || (a.first != l->end() && a.first->pos < b.first->pos); + }); + } + // Don't use this function unless you're sure that class T isn't a base class template NetObjOrderedListIterator emplace_back_direct(const NetObjTemporaryPtr>& l, Args&&... items) { return l->insert_single(l, nullptr, l->end(), l.get_obj_man()->template make_obj_direct(items...)); @@ -302,9 +308,6 @@ namespace NetworkingObjects { toRet.emplace_back(get(id)); return toRet; } - void sort_it_list(std::vector>& itList) const { - sort_netobj_ordered_list_iterator_list(data_list(), itList); - } bool empty() const { return data_list().empty(); } diff --git a/src/Bookmarks/BookmarkManager.cpp b/src/Bookmarks/BookmarkManager.cpp index 2cb5aec..b7c4a3a 100644 --- a/src/Bookmarks/BookmarkManager.cpp +++ b/src/Bookmarks/BookmarkManager.cpp @@ -170,7 +170,7 @@ void BookmarkManager::setup_list_gui() { uint32_t insertUndoPosition; std::vector insertedUndoIDs; - world.netObjMan.send_multi_update_messsage([&]() { + { std::unordered_map>> toEraseMap; uint32_t newIndex = index; std::vector> toInsertObjPtrs; @@ -218,7 +218,7 @@ void BookmarkManager::setup_list_gui() { insertUndoPosition = newIndex; for(auto& it : insertedIterators) insertedUndoIDs.emplace_back(world.undo.get_undoid_from_netid(it->obj.get_net_id())); - }, NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } class MoveBookmarksWorldUndoAction : public WorldUndoAction { public: @@ -269,7 +269,7 @@ void BookmarkManager::setup_list_gui() { } } - undoMan.world.netObjMan.send_multi_update_messsage([&]() { + { eraseListPtr->erase_list(eraseListPtr, toEraseList, &toMoveObjs); for(auto& [parentNetID, indexNetIDListPair] : toInsertToMap) { @@ -286,7 +286,7 @@ void BookmarkManager::setup_list_gui() { } parentListPtr->insert_sorted_list_and_send_create(parentListPtr, toInsert); } - }, NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } return true; } bool redo(WorldUndoManager& undoMan) override { @@ -316,7 +316,7 @@ void BookmarkManager::setup_list_gui() { return false; } - undoMan.world.netObjMan.send_multi_update_messsage([&]() { + { std::vector> toInsertObjPtrs; for(auto& [listToEraseFrom, setToErase] : toEraseMap) { @@ -334,7 +334,8 @@ void BookmarkManager::setup_list_gui() { toInsert.back().second.reassign_ids(); } listPtr->insert_sorted_list_and_send_create(listPtr, toInsert); - }, NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } + return true; } ~MoveBookmarksWorldUndoAction() {} diff --git a/src/CanvasComponents/MeshCanvasComponent.cpp b/src/CanvasComponents/MeshCanvasComponent.cpp index 195647a..0b56a8a 100644 --- a/src/CanvasComponents/MeshCanvasComponent.cpp +++ b/src/CanvasComponents/MeshCanvasComponent.cpp @@ -305,6 +305,7 @@ std::vector MeshCanvasComponent::attempt_split(Drawin mesh.d.meshPath = p; newComp->coords = compContainer->coords; mesh.simplify_paths(); + newComp->normalize_object_coordinates(); toRet.emplace_back(newComp); } @@ -314,21 +315,18 @@ std::vector MeshCanvasComponent::attempt_split(Drawin void MeshCanvasComponent::normalize_object_coordinates(CoordSpaceHelper& coords) { SkRect pathBounds = d.meshPath.getBounds(); SkPoint center = pathBounds.center(); - float maxDimScaleFactor = 1.0 / (std::max(pathBounds.width(), pathBounds.height()) * 0.5f); // Scale must be uniform. This scale factor should force the maximum coordinate = 1 after translation + constexpr float MAX_COORDINATE = 1000.0f; + float maxDimScaleFactor = MAX_COORDINATE / (std::max(pathBounds.width(), pathBounds.height()) * 0.5f); // Scale must be uniform. This scale factor should force the maximum coordinate = MAX_COORDINATE after transformation SkMatrix m = SkMatrix::I(); - // Might not be in correct order. Translate to origin first, then scale to normalize coordinates m.postTranslate(-center.x(), -center.y()).postScale(maxDimScaleFactor, maxDimScaleFactor); std::optional tryTransformPath = d.meshPath.tryMakeTransform(m); if(tryTransformPath) { d.meshPath = tryTransformPath.value(); - //for(auto& p : d.meshPath.points()) - // std::cout << vec_pretty(convert_vec2(p)) << std::endl; WorldVec newCenter = WorldVec{center.x(), center.y()} * coords.inverseScale; newCenter = rotate_world_coord(newCenter, coords.rotation); coords.translate(newCenter); coords.scale_about_double(coords.pos, maxDimScaleFactor); } - //std::cout << vec_pretty(convert_vec2(center)) << " " << coords.pos.x().display_int_str(3) << " " << coords.pos.y().display_int_str(3) << " " << coords.inverseScale << std::endl; } CanvasComponentEraseDetailResult MeshCanvasComponent::erase_detail(const SkPath& eraseAgainst) { diff --git a/src/DrawingProgram/DrawingProgramSelection.cpp b/src/DrawingProgram/DrawingProgramSelection.cpp index 6176384..bde633f 100644 --- a/src/DrawingProgram/DrawingProgramSelection.cpp +++ b/src/DrawingProgram/DrawingProgramSelection.cpp @@ -218,7 +218,7 @@ void DrawingProgramSelection::check_add_stroke_color_change_undo() { if(!undoMan.fill_netid_list_from_undoid_list(netIDs, undoIDs)) return false; - undoMan.world.netObjMan.send_multi_update_messsage([&]() { + { for(size_t i = 0; i < netIDs.size(); i++) { auto objPtr = undoMan.world.netObjMan.get_obj_temporary_ref_from_id(netIDs[i]); if(objPtr) { @@ -229,7 +229,7 @@ void DrawingProgramSelection::check_add_stroke_color_change_undo() { objPtr->send_comp_update(undoMan.world.drawProg, true); } } - }, NetworkingObjects::NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } return true; } diff --git a/src/DrawingProgram/Layers/DrawingProgramLayerManager.cpp b/src/DrawingProgram/Layers/DrawingProgramLayerManager.cpp index 77d7bf5..c31de99 100644 --- a/src/DrawingProgram/Layers/DrawingProgramLayerManager.cpp +++ b/src/DrawingProgram/Layers/DrawingProgramLayerManager.cpp @@ -203,12 +203,12 @@ void DrawingProgramLayerManager::push_components_to(const std::vector(compNetID); std::vector>> toInsert; std::vector> toErase = components->get_list(moveData.objs); - components->sort_it_list(toErase); // There's a chance that objects could have changed order on the client side, so sort them + components->sort_netobj_ordered_list_iterator_list(components, toErase); // There's a chance that objects could have changed order on the client side, so sort them std::vector> erasedObjs; std::vector newPos; @@ -228,7 +228,7 @@ void DrawingProgramLayerManager::push_components_to(const std::vectorinsert_sorted_list_and_send_create(components, toInsert); *moveData.oldPos = newPos; } - }, NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } return true; } ~MoveComponentsUndoAction() {} @@ -423,7 +423,7 @@ void DrawingProgramLayerManager::add_undo_erase_components(const std::unordered_ } } - undoMan.world.netObjMan.send_multi_update_messsage([&]() { + { for(auto& [parentNetID, insertData] : insertMap) { auto parentListPtr = undoMan.world.netObjMan.get_obj_temporary_ref_from_id>(parentNetID); std::vector, NetObjOwnerPtr>> toInsert(insertData.it.size()); @@ -435,7 +435,7 @@ void DrawingProgramLayerManager::add_undo_erase_components(const std::unordered_ parentListPtr->insert_sorted_list_and_send_create(parentListPtr, toInsert); insertData.copyData->clear(); } - }, NetworkingObjects::NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } return true; } @@ -458,12 +458,12 @@ void DrawingProgramLayerManager::add_undo_erase_components(const std::unordered_ } } - undoMan.world.netObjMan.send_multi_update_messsage([&]() { + { for(auto& [parentNetID, eData] : eraseMap) { auto parentListPtr = undoMan.world.netObjMan.get_obj_temporary_ref_from_id>(parentNetID); parentListPtr->erase_list(parentListPtr, eData); } - }, NetworkingObjects::NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } return true; } @@ -543,6 +543,8 @@ void DrawingProgramLayerManager::add_undo_place_components(DrawingProgramLayerLi bool redo(WorldUndoManager& undoMan) override { std::vector> it; NetObjTemporaryPtr> parentListPtr; + + auto p = std::chrono::steady_clock::now(); { std::optional parentNetID = undoMan.get_netid_from_undoid(parentUndoID); @@ -551,18 +553,27 @@ void DrawingProgramLayerManager::add_undo_place_components(DrawingProgramLayerLi parentListPtr = undoMan.world.netObjMan.get_obj_temporary_ref_from_id>(parentNetID.value()); } + p = std::chrono::steady_clock::now(); + { it = parentListPtr->at_ordered_indices(undoPlaceData.pos); } + p = std::chrono::steady_clock::now(); + std::vector, NetObjOwnerPtr>> toInsert(it.size()); for(size_t i = 0; i < it.size(); i++) { toInsert[i].first = it[i]; toInsert[i].second = undoMan.world.netObjMan.make_obj_direct(undoMan.world.netObjMan, *undoPlaceData.copyData.at(i)); undoMan.register_new_netid_to_existing_undoid(undoPlaceData.undoID.at(i), toInsert[i].second.get_net_id()); } + + p = std::chrono::steady_clock::now(); + parentListPtr->insert_sorted_list_and_send_create(parentListPtr, toInsert); + p = std::chrono::steady_clock::now(); + undoPlaceData.copyData.clear(); return true; } diff --git a/src/DrawingProgram/Layers/DrawingProgramLayerManager.hpp b/src/DrawingProgram/Layers/DrawingProgramLayerManager.hpp index f69b746..5f877e3 100644 --- a/src/DrawingProgram/Layers/DrawingProgramLayerManager.hpp +++ b/src/DrawingProgram/Layers/DrawingProgramLayerManager.hpp @@ -79,6 +79,7 @@ class DrawingProgramLayerManager { uint32_t total_component_count(); CanvasComponentContainer::ObjInfo* add_component_to_layer_being_edited(CanvasComponentContainer* newObj); + // BOTH add_many_components_to_layer CALLS REQUIRE A SORTED INSERT LIST std::vector add_many_components_to_layer_being_edited(const std::vector>& newObjs); std::vector add_many_components_to_layer(DrawingProgramLayerListItem* layer, const std::vector>& newObjs, bool newUndo = true); std::vector get_flattened_component_list() const; diff --git a/src/DrawingProgram/Layers/DrawingProgramLayerManagerGUI.cpp b/src/DrawingProgram/Layers/DrawingProgramLayerManagerGUI.cpp index f96a11c..1b7f411 100644 --- a/src/DrawingProgram/Layers/DrawingProgramLayerManagerGUI.cpp +++ b/src/DrawingProgram/Layers/DrawingProgramLayerManagerGUI.cpp @@ -202,7 +202,7 @@ void DrawingProgramLayerManagerGUI::setup_list_gui() { uint32_t insertUndoPosition; std::vector insertedUndoIDs; - world.netObjMan.send_multi_update_messsage([&]() { + { std::unordered_map>> toEraseMap; uint32_t newIndex = index; std::vector> toInsertObjPtrs; @@ -250,7 +250,7 @@ void DrawingProgramLayerManagerGUI::setup_list_gui() { insertUndoPosition = newIndex; for(auto& it : insertedIterators) insertedUndoIDs.emplace_back(world.undo.get_undoid_from_netid(it->obj.get_net_id())); - }, NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } class MoveLayersWorldUndoAction : public WorldUndoAction { public: @@ -301,7 +301,7 @@ void DrawingProgramLayerManagerGUI::setup_list_gui() { } } - undoMan.world.netObjMan.send_multi_update_messsage([&]() { + { eraseListPtr->erase_list(eraseListPtr, toEraseList, &toMoveObjs); for(auto& [parentNetID, indexNetIDListPair] : toInsertToMap) { @@ -318,7 +318,7 @@ void DrawingProgramLayerManagerGUI::setup_list_gui() { } parentListPtr->insert_sorted_list_and_send_create(parentListPtr, toInsert); } - }, NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } return true; } bool redo(WorldUndoManager& undoMan) override { @@ -348,7 +348,7 @@ void DrawingProgramLayerManagerGUI::setup_list_gui() { return false; } - undoMan.world.netObjMan.send_multi_update_messsage([&]() { + { std::vector> toInsertObjPtrs; for(auto& [listToEraseFrom, setToErase] : toEraseMap) { @@ -366,7 +366,7 @@ void DrawingProgramLayerManagerGUI::setup_list_gui() { toInsert.back().second.reassign_ids(); } listPtr->insert_sorted_list_and_send_create(listPtr, toInsert); - }, NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); + } return true; } diff --git a/src/DrawingProgram/Tools/BrushTool.cpp b/src/DrawingProgram/Tools/BrushTool.cpp index 7db7b4c..8b54abc 100644 --- a/src/DrawingProgram/Tools/BrushTool.cpp +++ b/src/DrawingProgram/Tools/BrushTool.cpp @@ -87,9 +87,13 @@ void BrushTool::commit_data(bool final) { std::optional simplified = Simplify(newMesh.d.meshPath); if(simplified.has_value()) newMesh.d.meshPath = simplified.value(); + containerPtr->normalize_object_coordinates(); } containerPtr->commit_update(drawP); - containerPtr->send_comp_update(drawP, final); + drawP.world.netObjMan.send_multi_update_messsage([&]() { + drawP.send_transforms_for({objInfoBeingEdited}); + containerPtr->send_comp_update(drawP, final); + }, NetworkingObjects::NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); } commitUpdate = false; } diff --git a/src/DrawingProgram/Tools/EraserTool.cpp b/src/DrawingProgram/Tools/EraserTool.cpp index 4c6607e..085140f 100644 --- a/src/DrawingProgram/Tools/EraserTool.cpp +++ b/src/DrawingProgram/Tools/EraserTool.cpp @@ -29,6 +29,7 @@ #include "../../GUIStuff/ElementHelpers/TextLabelHelpers.hpp" #include "../../GUIStuff/ElementHelpers/RadioButtonHelpers.hpp" #include "../../GUIStuff/ElementHelpers/CheckBoxHelpers.hpp" +#include "Helpers/NetworkingObjects/NetObjOrderedList.hpp" EraserTool::EraserTool(DrawingProgram& initDrawP): DrawingProgramToolBase(initDrawP) @@ -224,26 +225,32 @@ void EraserTool::commit_erase() { } return false; }); - bool firstPlacement = true; - for(auto& [layer, compMap] : newObjectsToPlace) { - drawP.layerMan.add_many_components_to_layer(layer, compMap, firstPlacement); - firstPlacement = false; - } - bool objectsToErase = !erasedComponents.empty(); - if(objectsToErase) - drawP.layerMan.erase_component_container(erasedComponents, !placedNewObject); - bool first = !objectsToErase && !placedNewObject; - for(auto& [comp, oldData] : updatedComponents) { - comp->obj->get_comp().simplify_paths(); - comp->obj->commit_update_dont_invalidate_cache(drawP); - comp->obj->send_comp_update(drawP, true); - if(first) { - first = false; - drawP.world.undo.push(std::make_unique(std::move(oldData), drawP.world.undo.get_undoid_from_netid(comp->obj.get_net_id()))); + + drawP.world.netObjMan.send_multi_update_messsage([&]() { + bool firstPlacement = true; + for(auto& [layer, compMap] : newObjectsToPlace) { + layer->get_layer().components->sort_netobj_ordered_list_iterator_insert_list(layer->get_layer().components, compMap); + drawP.layerMan.add_many_components_to_layer(layer, compMap, firstPlacement); + firstPlacement = false; } - else - drawP.world.undo.push_on_last(std::make_unique(std::move(oldData), drawP.world.undo.get_undoid_from_netid(comp->obj.get_net_id()))); - } + bool objectsToErase = !erasedComponents.empty(); + if(objectsToErase) + drawP.layerMan.erase_component_container(erasedComponents, !placedNewObject); + bool first = !objectsToErase && !placedNewObject; + for(auto& [comp, oldData] : updatedComponents) { + comp->obj->get_comp().simplify_paths(); + comp->obj->normalize_object_coordinates(); + comp->obj->commit_update_dont_invalidate_cache(drawP); + drawP.send_transforms_for({comp}); + comp->obj->send_comp_update(drawP, true); + if(first) { + first = false; + drawP.world.undo.push(std::make_unique(std::move(oldData), drawP.world.undo.get_undoid_from_netid(comp->obj.get_net_id()))); + } + else + drawP.world.undo.push_on_last(std::make_unique(std::move(oldData), drawP.world.undo.get_undoid_from_netid(comp->obj.get_net_id()))); + } + }, NetworkingObjects::NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); erasedComponents.clear(); updatedComponents.clear(); } diff --git a/src/WorldUndoManager.cpp b/src/WorldUndoManager.cpp index 01bfc31..cfd13d2 100644 --- a/src/WorldUndoManager.cpp +++ b/src/WorldUndoManager.cpp @@ -75,12 +75,15 @@ void WorldUndoManager::undo() { world.bMan.refresh_gui_data(); bool undoFail = false; - for(auto& u : std::views::reverse(undoQueue.back())) { - if(!u->undo(*this)) { - undoFail = true; - break; + + world.netObjMan.send_multi_update_messsage([&]() { + for(auto& u : std::views::reverse(undoQueue.back())) { + if(!u->undo(*this)) { + undoFail = true; + break; + } } - } + }, NetworkingObjects::NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); if(undoFail) { clear(); @@ -103,12 +106,14 @@ void WorldUndoManager::redo() { world.bMan.refresh_gui_data(); bool redoFail = false; - for(auto& u : redoQueue.back()) { - if(!u->redo(*this)) { - redoFail = true; - break; + world.netObjMan.send_multi_update_messsage([&]() { + for(auto& u : redoQueue.back()) { + if(!u->redo(*this)) { + redoFail = true; + break; + } } - } + }, NetworkingObjects::NetObjManager::SendUpdateType::SEND_TO_ALL, nullptr); if(redoFail) { clear();