sc/inc/document.hxx | 6 +++++
sc/inc/table.hxx | 17 ++++++++------
sc/source/core/data/documen4.cxx | 2 -
sc/source/core/data/document.cxx | 24 +++++++++++++++-----
sc/source/core/data/table1.cxx | 45 ++++++++++++++++++++-------------------
sc/source/core/data/table2.cxx | 2 -
sc/source/core/data/table3.cxx | 4 +--
sc/source/core/data/table7.cxx | 2 -
sc/source/filter/dif/difimp.cxx | 2 -
sc/source/ui/unoobj/docuno.cxx | 2 -
10 files changed, 65 insertions(+), 41 deletions(-)
New commits:
commit 4dfdb5a3c9b9ce1eb61c28d8f570398c04fb0d01
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Thu Apr 21 19:11:38 2022 +0200
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Thu Apr 21 21:07:25 2022 +0200
rework GetColumnsRange() and ScColumnsRange
The problem with GetColumnsRange() was that it was clamping the range
to the number of allocated columns, but that's not always wanted,
e.g. ScDocument::InsertMatrixFormula() needs to iterate over the whole
range and allocate columns if necessary instead of ignoring them.
From an API point of view it's also not very obvious that something
called GetColumnsRange() actually does something more than just
returning the given range.
Handle this by making GetColumnsRange() return the actual given range,
and add GetWriteableColumnsRange() and GetAllocatedColumnsRange()
for the specific cases. This also required changing ScColumnsRange
to work simply on SCCOL value instead of using std::vector iterator
(since the vector may not have all the elements in the range).
Change-Id: I9b645459461efe6b282e8ac5d7a29549830f46c1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/133295
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index 26e43c046164..d24f4f170239 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -2607,6 +2607,12 @@ public:
void finalizeOutlineImport();
bool TableExists( SCTAB nTab ) const;
+ // Returns the given column range, first allocating all the columns if
necessary.
+ SC_DLLPUBLIC ScColumnsRange GetWritableColumnsRange(SCTAB nTab, SCCOL
nColBegin, SCCOL nColEnd);
+ // Returns a column range, clamped to the allocated columns.
+ SC_DLLPUBLIC ScColumnsRange GetAllocatedColumnsRange(SCTAB nTab, SCCOL
nColBegin, SCCOL nColEnd) const;
+ // Returns the given range, without any adjustments. One of the variants
above may return
+ // a smaller range (better performance) if the use case is known.
SC_DLLPUBLIC ScColumnsRange GetColumnsRange(SCTAB nTab, SCCOL nColBegin,
SCCOL nColEnd) const;
bool IsInDocShellRecalc() const { return mbDocShellRecalc; }
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 36ff984ed8fa..ba8ed9e328d8 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -123,7 +123,7 @@ class ScColumnsRange final
public:
class Iterator final
{
- std::vector<std::unique_ptr<ScColumn,
o3tl::default_delete<ScColumn>>>::const_iterator maColIter;
+ SCCOL mCol;
public:
typedef std::bidirectional_iterator_tag iterator_category;
typedef SCCOL value_type;
@@ -131,17 +131,18 @@ class ScColumnsRange final
typedef const SCCOL* pointer;
typedef SCCOL reference;
- explicit Iterator(const std::vector<std::unique_ptr<ScColumn,
o3tl::default_delete<ScColumn>>>::const_iterator& colIter) : maColIter(colIter)
{}
+ explicit Iterator(SCCOL nCol) : mCol(nCol) {}
- Iterator& operator++() { ++maColIter; return *this;}
- Iterator& operator--() { --maColIter; return *this;}
+ Iterator& operator++() { ++mCol; return *this;}
+ Iterator& operator--() { --mCol; return *this;}
- bool operator==(const Iterator & rOther) const {return maColIter ==
rOther.maColIter;}
+ // Comparing iterators from different containers is undefined, so
comparing mCol is enough.
+ bool operator==(const Iterator & rOther) const {return mCol ==
rOther.mCol;}
bool operator!=(const Iterator & rOther) const {return !(*this ==
rOther);}
- SCCOL operator*() const {return (*maColIter)->GetCol();}
+ SCCOL operator*() const {return mCol;}
};
- ScColumnsRange(const Iterator & rBegin, const Iterator & rEnd) :
maBegin(rBegin), maEnd(rEnd) {}
+ ScColumnsRange(SCCOL nBegin, SCCOL nEnd) : maBegin(nBegin), maEnd(nEnd) {}
const Iterator & begin() { return maBegin; }
const Iterator & end() { return maEnd; }
std::reverse_iterator<Iterator> rbegin() { return
std::reverse_iterator<Iterator>(maEnd); }
@@ -1114,6 +1115,8 @@ public:
*/
static void UpdateSearchItemAddressForReplace( const SvxSearchItem&
rSearchItem, SCCOL& rCol, SCROW& rRow );
+ ScColumnsRange GetWritableColumnsRange(SCCOL begin, SCCOL end);
+ ScColumnsRange GetAllocatedColumnsRange(SCCOL begin, SCCOL end) const;
ScColumnsRange GetColumnsRange(SCCOL begin, SCCOL end) const;
SCCOL ClampToAllocatedColumns(SCCOL nCol) const { return std::min(nCol,
static_cast<SCCOL>(aCol.size() - 1)); }
SCCOL GetAllocatedColumnsCount() const { return aCol.size(); }
diff --git a/sc/source/core/data/documen4.cxx b/sc/source/core/data/documen4.cxx
index 271e07619ef8..dff8e71c7373 100644
--- a/sc/source/core/data/documen4.cxx
+++ b/sc/source/core/data/documen4.cxx
@@ -339,7 +339,7 @@ void ScDocument::InsertMatrixFormula(SCCOL nCol1, SCROW
nRow1,
*t->GetSingleRef() = aRefData;
}
- for (SCCOL nCol : GetColumnsRange(nTab1, nCol1, nCol2))
+ for (SCCOL nCol : GetWritableColumnsRange(nTab1, nCol1, nCol2))
{
for (SCROW nRow = nRow1; nRow <= nRow2; ++nRow)
{
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 60dc6b749b82..b0b9242a1a63 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -2550,15 +2550,27 @@ const ScTable* ScDocument::FetchTable( SCTAB nTab )
const
return maTabs[nTab].get();
}
-ScColumnsRange ScDocument::GetColumnsRange( SCTAB nTab, SCCOL nColBegin, SCCOL
nColEnd) const
+ScColumnsRange ScDocument::GetWritableColumnsRange( SCTAB nTab, SCCOL
nColBegin, SCCOL nColEnd)
{
if (!TableExists(nTab))
{
- std::vector<std::unique_ptr<ScColumn, o3tl::default_delete<ScColumn>>>
aEmptyVector;
- return ScColumnsRange(ScColumnsRange::Iterator(aEmptyVector.begin()),
- ScColumnsRange::Iterator(aEmptyVector.end()));
+ SAL_WARN("sc", "GetWritableColumnsRange() called for non-existent
table");
+ return ScColumnsRange(-1, -1);
}
+ return maTabs[nTab]->GetWritableColumnsRange(nColBegin, nColEnd);
+}
+ScColumnsRange ScDocument::GetAllocatedColumnsRange( SCTAB nTab, SCCOL
nColBegin, SCCOL nColEnd) const
+{
+ if (!TableExists(nTab))
+ return ScColumnsRange(-1, -1);
+ return maTabs[nTab]->GetAllocatedColumnsRange(nColBegin, nColEnd);
+}
+
+ScColumnsRange ScDocument::GetColumnsRange( SCTAB nTab, SCCOL nColBegin, SCCOL
nColEnd) const
+{
+ if (!TableExists(nTab))
+ return ScColumnsRange(-1, -1);
return maTabs[nTab]->GetColumnsRange(nColBegin, nColEnd);
}
@@ -6869,7 +6881,7 @@ ScAddress ScDocument::GetNotePosition( size_t nIndex )
const
{
for (size_t nTab = 0; nTab < maTabs.size(); ++nTab)
{
- for (SCCOL nCol : GetColumnsRange(nTab, 0, MaxCol()))
+ for (SCCOL nCol : GetAllocatedColumnsRange(nTab, 0, MaxCol()))
{
size_t nColNoteCount = GetNoteCount(nTab, nCol);
if (!nColNoteCount)
@@ -6896,7 +6908,7 @@ ScAddress ScDocument::GetNotePosition( size_t nIndex )
const
ScAddress ScDocument::GetNotePosition( size_t nIndex, SCTAB nTab ) const
{
- for (SCCOL nCol : GetColumnsRange(nTab, 0, MaxCol()))
+ for (SCCOL nCol : GetAllocatedColumnsRange(nTab, 0, MaxCol()))
{
size_t nColNoteCount = GetNoteCount(nTab, nCol);
if (!nColNoteCount)
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index 45bd5c777cc0..1d8a072a0635 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -2672,31 +2672,34 @@ const ScConditionalFormatList*
ScTable::GetCondFormList() const
return mpCondFormatList.get();
}
-ScColumnsRange ScTable::GetColumnsRange(SCCOL nColBegin, SCCOL nColEnd) const
+ScColumnsRange ScTable::GetWritableColumnsRange(SCCOL nColBegin, SCCOL nColEnd)
+{
+ // because the range is inclusive, some code will pass nColEnd<nColBegin
to indicate an empty range
+ if (nColEnd < nColBegin)
+ return ScColumnsRange(-1, -1);
+ assert( nColEnd >= 0 && nColEnd <= GetDoc().MaxCol());
+ CreateColumnIfNotExists(nColEnd);
+ return GetColumnsRange(nColBegin, nColEnd);
+}
+
+ScColumnsRange ScTable::GetAllocatedColumnsRange(SCCOL nColBegin, SCCOL
nColEnd) const
{
- ScColContainer::ScColumnVector::const_iterator beginIter;
- ScColContainer::ScColumnVector::const_iterator endIter;
+ if (nColBegin >= aCol.size())
+ return ScColumnsRange(-1, -1);
+ // clamp end of range to available columns
+ if (nColEnd >= aCol.size())
+ nColEnd = aCol.size() - 1;
+ return GetColumnsRange(nColBegin, nColEnd);
+}
+ScColumnsRange ScTable::GetColumnsRange(SCCOL nColBegin, SCCOL nColEnd) const
+{
// because the range is inclusive, some code will pass nColEnd<nColBegin
to indicate an empty range
if (nColEnd < nColBegin)
- {
- beginIter = aCol.end();
- endIter = aCol.end();
- }
- else if (nColBegin >= aCol.size())
- {
- beginIter = aCol.end();
- endIter = aCol.end();
- }
- else
- {
- // clamp end of range to available columns
- if (nColEnd >= aCol.size())
- nColEnd = aCol.size() - 1;
- beginIter = aCol.begin() + nColBegin;
- endIter = aCol.begin() + nColEnd + 1;
- }
- return ScColumnsRange(ScColumnsRange::Iterator(beginIter),
ScColumnsRange::Iterator(endIter));
+ return ScColumnsRange(-1, -1);
+ assert( nColBegin >= 0 && nColBegin <= GetDoc().MaxCol());
+ assert( nColEnd >= 0 && nColEnd <= GetDoc().MaxCol());
+ return ScColumnsRange(nColBegin, nColEnd + 1); // change inclusive end to
past-end
}
// out-of-line the cold part of the CreateColumnIfNotExists function
diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx
index 421a3c96dc02..e9b5bb371a85 100644
--- a/sc/source/core/data/table2.cxx
+++ b/sc/source/core/data/table2.cxx
@@ -260,7 +260,7 @@ bool ScTable::TestInsertCol( SCROW nStartRow, SCROW
nEndRow, SCSIZE nSize ) cons
&& ! pOutlineTable->TestInsertCol(nSize) )
return false;
- auto range = GetColumnsRange( rDocument.MaxCol() -
static_cast<SCCOL>(nSize) + 1, rDocument.MaxCol() );
+ auto range = GetAllocatedColumnsRange( rDocument.MaxCol() -
static_cast<SCCOL>(nSize) + 1, rDocument.MaxCol() );
for (auto it = range.rbegin(); it != range.rend(); ++it )
if (! aCol[*it].TestInsertCol(nStartRow, nEndRow))
return false;
diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx
index 511d4a3c80ed..a04f0b4c8541 100644
--- a/sc/source/core/data/table3.cxx
+++ b/sc/source/core/data/table3.cxx
@@ -1926,12 +1926,12 @@ public:
SCCOL nStartCol = mrParam.nCol1;
SCCOL nEndCol = mrParam.nCol2;
- for (SCCOL nCol : mrTab.GetColumnsRange(0, nStartCol - 1))
+ for (SCCOL nCol : mrTab.GetAllocatedColumnsRange(0, nStartCol - 1))
{
if (mrTab.HasData(nCol, nRow))
return true;
}
- for (SCCOL nCol : mrTab.GetColumnsRange(nEndCol + 1,
mrTab.GetDoc().MaxCol()))
+ for (SCCOL nCol : mrTab.GetAllocatedColumnsRange(nEndCol + 1,
mrTab.GetDoc().MaxCol()))
{
if (mrTab.HasData(nCol, nRow))
return true;
diff --git a/sc/source/core/data/table7.cxx b/sc/source/core/data/table7.cxx
index dc3a8c045dd9..30788b0daca7 100644
--- a/sc/source/core/data/table7.cxx
+++ b/sc/source/core/data/table7.cxx
@@ -321,7 +321,7 @@ void ScTable::EndListeningIntersectedGroups(
if (nCol2 < nCol1 || !IsColValid(nCol1) || !ValidCol(nCol2))
return;
- for (SCCOL nCol : GetColumnsRange(nCol1, nCol2))
+ for (SCCOL nCol : GetAllocatedColumnsRange(nCol1, nCol2))
aCol[nCol].EndListeningIntersectedGroups(rCxt, nRow1, nRow2,
pGroupPos);
}
diff --git a/sc/source/filter/dif/difimp.cxx b/sc/source/filter/dif/difimp.cxx
index ede4af6cf753..f6e1fd0b0a36 100644
--- a/sc/source/filter/dif/difimp.cxx
+++ b/sc/source/filter/dif/difimp.cxx
@@ -664,7 +664,7 @@ void DifAttrCache::SetNumFormat( const ScDocument* pDoc,
const SCCOL nCol, const
void DifAttrCache::Apply( ScDocument& rDoc, SCTAB nTab )
{
- for( SCCOL nCol : rDoc.GetColumnsRange(nTab, 0, rDoc.MaxCol()) )
+ for( SCCOL nCol : rDoc.GetWritableColumnsRange(nTab, 0, rDoc.MaxCol()) )
{
if( maColMap.count(nCol) )
maColMap[ nCol ]->Apply( rDoc, nCol, nTab );
diff --git a/sc/source/ui/unoobj/docuno.cxx b/sc/source/ui/unoobj/docuno.cxx
index f7bddf569605..e2b24d517b83 100644
--- a/sc/source/ui/unoobj/docuno.cxx
+++ b/sc/source/ui/unoobj/docuno.cxx
@@ -4627,7 +4627,7 @@ sal_Int32 SAL_CALL ScAnnotationsObj::getCount()
if (pDocShell)
{
const ScDocument& rDoc = pDocShell->GetDocument();
- for (SCCOL nCol : rDoc.GetColumnsRange(nTab, 0, rDoc.MaxCol()))
+ for (SCCOL nCol : rDoc.GetAllocatedColumnsRange(nTab, 0,
rDoc.MaxCol()))
nCount += rDoc.GetNoteCount(nTab, nCol);
}
return nCount;
