include/svx/svdundo.hxx | 19 ++++++++++++++++
svx/source/svdraw/svdpage.cxx | 12 ----------
svx/source/svdraw/svdundo.cxx | 48 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 11 deletions(-)
New commits:
commit 25a368c30acb54e0819d2b2b890a3fd1530d8a76
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Nov 30 20:27:24 2021 +0100
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Wed Dec 1 13:52:24 2021 +0100
tdf#145928 svx: fix undo of sorting shapes
The problem is that in sw there are these obnoxious virtual SdrObjects
that are created and destroyed by the layout.
0 SdrObject::~SdrObject()
3 SwVirtFlyDrawObj::~SwVirtFlyDrawObj()
5 SwFlyFrame::FinitDrawObj()
12 SwFrame::DestroyFrame(SwFrame*)
13 SwFrameFormat::DelFrames()
14 SwUndoFlyBase::DelFly(SwDoc*)
15 SwUndoDelLayFormat::SwUndoDelLayFormat(SwFrameFormat*)
16 SwHistoryTextFlyCnt::SwHistoryTextFlyCnt(SwFrameFormat*)
This misdesign means that SdrUndoObj::pObj may point to a deleted
object.
Commit 9bc6160e0acbc78be998129ea55ed7e4529959fa may create SdrUnoObj
when saving the document, so that is now an additional way to introduce
the problem.
Try to work around it by not using a SdrUndoObj subclass in this case,
but create a new SdrUndoAction subclass that uses SdrPage::sort() and
stores no pointers.
There is still the problem that changes in the layout or view settings
like "Hide tracked changes" could cause different virtual SdrObjects to
exist between when the Undo was recorded and when it is activated, in
which case nothing is done.
But that really requires a larger refactoring such as having a SdrPage
for the model that never contains virtual SdrObjects, and another
SdrPage for the view that does contain virtual SdrObjects to fix.
Change-Id: I4cf5d4eb8bb24a51730c55ff34a043b8a88fdb52
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126153
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/include/svx/svdundo.hxx b/include/svx/svdundo.hxx
index 67ca464b5b4c..5bb4e26c1a43 100644
--- a/include/svx/svdundo.hxx
+++ b/include/svx/svdundo.hxx
@@ -377,6 +377,24 @@ public:
virtual OUString GetComment() const override;
};
+class SdrUndoSort final : public SdrUndoAction
+{
+private:
+ ::std::vector<sal_Int32> m_OldSortOrder;
+ ::std::vector<sal_Int32> m_NewSortOrder;
+ sal_uInt16 const m_nPage;
+
+ void Do(::std::vector<sal_Int32> & rSortOrder);
+
+public:
+ SdrUndoSort(SdrPage & rPage,
+ ::std::vector<sal_Int32> const& rSortOrder);
+
+ virtual void Undo() override;
+ virtual void Redo() override;
+
+ virtual OUString GetComment() const override;
+};
// #i11702#
@@ -732,6 +750,7 @@ public:
virtual std::unique_ptr<SdrUndoAction> CreateUndoNewPage(SdrPage& rPage);
virtual std::unique_ptr<SdrUndoAction> CreateUndoCopyPage(SdrPage& rPage);
virtual std::unique_ptr<SdrUndoAction> CreateUndoSetPageNum(SdrPage&
rNewPg, sal_uInt16 nOldPageNum1, sal_uInt16 nNewPageNum1);
+ virtual std::unique_ptr<SdrUndoAction> CreateUndoSort(SdrPage& rPage,
::std::vector<sal_Int32> const& rSortOrder);
// Master page
virtual std::unique_ptr<SdrUndoAction>
CreateUndoPageRemoveMasterPage(SdrPage& rChangedPage);
diff --git a/svx/source/svdraw/svdpage.cxx b/svx/source/svdraw/svdpage.cxx
index 7167f69c5c12..334bde8e5450 100644
--- a/svx/source/svdraw/svdpage.cxx
+++ b/svx/source/svdraw/svdpage.cxx
@@ -718,25 +718,15 @@ void SdrObjList::sort( std::vector<sal_Int32>& sortOrder)
bool const isUndo(rModel.IsUndoEnabled());
if (isUndo)
{
- rModel.BegUndo(SvxResId(STR_SortShapes));
+
rModel.AddUndo(rModel.GetSdrUndoFactory().CreateUndoSort(*getSdrPageFromSdrObjList(),
sortOrder));
}
for (size_t i = 0; i < aNewSortOrder.size(); ++i)
{
aNewList[i] = maList[ aNewSortOrder[i] ];
- if (isUndo && i != sal::static_int_cast<size_t>(aNewSortOrder[i]))
- {
- rModel.AddUndo(rModel.GetSdrUndoFactory().CreateUndoObjectOrdNum(
- *aNewList[i], aNewSortOrder[i], i));
- }
aNewList[i]->SetOrdNum(i);
}
- if (isUndo)
- {
- rModel.EndUndo();
- }
-
std::swap(aNewList, maList);
}
diff --git a/svx/source/svdraw/svdundo.cxx b/svx/source/svdraw/svdundo.cxx
index 116c5e479a13..d26a27339541 100644
--- a/svx/source/svdraw/svdundo.cxx
+++ b/svx/source/svdraw/svdundo.cxx
@@ -42,6 +42,7 @@
#include <vcl/svapp.hxx>
#include <sfx2/viewsh.hxx>
#include <svx/svdoashp.hxx>
+#include <sal/log.hxx>
#include <osl/diagnose.h>
@@ -970,6 +971,48 @@ OUString SdrUndoObjOrdNum::GetComment() const
return ImpGetDescriptionStr(STR_UndoObjOrdNum);
}
+SdrUndoSort::SdrUndoSort(SdrPage & rPage,
+ ::std::vector<sal_Int32> const& rSortOrder)
+ : SdrUndoAction(rPage.getSdrModelFromSdrPage())
+ , m_OldSortOrder(rSortOrder.size())
+ , m_NewSortOrder(rSortOrder)
+ , m_nPage(rPage.GetPageNum())
+{
+ // invert order
+ for (size_t i = 0; i < rSortOrder.size(); ++i)
+ {
+ m_OldSortOrder[rSortOrder[i]] = i;
+ }
+}
+
+void SdrUndoSort::Do(::std::vector<sal_Int32> & rSortOrder)
+{
+ SdrPage & rPage(*rMod.GetPage(m_nPage));
+ if (rPage.GetObjCount() != rSortOrder.size())
+ {
+ // can probably happen with sw's cursed SdrVirtObj mess - no good
solution for that
+ SAL_WARN("svx", "SdrUndoSort size mismatch");
+ return;
+ }
+
+ // hopefully this can't throw
+ rPage.sort(rSortOrder);
+}
+
+void SdrUndoSort::Undo()
+{
+ Do(m_OldSortOrder);
+}
+
+void SdrUndoSort::Redo()
+{
+ Do(m_NewSortOrder);
+}
+
+OUString SdrUndoSort::GetComment() const
+{
+ return SvxResId(STR_SortShapes);
+}
SdrUndoObjSetText::SdrUndoObjSetText(SdrObject& rNewObj, sal_Int32 nText)
: SdrUndoObj(rNewObj)
@@ -1668,6 +1711,11 @@ std::unique_ptr<SdrUndoAction>
SdrUndoFactory::CreateUndoObjectOrdNum( SdrObject
return std::make_unique<SdrUndoObjOrdNum>( rObject, nOldOrdNum1,
nNewOrdNum1 );
}
+std::unique_ptr<SdrUndoAction> SdrUndoFactory::CreateUndoSort(SdrPage & rPage,
::std::vector<sal_Int32> const& rSortOrder)
+{
+ return std::make_unique<SdrUndoSort>(rPage, rSortOrder);
+}
+
std::unique_ptr<SdrUndoAction> SdrUndoFactory::CreateUndoReplaceObject(
SdrObject& rOldObject, SdrObject& rNewObject )
{
return std::make_unique<SdrUndoReplaceObj>( rOldObject, rNewObject );
