sc/inc/SheetView.hxx | 40 +++++++++++++ sc/inc/SheetViewManager.hxx | 6 -- sc/qa/unit/tiledrendering/SheetViewTest.cxx | 63 +++++++++++++++++++++ sc/source/core/data/SheetView.cxx | 47 +++++++++++++++- sc/source/core/data/SheetViewManager.cxx | 36 +----------- sc/source/core/data/table3.cxx | 10 +++ sc/source/ui/view/viewfunc.cxx | 81 +++++++++++++++++++--------- 7 files changed, 219 insertions(+), 64 deletions(-)
New commits: commit ca8a3549eafeaf64ed0e62bc05e4dcd8f13349f8 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Fri Sep 12 21:57:29 2025 +0200 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Wed Dec 17 07:16:37 2025 +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 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 8bcb1938a794..a0ef1164293d 100644 --- a/sc/source/core/data/SheetViewManager.cxx +++ b/sc/source/core/data/SheetViewManager.cxx @@ -133,37 +133,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); } commit 9b907c8fd529dd5737f9359d9bcb64240e4c6747 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Fri Sep 12 16:13:54 2025 +0200 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Wed Dec 17 07:16:29 2025 +0100 sc: unsort when synching a change from sheet view to default view If we sort in sheet view and change the value in sheet view, we need to reverse the sort when syncing the value with the default view. When a sheet view is sorted, we store the order inside SheetView. If we sort again, we need to combine the existing sort order with the new sort order (sort asc. and then sort desc). We also need sheet view info in ScTable of a sheet view, so we can properly set the sort order. This sent the information when we create the sheet view. Change-Id: I4d7f2c8add3f137025bcc9c6f29482795fa32ccb Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190891 Reviewed-by: Tomaž Vajngerl <[email protected]> Tested-by: Jenkins diff --git a/sc/inc/SheetView.hxx b/sc/inc/SheetView.hxx index bff043b55812..5c36ddf01fc8 100644 --- a/sc/inc/SheetView.hxx +++ b/sc/inc/SheetView.hxx @@ -12,6 +12,7 @@ #include "scdllapi.h" #include "types.hxx" #include <rtl/ustring.hxx> +#include "SheetViewTypes.hxx" class ScTable; @@ -29,11 +30,17 @@ private: bool mbSynced = true; OUString maName; + SCROW mnFirstRow; + SCROW mnLastRow; + std::vector<SCCOLROW> maOrder; + SheetViewID mnID; + public: - SheetView(ScTable* pTable, OUString const& rName); + SheetView(ScTable* pTable, OUString const& rName, SheetViewID nID); ScTable* getTablePointer() const; SCTAB getTableNumber() const; + SheetViewID getID() const { return mnID; } OUString const& GetName() { return maName; } @@ -45,6 +52,9 @@ public: /** Is the sheet view synced with its default view. */ bool isSynced() const { return mbSynced; } + + void addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, SCROW lastRow); + SCROW unsort(SCROW nRow) const; }; } diff --git a/sc/qa/unit/tiledrendering/SheetViewTest.cxx b/sc/qa/unit/tiledrendering/SheetViewTest.cxx index 9a59c2733bab..a17ee4c5e3fe 100644 --- a/sc/qa/unit/tiledrendering/SheetViewTest.cxx +++ b/sc/qa/unit/tiledrendering/SheetViewTest.cxx @@ -807,6 +807,69 @@ CPPUNIT_TEST_FIXTURE(SheetViewTest, testSyncAfterSorting_DefaultViewSort) } } +CPPUNIT_TEST_FIXTURE(SheetViewTest, testSyncAfterSorting_SheetViewSort) +{ + // Two related scenarios tested: + // + // 1. Auto-filter is sorted in the sheet view, then the data is changed in a sheet view. + // In this case the default view is unsorted and the sheet view is sorted, so the data + // in the sheet view needs to be first unsorted so we get the correct position of the + // cell that needs to be changed. + // 2. Continuation of scenario 1, where the sheet view is sorted again (ascending then + // descending order). In this case the sort orders must be combined correctly. + + // 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(); + + // New Sheet view + dispatchCommand(mxComponent, u".uno:NewSheetView"_ustr, {}); + + // Sort AutoFilter + dispatchCommand(mxComponent, u".uno:SortAscending"_ustr, {}); + + // Check values - Sheet View + CPPUNIT_ASSERT(checkValues(pTabViewSheetView, 0, 1, 4, { u"3", u"4", u"5", u"7" })); + CPPUNIT_ASSERT(checkValues(pTabViewDefaultView, 0, 1, 4, { u"4", u"5", u"3", u"7" })); + + typeCharsInCell(std::string("9"), 0, 1, pTabViewSheetView, pModelObj); + + CPPUNIT_ASSERT(checkValues(pTabViewSheetView, 0, 1, 4, { u"9", u"4", u"5", u"7" })); + CPPUNIT_ASSERT(checkValues(pTabViewDefaultView, 0, 1, 4, { u"4", u"5", u"9", u"7" })); + + // Scenario 2 + + // Sort AutoFilter + dispatchCommand(mxComponent, u".uno:SortDescending"_ustr, {}); + + CPPUNIT_ASSERT(checkValues(pTabViewSheetView, 0, 1, 4, { u"9", u"7", u"5", u"4" })); + CPPUNIT_ASSERT(checkValues(pTabViewDefaultView, 0, 1, 4, { u"4", u"5", u"9", u"7" })); + + typeCharsInCell(std::string("6"), 0, 3, pTabViewSheetView, pModelObj); + + CPPUNIT_ASSERT(checkValues(pTabViewSheetView, 0, 1, 4, { u"9", u"7", u"6", u"4" })); + CPPUNIT_ASSERT(checkValues(pTabViewDefaultView, 0, 1, 4, { u"4", u"6", u"9", u"7" })); + } +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/core/data/SheetView.cxx b/sc/source/core/data/SheetView.cxx index 0c51d5ea6678..a7b103deeb7c 100644 --- a/sc/source/core/data/SheetView.cxx +++ b/sc/source/core/data/SheetView.cxx @@ -12,9 +12,10 @@ namespace sc { -SheetView::SheetView(ScTable* pTable, OUString const& rName) +SheetView::SheetView(ScTable* pTable, OUString const& rName, SheetViewID nID) : mpTable(pTable) , maName(rName) + , mnID(nID) { } @@ -25,6 +26,41 @@ SCTAB SheetView::getTableNumber() const assert(mpTable); return mpTable->GetTab(); } + +void SheetView::addOrderIndices(std::vector<SCCOLROW> const& rOrder, SCROW firstRow, SCROW lastRow) +{ + mnFirstRow = firstRow; + mnLastRow = lastRow; + 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 SheetView::unsort(SCROW nRow) const +{ + 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; +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/core/data/SheetViewManager.cxx b/sc/source/core/data/SheetViewManager.cxx index cd86986236b0..8bcb1938a794 100644 --- a/sc/source/core/data/SheetViewManager.cxx +++ b/sc/source/core/data/SheetViewManager.cxx @@ -19,7 +19,7 @@ SheetViewManager::SheetViewManager() {} SheetViewID SheetViewManager::create(ScTable* pSheetViewTable) { SheetViewID nID(maViews.size()); - maViews.emplace_back(std::make_shared<SheetView>(pSheetViewTable, generateName())); + maViews.emplace_back(std::make_shared<SheetView>(pSheetViewTable, generateName(), nID)); return nID; } diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx index 6abdd60af189..e50074176af8 100644 --- a/sc/source/core/data/table3.cxx +++ b/sc/source/core/data/table3.cxx @@ -1807,7 +1807,15 @@ void ScTable::Sort( bool bAutoFilter = GetDoc().HasAutoFilter(rSortParam.nCol1, nRow1, GetTab()); if (bAutoFilter) { - GetSheetViewManager()->addOrderIndices(pArray->GetOrderIndices(), nRow1, nLastRow); + if (IsSheetViewHolder()) + { + auto pSheetView = mpSheetViewFor->GetSheetViewManager()->get(mnSheetViewID); + pSheetView->addOrderIndices(pArray->GetOrderIndices(), nRow1, nLastRow); + } + else + { + GetSheetViewManager()->addOrderIndices(pArray->GetOrderIndices(), nRow1, nLastRow); + } } } } diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx index 3b61a0b18f0c..bc9d68bfb4db 100644 --- a/sc/source/ui/view/viewfunc.cxx +++ b/sc/source/ui/view/viewfunc.cxx @@ -811,47 +811,73 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB nTab, applyFormulaToCell(*this, nCol, nRow, nTab, rString, pData, xModificator, aMark, bMatrixExpand, bRecord, bNumFmtChanged); - if (!rDoc.IsSheetViewHolder(nSelectedTab)) - { - auto pManager = rDoc.GetSheetViewManager(nSelectedTab); + if (rDoc.IsSheetViewHolder(nSelectedTab)) + return; - for (auto const& pSheetView : pManager->getSheetViews()) - { - if (!pSheetView) - continue; + auto pManager = rDoc.GetSheetViewManager(nSelectedTab); - SCTAB nSheetViewTab = pSheetView->getTableNumber(); + for (auto const& pSheetView : pManager->getSheetViews()) + { + if (!pSheetView) + continue; - ScMarkData aSheetViewMark(rDoc.GetSheetLimits()); - aSheetViewMark.SelectTable(nSheetViewTab, false); - ScRange aSheetViewRange(aMark.GetMarkArea()); - aSheetViewRange.aStart.SetTab(nSheetViewTab); - aSheetViewRange.aEnd.SetTab(nSheetViewTab); - aSheetViewMark.SetMarkArea(aSheetViewRange); + SCTAB nSheetViewTab = pSheetView->getTableNumber(); - applyFormulaToCell(*this, nCol, nRow, nSheetViewTab, rString, pData, xModificator, aSheetViewMark, bMatrixExpand, bRecord, bNumFmtChanged); - } + ScMarkData aSheetViewMark(rDoc.GetSheetLimits()); + aSheetViewMark.SelectTable(nSheetViewTab, false); + ScRange aSheetViewRange(aMark.GetMarkArea()); + aSheetViewRange.aStart.SetTab(nSheetViewTab); + aSheetViewRange.aEnd.SetTab(nSheetViewTab); + aSheetViewMark.SetMarkArea(aSheetViewRange); + + applyFormulaToCell(*this, nCol, nRow, nSheetViewTab, rString, pData, xModificator, aSheetViewMark, bMatrixExpand, bRecord, bNumFmtChanged); } } else { + sc::SheetViewID nSheetViewID = GetViewData().GetSheetViewID(); for (const auto& rTab : aMark) { - if (!rDoc.IsSheetViewHolder(rTab)) + if (rDoc.IsSheetViewHolder(rTab)) + continue; + + auto pManager = rDoc.GetSheetViewManager(rTab); + + for (auto const& pSheetView : pManager->getSheetViews()) { - auto pManager = rDoc.GetSheetViewManager(rTab); - for (auto const& pSheetView : pManager->getSheetViews()) - { - if (!pSheetView) - continue; + if (!pSheetView) + continue; - SCTAB nSheetViewTab = pSheetView->getTableNumber(); - SCROW nUnsortedRow = pManager->unsort(nRow); + SCTAB nSheetViewTab = pSheetView->getTableNumber(); + + if (GetViewData().GetSheetViewID() == sc::DefaultSheetViewID) + { + SCROW nUnsortedRow = nRow; + nUnsortedRow = pManager->unsort(nUnsortedRow); applyText(*this, nCol, nUnsortedRow, nSheetViewTab, rString, bNumFmtChanged); } + else if (GetViewData().GetSheetViewID() == pSheetView->getID()) + { + applyText(*this, nCol, nRow, nSheetViewTab, rString, bNumFmtChanged); + } + else + { + // TODO - we need to update other sheet views as well, test case needed + } + } + + { + SCROW nUnsortedRow = nRow; + + if (nSheetViewID != sc::DefaultSheetViewID) + { + auto pSheetView = pManager->get(nSheetViewID); + nUnsortedRow = pSheetView->unsort(nUnsortedRow); + } + applyText(*this, nCol, nUnsortedRow, rTab, rString, bNumFmtChanged); } - applyText(*this, nCol, nRow, rTab, rString, bNumFmtChanged); } + performAutoFormatAndUpdate(rString, aMark, nCol, nRow, nTab, bNumFmtChanged, bRecord, xModificator, *this); } }
