sc/CppunitTest_sc_basic_types_test.mk | 4 - sc/inc/SheetView.hxx | 37 +++++++++- sc/inc/SheetViewManager.hxx | 6 - sc/qa/unit/tiledrendering/SheetViewTest.cxx | 54 ++++++++++++++ sc/qa/unit/types/SortOrderReverserTest.cxx | 102 ++++++++++++++++++++++++++++ sc/source/core/data/SheetView.cxx | 60 +++++++++++----- sc/source/core/data/SheetViewManager.cxx | 34 --------- sc/source/ui/view/viewfunc.cxx | 9 +- 8 files changed, 247 insertions(+), 59 deletions(-)
New commits: commit 9eef653230649d29187beae813b8d17c526df3e1 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Fri Sep 12 23:41:17 2025 +0200 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Mon Feb 23 13:02:54 2026 +0100 sc: synching when default view and sheet view are sorted In this scenario we have sheet view and the default view sorted in different ways, so we need to first unsort the sheet view sort, and then resort the default view sort so we get the correct cell address of the cell that we are changing. Resort is the reverse of unsort. We need to search where in the sort order array is the value of the input row. The array index will be the result and the first row + index will give us the resorted row address. Change-Id: I08dd7f6fc5593464cb2e6bd36ffb3ff250cebf8d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190912 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200016 Tested-by: Jenkins CollaboraOffice <[email protected]> diff --git a/sc/inc/SheetView.hxx b/sc/inc/SheetView.hxx index 9cbc5d7489f7..32972acecfcb 100644 --- a/sc/inc/SheetView.hxx +++ b/sc/inc/SheetView.hxx @@ -33,6 +33,7 @@ public: * or it will return the input row without modification. **/ SCROW unsort(SCROW nRow) const; + SCROW resort(SCROW nRow) const; /** Adds or combines the order indices. * diff --git a/sc/qa/unit/tiledrendering/SheetViewTest.cxx b/sc/qa/unit/tiledrendering/SheetViewTest.cxx index a17ee4c5e3fe..3f80cf13bba7 100644 --- a/sc/qa/unit/tiledrendering/SheetViewTest.cxx +++ b/sc/qa/unit/tiledrendering/SheetViewTest.cxx @@ -870,6 +870,60 @@ CPPUNIT_TEST_FIXTURE(SheetViewTest, testSyncAfterSorting_SheetViewSort) } } +CPPUNIT_TEST_FIXTURE(SheetViewTest, testSyncAfterSorting_SortInDefaultAndSheetView) +{ + // Create two views, and leave the second one current. + ScModelObj* pModelObj = createDoc("SheetView_AutoFilter.ods"); + pModelObj->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>()); + + // Setup views + ScTestViewCallback aSheetView; + ScTabViewShell* pTabViewSheetView = aSheetView.getTabViewShell(); + + SfxLokHelper::createView(); + Scheduler::ProcessEventsToIdle(); + + ScTestViewCallback aDefaultView; + ScTabViewShell* pTabViewDefaultView = aDefaultView.getTabViewShell(); + + CPPUNIT_ASSERT(pTabViewSheetView != pTabViewDefaultView); + CPPUNIT_ASSERT(aSheetView.getViewID() != aDefaultView.getViewID()); + + // Switch to Sheet View and Create + { + SfxLokHelper::setView(aSheetView.getViewID()); + Scheduler::ProcessEventsToIdle(); + + dispatchCommand(mxComponent, u".uno:NewSheetView"_ustr, {}); + + // Sort AutoFilter + dispatchCommand(mxComponent, u".uno:SortAscending"_ustr, {}); + } + + // Switch to Sheet View and Create + { + SfxLokHelper::setView(aDefaultView.getViewID()); + Scheduler::ProcessEventsToIdle(); + + // Sort AutoFilter + dispatchCommand(mxComponent, u".uno:SortDescending"_ustr, {}); + + // Check values + CPPUNIT_ASSERT(checkValues(pTabViewSheetView, 0, 1, 4, { u"3", u"4", u"5", u"7" })); + CPPUNIT_ASSERT(checkValues(pTabViewDefaultView, 0, 1, 4, { u"7", u"5", u"4", u"3" })); + } + + { + SfxLokHelper::setView(aSheetView.getViewID()); + Scheduler::ProcessEventsToIdle(); + + typeCharsInCell(std::string("44"), 0, 2, pTabViewSheetView, pModelObj); + + CPPUNIT_ASSERT(checkValues(pTabViewSheetView, 0, 1, 4, { u"3", u"44", u"5", u"7" })); + CPPUNIT_ASSERT(checkValues(pTabViewDefaultView, 0, 1, 4, { u"7", u"5", u"44", u"3" })); + } +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/qa/unit/types/SortOrderReverserTest.cxx b/sc/qa/unit/types/SortOrderReverserTest.cxx index 0e550e1742cf..913a8db21846 100644 --- a/sc/qa/unit/types/SortOrderReverserTest.cxx +++ b/sc/qa/unit/types/SortOrderReverserTest.cxx @@ -76,4 +76,27 @@ CPPUNIT_TEST_FIXTURE(SortOrderReverserTest, testCombiningSortOrder) std::equal(aExpectedOrder.begin(), aExpectedOrder.end(), aReverser.maOrder.begin())); } +CPPUNIT_TEST_FIXTURE(SortOrderReverserTest, testResort) +{ + { + sc::SortOrderReverser aReverser; + aReverser.addOrderIndices({ 4, 3, 2, 1 }, 5, 8); + + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.resort(5)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.resort(6)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.resort(7)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.resort(8)); + } + + { + sc::SortOrderReverser aReverser; + aReverser.addOrderIndices({ 3, 1, 4, 2 }, 5, 8); + + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.resort(5)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.resort(6)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.resort(7)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.resort(8)); + } +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/core/data/SheetView.cxx b/sc/source/core/data/SheetView.cxx index f70a1000c272..b944b325e9d3 100644 --- a/sc/source/core/data/SheetView.cxx +++ b/sc/source/core/data/SheetView.cxx @@ -49,6 +49,23 @@ SCROW SortOrderReverser::unsort(SCROW nRow) const return nRow; } +SCROW SortOrderReverser::resort(SCROW nRow) const +{ + if (maOrder.empty()) + return nRow; + + if (nRow >= mnFirstRow && nRow <= mnLastRow) + { + SCCOLROW nOrderValue = nRow - mnFirstRow + 1; + for (size_t nIndex = 0; nIndex < maOrder.size(); ++nIndex) + { + if (maOrder[nIndex] == nOrderValue) + return mnFirstRow + nIndex; + } + } + return nRow; +} + SheetView::SheetView(ScTable* pTable, OUString const& rName, SheetViewID nID) : mpTable(pTable) , maName(rName) diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx index 8b01fdde87bc..a7f06c4ab712 100644 --- a/sc/source/ui/view/viewfunc.cxx +++ b/sc/source/ui/view/viewfunc.cxx @@ -877,7 +877,7 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB nTab, if (pSheetView->getSortOrder()) nUnsortedRow = pSheetView->getSortOrder()->unsort(nUnsortedRow); if (pManager->getSortOrder()) - nUnsortedRow = pManager->getSortOrder()->unsort(nUnsortedRow); + nUnsortedRow = pManager->getSortOrder()->resort(nUnsortedRow); } applyText(*this, nCol, nUnsortedRow, rTab, rString, bNumFmtChanged); } commit d3f11dae8b0678a9ce44b7a29e5ddcb096929b90 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Fri Sep 12 23:14:52 2025 +0200 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Mon Feb 23 13:02:47 2026 +0100 sc: add unit test for SortOrderReverser Add the unit test to the basic types test suite. The unit test checks the unosrt function when using different sort orders. It also checks combining the sort orders with addOrderIndices function. Additionally move the existing ScAddressTest to the "types" folder. Change-Id: I298918b971e3386b8ea513c09879045e25010ad9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190911 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200015 Tested-by: Jenkins CollaboraOffice <[email protected]> diff --git a/sc/CppunitTest_sc_basic_types_test.mk b/sc/CppunitTest_sc_basic_types_test.mk index c9ef8faaaa1a..5a417a22b7c8 100644 --- a/sc/CppunitTest_sc_basic_types_test.mk +++ b/sc/CppunitTest_sc_basic_types_test.mk @@ -13,6 +13,7 @@ $(eval $(call gb_CppunitTest_CppunitTest,sc_basic_types_test)) $(eval $(call gb_CppunitTest_add_exception_objects,sc_basic_types_test,\ sc/qa/unit/types/ScAddressTest \ + sc/qa/unit/types/SortOrderReverserTest \ )) $(eval $(call gb_CppunitTest_use_external,sc_basic_types_test,boost_headers)) @@ -25,7 +26,8 @@ $(eval $(call gb_CppunitTest_set_include,sc_basic_types_test,\ )) $(eval $(call gb_CppunitTest_use_libraries,sc_basic_types_test,\ - sal \ + sc \ + scui \ )) # Needed for include/test/cppunitasserthelper.hxx diff --git a/sc/inc/SheetView.hxx b/sc/inc/SheetView.hxx index 3a45450c9d79..9cbc5d7489f7 100644 --- a/sc/inc/SheetView.hxx +++ b/sc/inc/SheetView.hxx @@ -20,7 +20,7 @@ class ScTable; namespace sc { /** Stores the sort order and can reverse the sorting of rows (unsorting). */ -struct SortOrderReverser +struct SC_DLLPUBLIC SortOrderReverser { public: SCROW mnFirstRow; diff --git a/sc/qa/unit/types/SortOrderReverserTest.cxx b/sc/qa/unit/types/SortOrderReverserTest.cxx new file mode 100644 index 000000000000..0e550e1742cf --- /dev/null +++ b/sc/qa/unit/types/SortOrderReverserTest.cxx @@ -0,0 +1,79 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <cppunit/plugin/TestPlugIn.h> +#include <cppunit/TestAssert.h> +#include <cppunit/TestFixture.h> +#include <cppunit/extensions/HelperMacros.h> + +#include <SheetView.hxx> + +class SortOrderReverserTest : public CppUnit::TestFixture +{ +}; + +CPPUNIT_TEST_FIXTURE(SortOrderReverserTest, testUnsort) +{ + // Unchanged order - output rows should be the same as input + { + sc::SortOrderReverser aReverser; + aReverser.addOrderIndices({ 1, 2, 3, 4 }, 5, 8); + + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(5)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(6)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(7)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(8)); + } + + // Reversed order - output rows should be in reverse order than input + { + sc::SortOrderReverser aReverser; + aReverser.addOrderIndices({ 4, 3, 2, 1 }, 5, 8); + + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(5)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(6)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(7)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(8)); + } + + // Some random order - output rows should follow the order + { + sc::SortOrderReverser aReverser; + aReverser.addOrderIndices({ 3, 1, 4, 2 }, 5, 8); + + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(5)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(6)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(7)); + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(8)); + } +} + +CPPUNIT_TEST_FIXTURE(SortOrderReverserTest, testCombiningSortOrder) +{ + sc::SortOrderReverser aReverser; + + // first sorting order + aReverser.addOrderIndices({ 3, 1, 4, 2 }, 5, 8); + + // second sorting order - only reverses the order + aReverser.addOrderIndices({ 4, 3, 2, 1 }, 5, 8); + + // check + CPPUNIT_ASSERT_EQUAL(SCROW(6), aReverser.unsort(5)); + CPPUNIT_ASSERT_EQUAL(SCROW(8), aReverser.unsort(6)); + CPPUNIT_ASSERT_EQUAL(SCROW(5), aReverser.unsort(7)); + CPPUNIT_ASSERT_EQUAL(SCROW(7), aReverser.unsort(8)); + + // directly compare the order + std::vector<SCCOLROW> aExpectedOrder{ 2, 4, 1, 3 }; + CPPUNIT_ASSERT( + std::equal(aExpectedOrder.begin(), aExpectedOrder.end(), aReverser.maOrder.begin())); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ commit 20fa8ca548f7c219064532c3cb3c621161ec8ba2 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Fri Sep 12 21:57:29 2025 +0200 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Mon Feb 23 13:02:41 2026 +0100 sc: introduce SortOrderReverser to handle sort order conversion SortOrderReverser class handles the reversing of the sort order, which is needed on the sheet view and default view level and is more or less duplicated code in SheetView and SheetViewManager. With SortOrderReverser it is possible to remove the duplication and write a unit test just to test reversing the sort. Change-Id: I7a0726bf71e0b25bcf68403b7c13c3dfac889dd5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190910 Reviewed-by: Tomaž Vajngerl <[email protected]> Tested-by: Jenkins Reviewed-on: https://gerrit.libreoffice.org/c/core/+/200014 Tested-by: Tomaž Vajngerl <[email protected]> diff --git a/sc/inc/SheetView.hxx b/sc/inc/SheetView.hxx index 5c36ddf01fc8..3a45450c9d79 100644 --- a/sc/inc/SheetView.hxx +++ b/sc/inc/SheetView.hxx @@ -13,11 +13,35 @@ #include "types.hxx" #include <rtl/ustring.hxx> #include "SheetViewTypes.hxx" +#include <optional> class ScTable; namespace sc { +/** Stores the sort order and can reverse the sorting of rows (unsorting). */ +struct SortOrderReverser +{ +public: + SCROW mnFirstRow; + SCROW mnLastRow; + std::vector<SCCOLROW> maOrder; + + /** Reverse the sort for the input row and output the unsorted row. + * + * Uses the sort order. The row must be between range of [first row, last row] + * or it will return the input row without modification. + **/ + SCROW unsort(SCROW nRow) const; + + /** Adds or combines the order indices. + * + * Adds the indices if none are present, or combines the indices if the order indices + * were already added previously. + **/ + void addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, SCROW lastRow); +}; + /** Stores information of a sheet view. * * A sheet view is a special view of a sheet that can be filtered and sorted @@ -30,9 +54,7 @@ private: bool mbSynced = true; OUString maName; - SCROW mnFirstRow; - SCROW mnLastRow; - std::vector<SCCOLROW> maOrder; + std::optional<SortOrderReverser> moSortOrder; SheetViewID mnID; public: @@ -53,8 +75,14 @@ public: /** Is the sheet view synced with its default view. */ bool isSynced() const { return mbSynced; } + std::optional<SortOrderReverser> const& getSortOrder() const { return moSortOrder; } + + /** Adds or combines the order indices. + * + * Adds the indices if none are present, or combines the indices if the order indices + * were already added previously. + **/ void addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, SCROW lastRow); - SCROW unsort(SCROW nRow) const; }; } diff --git a/sc/inc/SheetViewManager.hxx b/sc/inc/SheetViewManager.hxx index a0e6527b1d11..205f08feb682 100644 --- a/sc/inc/SheetViewManager.hxx +++ b/sc/inc/SheetViewManager.hxx @@ -28,9 +28,7 @@ private: std::vector<std::shared_ptr<SheetView>> maViews; sal_Int32 maNameCounter = 0; - std::vector<SCCOLROW> maOrder; - SCROW mnFirstRow; - SCROW mnLastRow; + std::optional<SortOrderReverser> moSortOrder; bool isValidSheetViewID(SheetViewID nID) const { @@ -70,8 +68,8 @@ public: static OUString defaultViewName(); + std::optional<SortOrderReverser> const& getSortOrder() const { return moSortOrder; } void addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, SCROW lastRow); - SCROW unsort(SCROW nRow); }; } diff --git a/sc/source/core/data/SheetView.cxx b/sc/source/core/data/SheetView.cxx index a7b103deeb7c..f70a1000c272 100644 --- a/sc/source/core/data/SheetView.cxx +++ b/sc/source/core/data/SheetView.cxx @@ -12,25 +12,12 @@ namespace sc { -SheetView::SheetView(ScTable* pTable, OUString const& rName, SheetViewID nID) - : mpTable(pTable) - , maName(rName) - , mnID(nID) -{ -} - -ScTable* SheetView::getTablePointer() const { return mpTable; } - -SCTAB SheetView::getTableNumber() const -{ - assert(mpTable); - return mpTable->GetTab(); -} - -void SheetView::addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, SCROW lastRow) +void SortOrderReverser::addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, + SCROW lastRow) { mnFirstRow = firstRow; mnLastRow = lastRow; + if (maOrder.empty()) { maOrder = rOrder; @@ -48,7 +35,7 @@ void SheetView::addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW first } } -SCROW SheetView::unsort(SCROW nRow) const +SCROW SortOrderReverser::unsort(SCROW nRow) const { if (maOrder.empty()) return nRow; @@ -61,6 +48,28 @@ SCROW SheetView::unsort(SCROW nRow) const } return nRow; } + +SheetView::SheetView(ScTable* pTable, OUString const& rName, SheetViewID nID) + : mpTable(pTable) + , maName(rName) + , mnID(nID) +{ +} + +ScTable* SheetView::getTablePointer() const { return mpTable; } + +SCTAB SheetView::getTableNumber() const +{ + assert(mpTable); + return mpTable->GetTab(); +} + +void SheetView::addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, SCROW lastRow) +{ + if (!moSortOrder) + moSortOrder.emplace(); + moSortOrder->addOrderIndices(rOrder, firstRow, lastRow); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/core/data/SheetViewManager.cxx b/sc/source/core/data/SheetViewManager.cxx index edc4afdf8558..bbc5d63dd7ac 100644 --- a/sc/source/core/data/SheetViewManager.cxx +++ b/sc/source/core/data/SheetViewManager.cxx @@ -138,37 +138,9 @@ OUString SheetViewManager::defaultViewName() { return ScResId(STR_SHEET_VIEW_DEF void SheetViewManager::addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW nFirstRow, SCROW nLastRow) { - mnFirstRow = nFirstRow; - mnLastRow = nLastRow; - if (maOrder.empty()) - { - maOrder = rOrder; - } - else - { - assert(maOrder.size() == rOrder.size()); - std::vector<SCCOLROW> newOrder(maOrder.size()); - for (size_t nIndex = 0; nIndex < maOrder.size(); ++nIndex) - { - size_t nSortedIndex = rOrder[nIndex]; - newOrder[nIndex] = maOrder[nSortedIndex - 1]; - } - maOrder = newOrder; - } -} - -SCROW SheetViewManager::unsort(SCROW nRow) -{ - if (maOrder.empty()) - return nRow; - - if (nRow >= mnFirstRow && nRow <= mnLastRow) - { - size_t index = nRow - mnFirstRow; - auto nUnsortedRow = mnFirstRow + maOrder[index] - 1; - return nUnsortedRow; - } - return nRow; + if (!moSortOrder) + moSortOrder.emplace(); + moSortOrder->addOrderIndices(rOrder, nFirstRow, nLastRow); } } diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx index bc9d68bfb4db..8b01fdde87bc 100644 --- a/sc/source/ui/view/viewfunc.cxx +++ b/sc/source/ui/view/viewfunc.cxx @@ -853,7 +853,8 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB nTab, if (GetViewData().GetSheetViewID() == sc::DefaultSheetViewID) { SCROW nUnsortedRow = nRow; - nUnsortedRow = pManager->unsort(nUnsortedRow); + if (pManager->getSortOrder()) + nUnsortedRow = pManager->getSortOrder()->unsort(nUnsortedRow); applyText(*this, nCol, nUnsortedRow, nSheetViewTab, rString, bNumFmtChanged); } else if (GetViewData().GetSheetViewID() == pSheetView->getID()) @@ -872,7 +873,11 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB nTab, if (nSheetViewID != sc::DefaultSheetViewID) { auto pSheetView = pManager->get(nSheetViewID); - nUnsortedRow = pSheetView->unsort(nUnsortedRow); + + if (pSheetView->getSortOrder()) + nUnsortedRow = pSheetView->getSortOrder()->unsort(nUnsortedRow); + if (pManager->getSortOrder()) + nUnsortedRow = pManager->getSortOrder()->unsort(nUnsortedRow); } applyText(*this, nCol, nUnsortedRow, rTab, rString, bNumFmtChanged); }
