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.lunak@collabora.com>

3 files changed, 65 insertions, 40 deletions

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);
     }
 }