sc/inc/SparklineList.hxx | 1 sc/inc/table.hxx | 1 sc/qa/unit/SparklineTest.cxx | 114 +++++++++++++++++++++++------- sc/source/core/data/column2.cxx | 38 +++++++++- sc/source/core/data/table2.cxx | 2 sc/source/core/data/table4.cxx | 56 ++++++++++++++ sc/source/ui/sparklines/SparklineList.cxx | 24 ++++++ 7 files changed, 208 insertions(+), 28 deletions(-)
New commits: commit 36ffd9022ff988f9f10e3bcc9b2b2a4f982b74bc Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk> AuthorDate: Mon Apr 4 00:56:47 2022 +0900 Commit: Tomaž Vajngerl <qui...@gmail.com> CommitDate: Thu Apr 7 10:36:08 2022 +0200 sc: take sparklines into account with auto fill Change-Id: I6bdb5f4291aece7ec02d8de0731b8f983b4f2bb2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132592 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl <qui...@gmail.com> diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx index ca42780c71b1..de8f9c231077 100644 --- a/sc/inc/table.hxx +++ b/sc/inc/table.hxx @@ -479,6 +479,7 @@ public: sc::SparklineList& GetSparklineList(); void CopySparklinesToTable(SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, ScTable* pDestTab); + void FillSparkline(bool bVertical, SCCOLROW nFixed, SCCOLROW nIteratingStart, SCCOLROW nIteratingEnd, SCCOLROW nFillStart, SCCOLROW nFillEnd); // Notes / Comments std::unique_ptr<ScPostIt> ReleaseNote( SCCOL nCol, SCROW nRow ); diff --git a/sc/source/core/data/table4.cxx b/sc/source/core/data/table4.cxx index ba322db9ed57..9f2b26cff25e 100644 --- a/sc/source/core/data/table4.cxx +++ b/sc/source/core/data/table4.cxx @@ -1204,10 +1204,66 @@ void ScTable::FillAuto( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, nProgress = pProgress->GetState(); } + if (bVertical) + FillSparkline(bVertical, nCol, nRow1, nRow2, nIStart, nIEnd); + else + FillSparkline(bVertical, nRow, nCol1, nCol2, nIStart, nIEnd); + nActFormCnt += nMaxFormCnt; } } +void ScTable::FillSparkline(bool bVertical, SCCOLROW nFixed, + SCCOLROW nStart, SCCOLROW nEnd, + SCCOLROW nFillStart, SCCOLROW nFillEnd) +{ + bool bHasSparklines = false; + std::vector<std::shared_ptr<sc::Sparkline>> aSparklineSeries; + + for (SCROW nCurrent = nStart; nCurrent <= nEnd; nCurrent++) + { + auto pSparkline = bVertical ? GetSparkline(nFixed, nCurrent) : GetSparkline(nCurrent, nFixed); + bHasSparklines = bHasSparklines || bool(pSparkline); + aSparklineSeries.push_back(pSparkline); + } + + if (bHasSparklines) + { + for (SCCOLROW nCurrent = nFillStart; nCurrent <= nFillEnd; nCurrent++) + { + size_t nIndex = size_t(nFillStart - nCurrent) % aSparklineSeries.size(); + if (auto& rpSparkline = aSparklineSeries[nIndex]) + { + auto pGroup = rpSparkline->getSparklineGroup(); + + auto* pNewSparkline = bVertical ? CreateSparkline(nFixed, nCurrent, pGroup) + : CreateSparkline(nCurrent, nFixed, pGroup); + if (pNewSparkline) + { + SCCOLROW nPosition = bVertical ? rpSparkline->getRow() + : rpSparkline->getColumn(); + SCCOLROW nDelta = nCurrent - nPosition; + ScRangeList aRangeList(rpSparkline->getInputRange()); + for (ScRange& rRange : aRangeList) + { + if (bVertical) + { + rRange.aStart.IncRow(nDelta); + rRange.aEnd.IncRow(nDelta); + } + else + { + rRange.aStart.IncCol(nDelta); + rRange.aEnd.IncCol(nDelta); + } + } + pNewSparkline->setInputRange(aRangeList); + } + } + } + } +} + OUString ScTable::GetAutoFillPreview( const ScRange& rSource, SCCOL nEndX, SCROW nEndY ) { OUString aValue; commit 93d7fbf73f8be7f09dd36e86f0dd06dfc8daa65c Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk> AuthorDate: Sun Apr 3 15:53:55 2022 +0900 Commit: Tomaž Vajngerl <qui...@gmail.com> CommitDate: Thu Apr 7 10:35:59 2022 +0200 sc: improve keeping track of sparklines in SparklineList Issues can happen when saving a document with sparklines where a sparkline is deleted and then undo-ed. The reason for this is because the SparlineList wasn't correctly updated when deleting, copying or adding sparklines. This change adds hooks when new sparklines are created or when sparklines are deleted to report this into SparlineList. SparklineList garbage-collects itself but this is not enough when we rely that the non-deleted weak pointers to the sparkline groups contain the correct non-deleted weak pointers to the correct sparklines. Change-Id: I976cbe7e6168813d3dd5089c036cc7fe4e357fb2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132554 Tested-by: Tomaž Vajngerl <qui...@gmail.com> Reviewed-by: Tomaž Vajngerl <qui...@gmail.com> diff --git a/sc/inc/SparklineList.hxx b/sc/inc/SparklineList.hxx index bab467a09469..e9ca8591237c 100644 --- a/sc/inc/SparklineList.hxx +++ b/sc/inc/SparklineList.hxx @@ -36,6 +36,7 @@ public: SparklineList(); void addSparkline(std::shared_ptr<Sparkline> const& pSparkline); + void removeSparkline(std::shared_ptr<Sparkline> const& pSparkline); std::vector<std::shared_ptr<SparklineGroup>> getSparklineGroups(); diff --git a/sc/qa/unit/SparklineTest.cxx b/sc/qa/unit/SparklineTest.cxx index 99efd55f8209..f8d07edc53e6 100644 --- a/sc/qa/unit/SparklineTest.cxx +++ b/sc/qa/unit/SparklineTest.cxx @@ -458,47 +458,109 @@ void SparklineTest::testUndoRedoClearContentForSparkline() insertTestData(rDocument); // Sparkline range - ScRange aRange(0, 6, 0, 0, 6, 0); + ScRange aDataRange(0, 0, 0, 3, 5, 0); //A1:D6 + ScRange aRange(0, 6, 0, 3, 6, 0); // A7:D7 - // Check Sparkline at cell A7 doesn't exists - auto pSparkline = rDocument.GetSparkline(aRange.aStart); - CPPUNIT_ASSERT(!pSparkline); + // Check Sparklines - none should exist + { + CPPUNIT_ASSERT(!rDocument.HasSparkline(ScAddress(0, 6, 0))); + CPPUNIT_ASSERT(!rDocument.HasSparkline(ScAddress(1, 6, 0))); + CPPUNIT_ASSERT(!rDocument.HasSparkline(ScAddress(2, 6, 0))); + CPPUNIT_ASSERT(!rDocument.HasSparkline(ScAddress(3, 6, 0))); + } auto pSparklineGroup = std::make_shared<sc::SparklineGroup>(); - CPPUNIT_ASSERT(rDocFunc.InsertSparklines(ScRange(0, 0, 0, 0, 5, 0), aRange, pSparklineGroup)); + CPPUNIT_ASSERT(rDocFunc.InsertSparklines(aDataRange, aRange, pSparklineGroup)); - // Check Sparkline at cell A7 exists - pSparkline = rDocument.GetSparkline(aRange.aStart); - CPPUNIT_ASSERT(pSparkline); - CPPUNIT_ASSERT_EQUAL(SCCOL(0), pSparkline->getColumn()); - CPPUNIT_ASSERT_EQUAL(SCROW(6), pSparkline->getRow()); + // Check Sparklines + { + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(0, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(1, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(2, 6, 0))); + // D7 exists + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(3, 6, 0))); + + // Check D7 + auto pSparkline = rDocument.GetSparkline(ScAddress(3, 6, 0)); + CPPUNIT_ASSERT_EQUAL(SCCOL(3), pSparkline->getColumn()); + CPPUNIT_ASSERT_EQUAL(SCROW(6), pSparkline->getRow()); + + // Check collections + auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0)); + auto pSparklineGroups = pSparklineList->getSparklineGroups(); + CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklineGroups.size()); - // Clear content - including sparkline - ScMarkData aMark(rDocument.GetSheetLimits()); - aMark.SetMarkArea(aRange.aStart); - rDocFunc.DeleteContents(aMark, InsertDeleteFlags::CONTENTS, true, true); + auto pSparklines = pSparklineList->getSparklinesFor(pSparklineGroups[0]); + CPPUNIT_ASSERT_EQUAL(size_t(4), pSparklines.size()); + } - // Check Sparkline at cell A7 doesn't exists - pSparkline = rDocument.GetSparkline(aRange.aStart); - CPPUNIT_ASSERT(!pSparkline); + // Clear content of cell D7 - including sparkline + { + ScMarkData aMark(rDocument.GetSheetLimits()); + aMark.SetMarkArea(ScAddress(3, 6, 0)); + rDocFunc.DeleteContents(aMark, InsertDeleteFlags::CONTENTS, true, true); + } + + // Check Sparklines + { + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(1, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(1, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(2, 6, 0))); + // D7 is gone + CPPUNIT_ASSERT(!rDocument.HasSparkline(ScAddress(3, 6, 0))); + + // Check collections + auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0)); + auto pSparklineGroups = pSparklineList->getSparklineGroups(); + CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklineGroups.size()); + + auto pSparklines = pSparklineList->getSparklinesFor(pSparklineGroups[0]); + CPPUNIT_ASSERT_EQUAL(size_t(3), pSparklines.size()); + } // Undo rDocument.GetUndoManager()->Undo(); - // Check Sparkline at cell A7 exists - pSparkline = rDocument.GetSparkline(aRange.aStart); - CPPUNIT_ASSERT(pSparkline); - CPPUNIT_ASSERT_EQUAL(SCCOL(0), pSparkline->getColumn()); - CPPUNIT_ASSERT_EQUAL(SCROW(6), pSparkline->getRow()); + // Check Sparkline + { + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(0, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(1, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(2, 6, 0))); + // D7 exists - again + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(3, 6, 0))); + + // Check D7 + auto pSparkline = rDocument.GetSparkline(ScAddress(3, 6, 0)); + CPPUNIT_ASSERT_EQUAL(SCCOL(3), pSparkline->getColumn()); + CPPUNIT_ASSERT_EQUAL(SCROW(6), pSparkline->getRow()); + + auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0)); + auto pSparklineGroups = pSparklineList->getSparklineGroups(); + CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklineGroups.size()); + + auto pSparklines = pSparklineList->getSparklinesFor(pSparklineGroups[0]); + CPPUNIT_ASSERT_EQUAL(size_t(4), pSparklines.size()); + } // Redo rDocument.GetUndoManager()->Redo(); - // Check Sparkline at cell A7 doesn't exists - pSparkline = rDocument.GetSparkline(aRange.aStart); - CPPUNIT_ASSERT(!pSparkline); + // Check Sparklines + { + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(1, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(1, 6, 0))); + CPPUNIT_ASSERT(rDocument.HasSparkline(ScAddress(2, 6, 0))); + // D7 is gone - again + CPPUNIT_ASSERT(!rDocument.HasSparkline(ScAddress(3, 6, 0))); - CPPUNIT_ASSERT(!rDocument.HasSparkline(aRange.aStart)); + // Check collections + auto* pSparklineList = rDocument.GetSparklineList(SCTAB(0)); + auto pSparklineGroups = pSparklineList->getSparklineGroups(); + CPPUNIT_ASSERT_EQUAL(size_t(1), pSparklineGroups.size()); + + auto pSparklines = pSparklineList->getSparklinesFor(pSparklineGroups[0]); + CPPUNIT_ASSERT_EQUAL(size_t(3), pSparklines.size()); + } xDocSh->DoClose(); } diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx index 7f1d3a722269..249d4c1346ac 100644 --- a/sc/source/core/data/column2.cxx +++ b/sc/source/core/data/column2.cxx @@ -43,6 +43,7 @@ #include <tokenstringcontext.hxx> #include <sortparam.hxx> #include <SparklineGroup.hxx> +#include <SparklineList.hxx> #include <editeng/eeitem.hxx> #include <o3tl/safeint.hxx> @@ -1996,6 +1997,29 @@ void ScColumn::DeleteEmptyBroadcasters() // Sparklines +namespace +{ + +class DeletingSparklinesHandler +{ + ScDocument& m_rDocument; + SCTAB m_nTab; + +public: + DeletingSparklinesHandler(ScDocument& rDocument, SCTAB nTab) + : m_rDocument(rDocument) + , m_nTab(nTab) + {} + + void operator() (size_t /*nRow*/, const sc::SparklineCell* pCell) + { + auto* pList = m_rDocument.GetSparklineList(m_nTab); + pList->removeSparkline(pCell->getSparkline()); + } +}; + +} // end anonymous ns + sc::SparklineCell* ScColumn::GetSparklineCell(SCROW nRow) { return maSparklines.get<sc::SparklineCell*>(nRow); @@ -2003,11 +2027,16 @@ sc::SparklineCell* ScColumn::GetSparklineCell(SCROW nRow) void ScColumn::CreateSparklineCell(SCROW nRow, std::shared_ptr<sc::Sparkline> const& pSparkline) { + auto* pList = GetDoc().GetSparklineList(GetTab()); + pList->addSparkline(pSparkline); maSparklines.set(nRow, new sc::SparklineCell(pSparkline)); } void ScColumn::DeleteSparklineCells(sc::ColumnBlockPosition& rBlockPos, SCROW nRow1, SCROW nRow2) { + DeletingSparklinesHandler aFunction(GetDoc(), nTab); + sc::ParseSparkline(maSparklines.begin(), maSparklines, nRow1, nRow2, aFunction); + rBlockPos.miSparklinePos = maSparklines.set_empty(rBlockPos.miSparklinePos, nRow1, nRow2); } @@ -2016,6 +2045,9 @@ bool ScColumn::DeleteSparkline(SCROW nRow) if (!GetDoc().ValidRow(nRow)) return false; + DeletingSparklinesHandler aFunction(GetDoc(), nTab); + sc::ParseSparkline(maSparklines.begin(), maSparklines, nRow, nRow, aFunction); + maSparklines.set_empty(nRow, nRow); return true; } @@ -2060,12 +2092,16 @@ public: auto const& pSparkline = pCell->getSparkline(); auto const& pGroup = pCell->getSparklineGroup(); - auto pDestinationGroup = mrDestColumn.GetDoc().SearchSparklineGroup(pGroup->getID()); + auto& rDestDoc = mrDestColumn.GetDoc(); + auto pDestinationGroup = rDestDoc.SearchSparklineGroup(pGroup->getID()); if (!pDestinationGroup) pDestinationGroup = std::make_shared<sc::SparklineGroup>(*pGroup); // Copy the group auto pNewSparkline = std::make_shared<sc::Sparkline>(mrDestColumn.GetCol(), nDestRow, pDestinationGroup); pNewSparkline->setInputRange(pSparkline->getInputRange()); + auto* pList = rDestDoc.GetSparklineList(mrDestColumn.GetTab()); + pList->addSparkline(pNewSparkline); + miDestPosition = mrDestSparkline.set(miDestPosition, nDestRow, new sc::SparklineCell(pNewSparkline)); } }; diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx index d0d9598fa952..421a3c96dc02 100644 --- a/sc/source/core/data/table2.cxx +++ b/sc/source/core/data/table2.cxx @@ -1848,8 +1848,8 @@ sc::Sparkline* ScTable::CreateSparkline(SCCOL nCol, SCROW nRow, std::shared_ptr< ScColumn& rColumn = CreateColumnIfNotExists(nCol); std::shared_ptr<sc::Sparkline> pSparkline(new sc::Sparkline(nCol, nRow, pSparklineGroup)); - maSparklineList.addSparkline(pSparkline); rColumn.CreateSparklineCell(nRow, pSparkline); + return pSparkline.get(); } diff --git a/sc/source/ui/sparklines/SparklineList.cxx b/sc/source/ui/sparklines/SparklineList.cxx index 7ee52ac74e27..1cae3089616d 100644 --- a/sc/source/ui/sparklines/SparklineList.cxx +++ b/sc/source/ui/sparklines/SparklineList.cxx @@ -25,6 +25,30 @@ void SparklineList::addSparkline(std::shared_ptr<Sparkline> const& pSparkline) m_aSparklineGroups.push_back(pWeakGroup); } +void SparklineList::removeSparkline(std::shared_ptr<Sparkline> const& pSparkline) +{ + auto pWeakGroup = std::weak_ptr<SparklineGroup>(pSparkline->getSparklineGroup()); + auto iteratorGroup = m_aSparklineGroupMap.find(pWeakGroup); + if (iteratorGroup != m_aSparklineGroupMap.end()) + { + auto& rWeakSparklines = iteratorGroup->second; + + for (auto iterator = rWeakSparklines.begin(); iterator != rWeakSparklines.end();) + { + auto pCurrentSparkline = iterator->lock(); + + if (pCurrentSparkline && pCurrentSparkline != pSparkline) + { + iterator++; + } + else + { + iterator = rWeakSparklines.erase(iterator); + } + } + } +} + std::vector<std::shared_ptr<SparklineGroup>> SparklineList::getSparklineGroups() { std::vector<std::shared_ptr<SparklineGroup>> toReturn;