sc/qa/unit/data/ods/tdf160329_sortWithHiddenRows.ods |binary
 sc/qa/unit/scshapetest.cxx                           |   37 +++++++++
 sc/source/core/data/table3.cxx                       |   78 +++++++++++++++++--
 3 files changed, 109 insertions(+), 6 deletions(-)

New commits:
commit f0a2969d15e3101d7f96a7fe77bca06a5d70f57a
Author:     Regina Henschel <rb.hensc...@t-online.de>
AuthorDate: Sat Apr 27 19:25:32 2024 +0200
Commit:     Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Wed May 1 09:12:59 2024 +0200

    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.gran...@collabora.co.uk>
    Tested-by: Noel Grandin <noel.gran...@collabora.co.uk>

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
Binary files /dev/null and 
b/sc/qa/unit/data/ods/tdf160329_sortWithHiddenRows.ods differ
diff --git a/sc/qa/unit/scshapetest.cxx b/sc/qa/unit/scshapetest.cxx
index 5aa24ede6ea6..d434084519f3 100644
--- a/sc/qa/unit/scshapetest.cxx
+++ b/sc/qa/unit/scshapetest.cxx
@@ -62,6 +62,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
@@ -1295,6 +1303,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 4e06aca8231a..ee00f2d58161 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 =
@@ -1030,6 +1071,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);
@@ -1044,6 +1089,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.
@@ -1201,9 +1256,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 =
@@ -1224,6 +1276,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);
@@ -1238,6 +1294,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).

Reply via email to