sc/inc/attarray.hxx | 1
sc/inc/column.hxx | 6 ---
sc/inc/dociter.hxx | 4 --
sc/inc/document.hxx | 5 --
sc/inc/table.hxx | 4 --
sc/qa/unit/ucalc.cxx | 66 ++++++++++++++++++++++++++++++++++++---
sc/source/core/data/attarray.cxx | 20 -----------
sc/source/core/data/dociter.cxx | 61 +++++-------------------------------
sc/source/core/data/table1.cxx | 15 --------
9 files changed, 71 insertions(+), 111 deletions(-)
New commits:
commit 07745a031da255991c2d4c1533e916bb604d66c2
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Apr 12 15:19:38 2022 +0200
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Tue Apr 12 23:10:29 2022 +0200
don't ignore GetDefPattern() in ScHorizontalAttrIterator
As said in the previous commit, the default pattern is the default
style that can be edited by the user, so it's principially incorrect
to simply ignore it. If needed for performance, then it needs
to be done explicitly. This change currently should not affect
anything, as ScHorizontalAttrIterator is used only in tests.
Change-Id: I31f153d427cdfd6e98a4d7a3584cfa89676d4c33
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132912
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/inc/dociter.hxx b/sc/inc/dociter.hxx
index 2aee6ac950c1..99ceb1e99972 100644
--- a/sc/inc/dociter.hxx
+++ b/sc/inc/dociter.hxx
@@ -499,8 +499,6 @@ public:
bool GetNext( double& rValue, FormulaError& rErr );
};
-// returns all areas with non-default formatting (horizontal)
-
class ScHorizontalAttrIterator
{
private:
@@ -518,11 +516,9 @@ private:
ppPatterns;
SCCOL nCol;
SCROW nRow;
- bool bRowEmpty;
SCROW nMinNextEnd;
void InitForNextRow(bool bInitialization);
- bool InitForNextAttr();
public:
ScHorizontalAttrIterator( ScDocument& rDocument, SCTAB nTable,
diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index 020e43c6dbde..8da0c6132aee 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -117,6 +117,7 @@ public:
void testValueIterator();
void testHorizontalAttrIterator();
void testIteratorsUnallocatedColumnsAttributes();
+ void testIteratorsDefPattern();
void testLastChangedColFlagsWidth();
/**
@@ -251,6 +252,7 @@ public:
CPPUNIT_TEST(testValueIterator);
CPPUNIT_TEST(testHorizontalAttrIterator);
CPPUNIT_TEST(testIteratorsUnallocatedColumnsAttributes);
+ CPPUNIT_TEST(testIteratorsDefPattern);
CPPUNIT_TEST(testLastChangedColFlagsWidth);
CPPUNIT_TEST(testCellBroadcaster);
CPPUNIT_TEST(testFuncParam);
@@ -1394,12 +1396,15 @@ void Test::testHorizontalAttrIterator()
SCCOL nCol1, nCol2;
SCROW nRow;
size_t nCheckPos = 0;
- for (const ScPatternAttr* pAttr = aIter.GetNext(nCol1, nCol2, nRow);
pAttr; pAttr = aIter.GetNext(nCol1, nCol2, nRow), ++nCheckPos)
+ for (const ScPatternAttr* pAttr = aIter.GetNext(nCol1, nCol2, nRow);
pAttr; pAttr = aIter.GetNext(nCol1, nCol2, nRow))
{
- CPPUNIT_ASSERT_MESSAGE("Iteration longer than expected.",
nCheckPos < nCheckLen);
- CPPUNIT_ASSERT_EQUAL(aChecks[nCheckPos][0],
static_cast<int>(nCol1));
- CPPUNIT_ASSERT_EQUAL(aChecks[nCheckPos][1],
static_cast<int>(nCol2));
- CPPUNIT_ASSERT_EQUAL(aChecks[nCheckPos][2],
static_cast<int>(nRow));
+ if( pAttr == m_pDoc->GetDefPattern())
+ continue;
+ CPPUNIT_ASSERT_MESSAGE("Iteration longer than expected.",
nCheckPos < nCheckLen);
+ CPPUNIT_ASSERT_EQUAL(aChecks[nCheckPos][0],
static_cast<int>(nCol1));
+ CPPUNIT_ASSERT_EQUAL(aChecks[nCheckPos][1],
static_cast<int>(nCol2));
+ CPPUNIT_ASSERT_EQUAL(aChecks[nCheckPos][2],
static_cast<int>(nRow));
+ ++nCheckPos;
}
}
@@ -1458,6 +1463,57 @@ void Test::testIteratorsUnallocatedColumnsAttributes()
m_pDoc->DeleteTab(0);
}
+void Test::testIteratorsDefPattern()
+{
+ m_pDoc->InsertTab(0, "Tab1");
+
+ // The default pattern is the default style, which can be edited by the
user.
+ // As such iterators should not ignore it by default, because it might
contain
+ // some attributes set.
+
+ // Set cells as bold, default allocated, bold, default unallocated.
+ SCCOL firstCol = 100;
+ SCCOL lastCol = 103;
+ ScPatternAttr boldAttr(m_pDoc->GetPool());
+ boldAttr.GetItemSet().Put(SvxWeightItem(WEIGHT_BOLD, ATTR_FONT_WEIGHT));
+ m_pDoc->ApplyPattern(100, 0, 0, boldAttr);
+ m_pDoc->ApplyPattern(102, 0, 0, boldAttr);
+
+ CPPUNIT_ASSERT_EQUAL(SCCOL(102 + 1), m_pDoc->GetAllocatedColumnsCount(0));
+ const ScPatternAttr* pattern = m_pDoc->GetPattern(100, 0, 0);
+ const ScPatternAttr* defPattern = m_pDoc->GetDefPattern();
+ CPPUNIT_ASSERT(pattern != defPattern);
+ CPPUNIT_ASSERT_EQUAL(pattern, m_pDoc->GetPattern(102, 0, 0));
+ CPPUNIT_ASSERT_EQUAL(defPattern, m_pDoc->GetPattern(101, 0, 0));
+ CPPUNIT_ASSERT_EQUAL(defPattern, m_pDoc->GetPattern(103, 0, 0));
+
+ // Test iterators.
+ ScDocAttrIterator docit( *m_pDoc, 0, firstCol, 0, lastCol, 0 );
+ SCCOL col1, col2;
+ SCROW row1, row2;
+ CPPUNIT_ASSERT_EQUAL(pattern, docit.GetNext( col1, row1, row2 ));
+ CPPUNIT_ASSERT_EQUAL(defPattern, docit.GetNext( col1, row1, row2 ));
+ CPPUNIT_ASSERT_EQUAL(pattern, docit.GetNext( col1, row1, row2 ));
+ CPPUNIT_ASSERT_EQUAL(defPattern, docit.GetNext( col1, row1, row2 ));
+ CPPUNIT_ASSERT(docit.GetNext( col1, row1, row2 ) == nullptr );
+
+ ScAttrRectIterator rectit( *m_pDoc, 0, firstCol, 0, lastCol, 0 );
+ CPPUNIT_ASSERT_EQUAL(pattern, rectit.GetNext( col1, col2, row1, row2 ));
+ CPPUNIT_ASSERT_EQUAL(defPattern, rectit.GetNext( col1, col2, row1, row2 ));
+ CPPUNIT_ASSERT_EQUAL(pattern, rectit.GetNext( col1, col2, row1, row2 ));
+ CPPUNIT_ASSERT_EQUAL(defPattern, rectit.GetNext( col1, col2, row1, row2 ));
+ CPPUNIT_ASSERT(rectit.GetNext( col1, col2, row1, row2 ) == nullptr );
+
+ ScHorizontalAttrIterator horit( *m_pDoc, 0, firstCol, 0, lastCol, 0 );
+ CPPUNIT_ASSERT_EQUAL(pattern, horit.GetNext( col1, col2, row1 ));
+ CPPUNIT_ASSERT_EQUAL(defPattern, horit.GetNext( col1, col2, row1 ));
+ CPPUNIT_ASSERT_EQUAL(pattern, horit.GetNext( col1, col2, row1 ));
+ CPPUNIT_ASSERT_EQUAL(defPattern, horit.GetNext( col1, col2, row1 ));
+ CPPUNIT_ASSERT(horit.GetNext( col1, col2, row1 ) == nullptr );
+
+ m_pDoc->DeleteTab(0);
+}
+
void Test::testLastChangedColFlagsWidth()
{
m_pDoc->InsertTab(0, "Tab1");
diff --git a/sc/source/core/data/dociter.cxx b/sc/source/core/data/dociter.cxx
index 6bb47df2a9d8..01583d570d63 100644
--- a/sc/source/core/data/dociter.cxx
+++ b/sc/source/core/data/dociter.cxx
@@ -2396,7 +2396,6 @@ ScHorizontalAttrIterator::ScHorizontalAttrIterator(
ScDocument& rDocument, SCTAB
nRow = nStartRow;
nCol = nStartCol;
- bRowEmpty = false;
pIndices.reset( new SCSIZE[nEndCol-nStartCol+1] );
pNextEnd.reset( new SCROW[nEndCol-nStartCol+1] );
@@ -2412,7 +2411,6 @@ ScHorizontalAttrIterator::~ScHorizontalAttrIterator()
void ScHorizontalAttrIterator::InitForNextRow(bool bInitialization)
{
- bool bEmpty = true;
nMinNextEnd = rDoc.MaxRow();
SCCOL nThisHead = 0;
@@ -2440,18 +2438,12 @@ void ScHorizontalAttrIterator::InitForNextRow(bool
bInitialization)
{
pNextEnd[nPos] = rDoc.MaxRow();
assert( pNextEnd[nPos] >= nRow && "Sequence out of order" );
- ppPatterns[nPos] = nullptr;
+ ppPatterns[nPos] = rDoc.GetDefPattern();
}
else if ( nIndex < pArray.Count() )
{
const ScPatternAttr* pPattern = pArray.mvData[nIndex].pPattern;
SCROW nThisEnd = pArray.mvData[nIndex].nEndRow;
-
- if ( IsDefaultItem( pPattern ) )
- pPattern = nullptr;
- else
- bEmpty = false; // Found attributes
-
pNextEnd[nPos] = nThisEnd;
assert( pNextEnd[nPos] >= nRow && "Sequence out of order" );
ppPatterns[nPos] = pPattern;
@@ -2463,8 +2455,6 @@ void ScHorizontalAttrIterator::InitForNextRow(bool
bInitialization)
ppPatterns[nPos] = nullptr;
}
}
- else if ( ppPatterns[nPos] )
- bEmpty = false; // Area not at the end yet
if ( nMinNextEnd > pNextEnd[nPos] )
nMinNextEnd = pNextEnd[nPos];
@@ -2477,24 +2467,7 @@ void ScHorizontalAttrIterator::InitForNextRow(bool
bInitialization)
}
}
- if (bEmpty)
- nRow = nMinNextEnd; // Skip until end of next section
- else
- pHorizEnd[nThisHead] = nEndCol; // set the end position of the last
horizontal group, too
- bRowEmpty = bEmpty;
-}
-
-bool ScHorizontalAttrIterator::InitForNextAttr()
-{
- if ( !ppPatterns[nCol-nStartCol] ) // Skip default items
- {
- assert( pHorizEnd[nCol-nStartCol] < rDoc.MaxCol()+1 && "missing stored
data" );
- nCol = pHorizEnd[nCol-nStartCol] + 1;
- if ( nCol > nEndCol )
- return false;
- }
-
- return true;
+ pHorizEnd[nThisHead] = nEndCol; // set the end position of the last
horizontal group, too
}
const ScPatternAttr* ScHorizontalAttrIterator::GetNext( SCCOL& rCol1, SCCOL&
rCol2, SCROW& rRow )
@@ -2502,7 +2475,7 @@ const ScPatternAttr* ScHorizontalAttrIterator::GetNext(
SCCOL& rCol1, SCCOL& rCo
assert(nTab < rDoc.GetTableCount() && "index out of bounds, FIX IT");
for (;;)
{
- if ( !bRowEmpty && nCol <= nEndCol && InitForNextAttr() )
+ if ( nCol <= nEndCol )
{
const ScPatternAttr* pPat = ppPatterns[nCol-nStartCol];
rRow = nRow;
@@ -2520,7 +2493,7 @@ const ScPatternAttr* ScHorizontalAttrIterator::GetNext(
SCCOL& rCol1, SCCOL& rCo
return nullptr; // Found nothing
nCol = nStartCol; // Start at the left again
- if ( bRowEmpty || nRow > nMinNextEnd )
+ if ( nRow > nMinNextEnd )
InitForNextRow(false);
}
}
commit 4d45d9886fa5830f4c8c92268ffca4297650ed70
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Apr 12 14:58:33 2022 +0200
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Tue Apr 12 23:10:16 2022 +0200
don't artificially clamp attribute iterators range
Even ScDocument::GetDefPattern() can be modified by the user (it's
the default style that can be edited), so it's conceptually
incorrect to ignore it while iterating, and clamping to allocated
columns is also no longer needed. If this makes some code slow,
then that needs explicit handling in that code or some other
way of speeding things up.
Change-Id: I4a7c7fef0a8625b559bbce4580df19a5e9ed92a7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132911
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/inc/attarray.hxx b/sc/inc/attarray.hxx
index 8153b441ffe6..f4028b844c62 100644
--- a/sc/inc/attarray.hxx
+++ b/sc/inc/attarray.hxx
@@ -225,7 +225,6 @@ public:
bool Reserve( SCSIZE nReserve );
SCSIZE Count() const { return mvData.size(); }
SCSIZE Count( SCROW nRow1, SCROW nRow2 ) const;
- bool HasNonDefPattern( SCROW nStartRow, SCROW nEndRow ) const;
private:
const ScPatternAttr* SetPatternAreaImpl( SCROW nStartRow, SCROW nEndRow,
const ScPatternAttr* pPattern,
diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index 0a2b16c809d9..676e3fc0e634 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -166,7 +166,6 @@ public:
std::unique_ptr<ScAttrIterator> CreateAttrIterator( SCROW nStartRow, SCROW
nEndRow ) const;
bool IsAllAttrEqual( const ScColumnData& rCol, SCROW nStartRow,
SCROW nEndRow ) const;
- bool HasNonDefPattern( SCROW nStartRow, SCROW nEndRow ) const;
void ClearSelectionItems( const sal_uInt16* pWhich, const
ScMarkData& rMark, SCCOL nCol );
void ChangeSelectionIndent( bool bIncrement, const ScMarkData&
rMark, SCCOL nCol );
@@ -864,11 +863,6 @@ inline bool ScColumnData::IsAllAttrEqual( const
ScColumnData& rCol, SCROW nStart
return pAttrArray->IsAllEqual( *rCol.pAttrArray, nStartRow, nEndRow );
}
-inline bool ScColumnData::HasNonDefPattern( SCROW nStartRow, SCROW nEndRow )
const
-{
- return pAttrArray->HasNonDefPattern( nStartRow, nEndRow );
-}
-
inline bool ScColumn::IsVisibleAttrEqual( const ScColumn& rCol, SCROW
nStartRow, SCROW nEndRow ) const
{
return pAttrArray->IsVisibleEqual( *rCol.pAttrArray, nStartRow, nEndRow );
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index 2093847f26eb..dff68fc6e344 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -823,11 +823,6 @@ public:
SC_DLLPUBLIC SCCOL ClampToAllocatedColumns(SCTAB nTab, SCCOL nCol) const;
SC_DLLPUBLIC SCCOL GetAllocatedColumnsCount(SCTAB nTab) const;
- // Limits nCol to the last column that can possibly have a non-default
pattern. The purpose
- // is to avoid checking unallocated columns if they don't have attributes
set, so this is
- // never less than ClampToAllocatedColumns().
- SCCOL ClampToMaxNonDefPatternColumn(SCTAB nTab, SCCOL nCol, SCROW nRow1,
SCROW nRow2) const;
- SCCOL GetMaxNonDefPatternColumnsCount(SCTAB nTab, SCROW nRow1, SCROW
nRow2) const;
SC_DLLPUBLIC ScDBCollection* GetDBCollection() const { return
pDBCollection.get();}
void SetDBCollection(
std::unique_ptr<ScDBCollection> pNewDBCollection,
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index de8f9c231077..36ff984ed8fa 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -1117,10 +1117,6 @@ public:
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(); }
- SCCOL ClampToMaxNonDefPatternColumn(SCCOL nCol, SCROW nRow1, SCROW nRow2)
const;
- SCCOL ClampToMaxNonDefPatternColumn(SCCOL nCol) const
- { return ClampToMaxNonDefPatternColumn(nCol, 0, rDocument.MaxRow()); }
- SCCOL GetMaxNonDefPatternColumnsCount(SCROW nRow1, SCROW nRow2) const;
/**
* Serializes the sheet's geometry data.
diff --git a/sc/source/core/data/attarray.cxx b/sc/source/core/data/attarray.cxx
index 042213bda05c..2f7e686ac383 100644
--- a/sc/source/core/data/attarray.cxx
+++ b/sc/source/core/data/attarray.cxx
@@ -2690,24 +2690,4 @@ SCSIZE ScAttrArray::Count( SCROW nStartRow, SCROW
nEndRow ) const
return nIndex2 - nIndex1 + 1;
}
-bool ScAttrArray::HasNonDefPattern( SCROW nStartRow, SCROW nEndRow ) const
-{
- if ( mvData.empty() )
- return false;
-
- SCSIZE nIndex1, nIndex2;
-
- if( !Search( nStartRow, nIndex1 ) )
- return false;
-
- if( !Search( nEndRow, nIndex2 ) )
- nIndex2 = mvData.size() - 1;
-
- const ScPatternAttr* defPattern = rDocument.GetDefPattern();
- for( SCSIZE index = nIndex1; index <= nIndex2; ++index )
- if( mvData[index].pPattern != defPattern )
- return true;
- return false;
-}
-
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/dociter.cxx b/sc/source/core/data/dociter.cxx
index 35148072f387..6bb47df2a9d8 100644
--- a/sc/source/core/data/dociter.cxx
+++ b/sc/source/core/data/dociter.cxx
@@ -2394,8 +2394,6 @@ ScHorizontalAttrIterator::ScHorizontalAttrIterator(
ScDocument& rDocument, SCTAB
assert(nTab < rDoc.GetTableCount() && "index out of bounds, FIX IT");
assert(rDoc.maTabs[nTab]);
- nEndCol = rDoc.maTabs[nTab]->ClampToMaxNonDefPatternColumn(nEndCol);
-
nRow = nStartRow;
nCol = nStartCol;
bRowEmpty = false;
@@ -2644,14 +2642,7 @@ ScDocAttrIterator::ScDocAttrIterator(ScDocument&
rDocument, SCTAB nTable,
nCol( nCol1 )
{
if ( ValidTab(nTab) && nTab < rDoc.GetTableCount() && rDoc.maTabs[nTab] )
- {
- SCCOL attrColCount =
rDoc.maTabs[nTab]->GetMaxNonDefPatternColumnsCount(nStartRow, nEndRow);
- if( nCol < attrColCount )
- {
- nEndCol = std::min<SCCOL>(nEndCol,attrColCount - 1);
- pColIter = rDoc.maTabs[nTab]->ColumnData(nCol).CreateAttrIterator(
nStartRow, nEndRow );
- }
- }
+ pColIter = rDoc.maTabs[nTab]->ColumnData(nCol).CreateAttrIterator(
nStartRow, nEndRow );
}
ScDocAttrIterator::~ScDocAttrIterator()
@@ -2780,16 +2771,11 @@ ScAttrRectIterator::ScAttrRectIterator(ScDocument&
rDocument, SCTAB nTable,
{
if ( ValidTab(nTab) && nTab < rDoc.GetTableCount() && rDoc.maTabs[nTab] )
{
- SCCOL attrColCount =
rDoc.maTabs[nTab]->GetMaxNonDefPatternColumnsCount(nStartRow, nEndRow);
- if( nCol1 < attrColCount )
- {
- nEndCol = std::min<SCCOL>(nEndCol,attrColCount - 1);
- pColIter =
rDoc.maTabs[nTab]->ColumnData(nIterStartCol).CreateAttrIterator( nStartRow,
nEndRow );
- while ( nIterEndCol < nEndCol &&
- rDoc.maTabs[nTab]->ColumnData(nIterEndCol).IsAllAttrEqual(
- rDoc.maTabs[nTab]->ColumnData(nIterEndCol+1),
nStartRow, nEndRow ) )
- ++nIterEndCol;
- }
+ pColIter =
rDoc.maTabs[nTab]->ColumnData(nIterStartCol).CreateAttrIterator( nStartRow,
nEndRow );
+ while ( nIterEndCol < nEndCol &&
+ rDoc.maTabs[nTab]->ColumnData(nIterEndCol).IsAllAttrEqual(
+ rDoc.maTabs[nTab]->ColumnData(nIterEndCol+1), nStartRow,
nEndRow ) )
+ ++nIterEndCol;
}
}
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index 5c35eba2fb9c..45bd5c777cc0 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -2699,21 +2699,6 @@ ScColumnsRange ScTable::GetColumnsRange(SCCOL nColBegin,
SCCOL nColEnd) const
return ScColumnsRange(ScColumnsRange::Iterator(beginIter),
ScColumnsRange::Iterator(endIter));
}
-SCCOL ScTable::ClampToMaxNonDefPatternColumn(SCCOL nCol, SCROW nRow1, SCROW
nRow2) const
-{
- // The purpose so to avoid unallocated columns if possible, so don't check
allocated ones.
- if( nCol < GetAllocatedColumnsCount())
- return nCol;
- return std::min<SCCOL>(nCol, GetMaxNonDefPatternColumnsCount( nRow1, nRow2
) - 1 );
-}
-
-SCCOL ScTable::GetMaxNonDefPatternColumnsCount(SCROW nRow1, SCROW nRow2) const
-{
- if( aDefaultColData.HasNonDefPattern(nRow1, nRow2))
- return GetDoc().GetMaxColCount();
- return GetAllocatedColumnsCount();
-}
-
// out-of-line the cold part of the CreateColumnIfNotExists function
void ScTable::CreateColumnIfNotExistsImpl( const SCCOL nScCol ) const
{
