tdf#160329 update objects after row sort is finished

The problem was that when the object position was updated to the anchor
values by recalcPos() method, the document did not yet have the new
state of the hidden rows. As a result, incorrect positions were
calculated. Therefore, the update of the position is moved to a place
after the update of the visibility of the rows.

Sorting rows must not change the visibility of objects. However, updating
the visibility of rows sets all objects to visible. Now the visibility
state of an object is saved and restored later so that the recalcPos()
method receives the correct state for the object.

Change-Id: Ia32698c1d45cd81702e6d00c5dfc100f6f6f399c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166799
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
(cherry picked from commit f0a2969d15e3101d7f96a7fe77bca06a5d70f57a)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166903
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
(cherry picked from commit 1347274fc1cb780db4c7c9b8c0272e1730b95c04)

3 files changed, 109 insertions, 6 deletions

diff --git a/sc/qa/unit/data/ods/tdf160329_sortWithHiddenRows.ods b/sc/qa/unit/data/ods/tdf160329_sortWithHiddenRows.ods
new file mode 100644
index 000000000000..5ba746ad7baa
--- /dev/null
+++ b/sc/qa/unit/data/ods/tdf160329_sortWithHiddenRows.ods
diff --git a/sc/qa/unit/scshapetest.cxx b/sc/qa/unit/scshapetest.cxx
index c5b4b098c80e..2e9ed6281ce7 100644
--- a/sc/qa/unit/scshapetest.cxx
+++ b/sc/qa/unit/scshapetest.cxx
@@ -66,6 +66,14 @@ static SdrObject* lcl_getSdrObjectWithAssert(ScDocument& rDoc, sal_uInt16 nObjNu
     return pObj;
 }
 
+static SdrObject* lcl_getSdrObjectbyName(ScDocument& rDoc, std::u16string_view rName)
+{
+    ScDrawLayer* pDrawLayer = rDoc.GetDrawLayer();
+    const SdrPage* pPage = pDrawLayer->GetPage(0);
+    SdrObject* pObj = pPage->GetObjByName(rName);
+    return pObj;
+}
+
 CPPUNIT_TEST_FIXTURE(ScShapeTest, testTdf144242_OpenBezier_noSwapWH)
 {
     // Shapes, which have rotation incorporated in their points, got erroneously width-height
@@ -1299,6 +1307,35 @@ CPPUNIT_TEST_FIXTURE(ScShapeTest, testTdf160369_groupshape)
     CPPUNIT_ASSERT_RECTANGLE_EQUAL_WITH_TOLERANCE(aOrigRect, aAfterRect, 1);
 }
 
+CPPUNIT_TEST_FIXTURE(ScShapeTest, testTdf160329_sortWithHiddenRows)
+{
+    // Load a document, which has images anchored to cell and rows hidden
+    createScDoc("ods/tdf160329_sortWithHiddenRows.ods");
+    ScDocument* pDoc = getScDoc();
+
+    // Sort the rows
+    uno::Sequence<beans::PropertyValue> aArgs1
+        = { comphelper::makePropertyValue("DbName", u"myRange"_ustr) };
+    dispatchCommand(mxComponent, ".uno:SelectDB", aArgs1);
+    uno::Sequence<beans::PropertyValue> aArgs2
+        = { comphelper::makePropertyValue("ByRows", true),
+            comphelper::makePropertyValue("HasHeader", true),
+            comphelper::makePropertyValue("Col1", sal_Int32(1)),
+            comphelper::makePropertyValue("Ascending1", false),
+            comphelper::makePropertyValue("IncludeImages", true) };
+    dispatchCommand(mxComponent, ".uno:DataSort", aArgs2);
+
+    // Make sure objects are on correct position
+    SdrObject* pObj = lcl_getSdrObjectbyName(*pDoc, std::u16string_view(u"ImageD"));
+    Point aPos = pObj->GetSnapRect().TopLeft();
+    // The position was (3000|2899) without fix.
+    CPPUNIT_ASSERT_POINT_EQUAL_WITH_TOLERANCE(Point(3000, 5898), aPos, 1);
+    pObj = lcl_getSdrObjectbyName(*pDoc, std::u16string_view(u"ImageE"));
+    aPos = pObj->GetSnapRect().TopLeft();
+    // The position was (2600|2499) without fix.
+    CPPUNIT_ASSERT_POINT_EQUAL_WITH_TOLERANCE(Point(2600, 4399), aPos, 1);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx
index 470414d32c76..75b15b3c8c95 100644
--- a/sc/source/core/data/table3.cxx
+++ b/sc/source/core/data/table3.cxx
@@ -911,6 +911,50 @@ void ScTable::SortReorderByColumn(
     }
 }
 
+static void backupObjectsVisibility(const std::vector<std::unique_ptr<SortedColumn>>& rSortedCols,
+                                    std::vector<std::vector<std::vector<bool>>>& rBackup)
+{
+    size_t nSortedCols = rSortedCols.size();
+    for (size_t iCol = 0; iCol < nSortedCols; ++iCol)
+    {
+        std::vector<std::vector<SdrObject*>>& rSingleColCellDrawObjects
+            = rSortedCols[iCol]->maCellDrawObjects;
+        size_t nSingleColCellDrawObjects = rSingleColCellDrawObjects.size();
+        std::vector<std::vector<bool>> aColBackup;
+        for (size_t jRow = 0; jRow < nSingleColCellDrawObjects; ++jRow)
+        {
+            std::vector<SdrObject*>& rCellDrawObjects = rSingleColCellDrawObjects[jRow];
+            std::vector<bool> aCellBackup;
+            for (auto& pObject : rCellDrawObjects)
+            {
+                aCellBackup.push_back(pObject->IsVisible());
+            }
+            aColBackup.push_back(std::move(aCellBackup));
+        }
+        rBackup.push_back(std::move(aColBackup));
+    }
+}
+
+static void restoreObjectsVisibility(std::vector<std::unique_ptr<SortedColumn>>& rSortedCols,
+                                     const std::vector<std::vector<std::vector<bool>>>& rBackup)
+{
+    size_t nSortedCols = rSortedCols.size();
+    for (size_t iCol = 0; iCol < nSortedCols; ++iCol)
+    {
+        std::vector<std::vector<SdrObject*>>& rSingleColCellDrawObjects
+            = rSortedCols[iCol]->maCellDrawObjects;
+        size_t nSingleColCellDrawObjects = rSingleColCellDrawObjects.size();
+        for (size_t jRow = 0; jRow < nSingleColCellDrawObjects; jRow++)
+        {
+            std::vector<SdrObject*>& rCellDrawObjects = rSingleColCellDrawObjects[jRow];
+            for (size_t kCell = 0; kCell < rCellDrawObjects.size(); ++kCell)
+            {
+                rCellDrawObjects[kCell]->SetVisible(rBackup[iCol][jRow][kCell]);
+            }
+        }
+    }
+}
+
 void ScTable::SortReorderByRow( ScSortInfoArray* pArray, SCCOL nCol1, SCCOL nCol2,
         ScProgress* pProgress, bool bOnlyDataAreaExtras )
 {
@@ -1007,9 +1051,6 @@ void ScTable::SortReorderByRow( ScSortInfoArray* pArray, SCCOL nCol1, SCCOL nCol
             aCol[nThisCol].UpdateNoteCaptions(nRow1, nRow2);
         }
 
-        // Update draw object positions
-        aCol[nThisCol].UpdateDrawObjects(aSortedCols[i]->maCellDrawObjects, nRow1, nRow2);
-
         {
             // Get all row spans where the pattern is not NULL.
             std::vector<PatternSpan> aSpans =
@@ -1037,6 +1078,10 @@ void ScTable::SortReorderByRow( ScSortInfoArray* pArray, SCCOL nCol1, SCCOL nCol
         aRowFlags.maRowsHidden.build_tree();
         aRowFlags.maRowsFiltered.build_tree();
 
+        // Backup visibility state of objects. States will be lost when changing the flags below.
+        std::vector<std::vector<std::vector<bool>>> aBackup;
+        backupObjectsVisibility(aSortedCols, aBackup);
+
         // Remove all flags in the range first.
         SetRowHidden(nRow1, nRow2, false);
         SetRowFiltered(nRow1, nRow2, false);
@@ -1051,6 +1096,16 @@ void ScTable::SortReorderByRow( ScSortInfoArray* pArray, SCCOL nCol1, SCCOL nCol
 
         for (const auto& rSpan : aSpans)
             SetRowFiltered(rSpan.mnRow1, rSpan.mnRow2, true);
+
+        //Restore visibility state of objects
+        restoreObjectsVisibility(aSortedCols, aBackup);
+    }
+
+    // Update draw object positions
+    for (size_t i = 0, n = aSortedCols.size(); i < n; ++i)
+    {
+        SCCOL nThisCol = i + nCol1;
+        aCol[nThisCol].UpdateDrawObjects(aSortedCols[i]->maCellDrawObjects, nRow1, nRow2);
     }
 
     // Notify the cells' listeners to (re-)start listening.
@@ -1208,9 +1263,6 @@ void ScTable::SortReorderByRowRefUpdate(
             aCol[nThisCol].UpdateNoteCaptions(nRow1, nRow2);
         }
 
-        // Update draw object positions
-        aCol[nThisCol].UpdateDrawObjects(aSortedCols[i]->maCellDrawObjects, nRow1, nRow2);
-
         {
             // Get all row spans where the pattern is not NULL.
             std::vector<PatternSpan> aSpans =
@@ -1238,6 +1290,10 @@ void ScTable::SortReorderByRowRefUpdate(
         aRowFlags.maRowsHidden.build_tree();
         aRowFlags.maRowsFiltered.build_tree();
 
+        // Backup visibility state of objects. States will be lost when changing the flags below.
+        std::vector<std::vector<std::vector<bool>>> aBackup;
+        backupObjectsVisibility(aSortedCols, aBackup);
+
         // Remove all flags in the range first.
         SetRowHidden(nRow1, nRow2, false);
         SetRowFiltered(nRow1, nRow2, false);
@@ -1252,6 +1308,16 @@ void ScTable::SortReorderByRowRefUpdate(
 
         for (const auto& rSpan : aSpans)
             SetRowFiltered(rSpan.mnRow1, rSpan.mnRow2, true);
+
+        //Restore visibility state of objects
+        restoreObjectsVisibility(aSortedCols, aBackup);
+    }
+
+    // Update draw object positions
+    for (size_t i = 0, n = aSortedCols.size(); i < n; ++i)
+    {
+        SCCOL nThisCol = i + nCol1;
+        aCol[nThisCol].UpdateDrawObjects(aSortedCols[i]->maCellDrawObjects, nRow1, nRow2);
     }
 
     // Set up row reorder map (for later broadcasting of reference updates).