fdo#73001: Simplify the selection function logic & calculate correct results.

Fixing a bug and cleaning up the code all at the same time.  And don't forget
to write test for it as well.

Change-Id: Ia0322c4bebd4c5debcbfa4bb0902afbe581208b2

3 files changed, 70 insertions, 124 deletions

diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 60b7b42ab0f7..4d7b4bf44004 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -3254,7 +3254,6 @@ namespace {
 class UpdateSubTotalHandler
 {
     ScFunctionData& mrData;
-    ScFlatBoolRowSegments& mrHiddenRows;
 
     void update(double fVal, bool bVal)
     {
@@ -3311,22 +3310,25 @@ class UpdateSubTotalHandler
     }
 
 public:
-    UpdateSubTotalHandler(ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows) :
-        mrData(rData), mrHiddenRows(rHiddenRows) {}
+    UpdateSubTotalHandler(ScFunctionData& rData) : mrData(rData) {}
 
-    void operator() (size_t nRow, double fVal)
+    void operator() (size_t /*nRow*/, double fVal)
     {
-        if (mrHiddenRows.getValue(nRow))
-            return;
-
         update(fVal, true);
     }
 
-    void operator() (size_t nRow, ScFormulaCell* pCell)
+    void operator() (size_t /*nRow*/, const svl::SharedString&)
     {
-        if (mrHiddenRows.getValue(nRow))
-            return;
+        update(0.0, false);
+    }
 
+    void operator() (size_t /*nRow*/, const EditTextObject*)
+    {
+        update(0.0, false);
+    }
+
+    void operator() (size_t /*nRow*/, ScFormulaCell* pCell)
+    {
         double fVal = 0.0;
         bool bVal = false;
         if (mrData.eFunc != SUBTOTAL_FUNC_CNT2) // it doesn't interest us
@@ -3353,99 +3355,63 @@ public:
 
 //  multiple selections:
 void ScColumn::UpdateSelectionFunction(
-    const ScMarkData& rMark, ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows,
-    bool bDoExclude, SCROW nExStartRow, SCROW nExEndRow)
+    const ScMarkData& rMark, ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows )
 {
-    if ( rData.eFunc != SUBTOTAL_FUNC_SELECTION_COUNT )
+    sc::SingleColumnSpanSet aSpanSet;
+    aSpanSet.scan(rMark, nTab, nCol); // mark all selected rows.
+
+    // Exclude all hidden rows.
+    ScFlatBoolRowSegments::RangeData aRange;
+    SCROW nRow = 0;
+    while (nRow <= MAXROW)
     {
-        sc::SingleColumnSpanSet aSpanSet;
-        aSpanSet.scan(rMark, nTab, nCol);
-        if (bDoExclude)
-        {
-            aSpanSet.set(0, nExStartRow, false);
-            aSpanSet.set(nExEndRow+1, MAXROWCOUNT, false);
-        }
+        if (!rHiddenRows.getRangeData(nRow, aRange))
+            break;
 
-        sc::SingleColumnSpanSet::SpansType aSpans;
-        aSpanSet.getSpans(aSpans);
-        UpdateSubTotalHandler aFunc(rData, rHiddenRows);
-        sc::CellStoreType::iterator itCellPos = maCells.begin();
-        sc::SingleColumnSpanSet::SpansType::const_iterator it = aSpans.begin(), itEnd = aSpans.end();
-        for (; it != itEnd; ++it)
-        {
-            itCellPos = sc::ProcessFormulaNumeric(
-                itCellPos, maCells, it->mnRow1, it->mnRow2, aFunc);
-        }
+        if (aRange.mbValue)
+            // Hidden range detected.
+            aSpanSet.set(nRow, aRange.mnRow2, false);
+
+        nRow = aRange.mnRow2 + 1;
     }
-    else
-    {
-        SCROW nTop, nBottom;
 
-        // ScMarkData::GetArray() returns a valid array only if
-        // 'rMark.IsMultiMarked()' returns true.
-        // Since ScTable::UpdateSelectionFunction() already checked that first
-        // before calling this method it does not need to be repeated here.
+    sc::SingleColumnSpanSet::SpansType aSpans;
+    aSpanSet.getSpans(aSpans);
 
-        ScMarkArrayIter aIter(rMark.GetArray() + nCol);
-        ScFlatBoolRowSegments::RangeData aData;
+    sc::SingleColumnSpanSet::SpansType::const_iterator it = aSpans.begin(), itEnd = aSpans.end();
 
-        while (aIter.Next( nTop, nBottom ))
+    switch (rData.eFunc)
+    {
+        case SUBTOTAL_FUNC_SELECTION_COUNT:
         {
-            sal_Int32 nCellCount = 0;    // to get the count of selected visible cells
-            SCROW nRow = nTop;
-
-            while ( nRow <= nBottom )
+            // Simply count selected rows regardless of cell contents.
+            for (; it != itEnd; ++it)
+                rData.nCount += it->mnRow2 - it->mnRow1 + 1;
+        }
+        break;
+        case SUBTOTAL_FUNC_CNT2:
+        {
+            // We need to parse all non-empty cells.
+            sc::CellStoreType::const_iterator itCellPos = maCells.begin();
+            UpdateSubTotalHandler aFunc(rData);
+            for (; it != itEnd; ++it)
             {
-                if (!rHiddenRows.getRangeData(nRow, aData))     // failed to get range data
-                    break;
-
-                if (aData.mnRow2 > nBottom)
-                    aData.mnRow2 = nBottom;
-
-                if (!aData.mbValue)
-                {
-                    nCellCount += aData.mnRow2 - nRow + 1;
-
-                    // Till this point, nCellCount also includes count of those cells which are excluded
-                    // So, they should be decremented now.
-
-                    if ( bDoExclude && nExStartRow >= nRow && nExEndRow <= aData.mnRow2 )
-                        nCellCount -= nExEndRow - nExStartRow + 1;
-                }
-                nRow = aData.mnRow2 + 1;
+                itCellPos = sc::ParseAllNonEmpty(
+                    itCellPos, maCells, it->mnRow1, it->mnRow2, aFunc);
             }
-            rData.nCount += nCellCount;
         }
-    }
-}
-
-//  with bNoMarked ignore the multiple selections
-void ScColumn::UpdateAreaFunction(
-    ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows, SCROW nStartRow, SCROW nEndRow)
-{
-    if ( rData.eFunc != SUBTOTAL_FUNC_SELECTION_COUNT )
-    {
-        UpdateSubTotalHandler aFunc(rData, rHiddenRows);
-        sc::ProcessFormulaNumeric(
-            maCells.begin(), maCells, nStartRow, nEndRow, aFunc);
-    }
-    else
-    {
-        sal_Int32 nCellCount = 0;    // to get the count of selected visible cells
-        SCROW nRow = nStartRow;
-        ScFlatBoolRowSegments::RangeData aData;
-
-        while (nRow <= nEndRow)
+        break;
+        default:
         {
-            if (!rHiddenRows.getRangeData(nRow, aData))
-               break;
-
-            if (!aData.mbValue)
-                nCellCount += (aData.mnRow2 <= nEndRow ? aData.mnRow2 : nEndRow) - nRow + 1;
-
-            nRow = aData.mnRow2 + 1;
+            // We need to parse only numeric values.
+            sc::CellStoreType::const_iterator itCellPos = maCells.begin();
+            UpdateSubTotalHandler aFunc(rData);
+            for (; it != itEnd; ++it)
+            {
+                itCellPos = sc::ParseFormulaNumeric(
+                    itCellPos, maCells, it->mnRow1, it->mnRow2, aFunc);
+            }
         }
-        rData.nCount += nCellCount;
     }
 }
 
diff --git a/sc/source/core/data/documen4.cxx b/sc/source/core/data/documen4.cxx
index 0aa6383f1211..ed496961c38b 100644
--- a/sc/source/core/data/documen4.cxx
+++ b/sc/source/core/data/documen4.cxx
@@ -613,22 +613,17 @@ bool ScDocument::GetSelectionFunction( ScSubTotalFunc eFunc,
 {
     ScFunctionData aData(eFunc);
 
-    ScRange aSingle( rCursor );
-    if ( rMark.IsMarked() )
-        rMark.GetMarkArea(aSingle);
-
-    SCCOL nStartCol = aSingle.aStart.Col();
-    SCROW nStartRow = aSingle.aStart.Row();
-    SCCOL nEndCol = aSingle.aEnd.Col();
-    SCROW nEndRow = aSingle.aEnd.Row();
+    ScMarkData aMark(rMark);
+    aMark.MarkToMulti();
+    if (!aMark.IsMultiMarked())
+        aMark.SetMarkArea(rCursor);
 
     SCTAB nMax = static_cast<SCTAB>(maTabs.size());
-    ScMarkData::const_iterator itr = rMark.begin(), itrEnd = rMark.end();
+    ScMarkData::const_iterator itr = aMark.begin(), itrEnd = aMark.end();
 
     for (; itr != itrEnd && *itr < nMax && !aData.bError; ++itr)
         if (maTabs[*itr])
-            maTabs[*itr]->UpdateSelectionFunction( aData,
-                            nStartCol, nStartRow, nEndCol, nEndRow, rMark );
+            maTabs[*itr]->UpdateSelectionFunction(aData, aMark);
 
             //! rMark an UpdateSelectionFunction uebergeben !!!!!
 
diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx
index a92748568d2f..16ed68dd7de6 100644
--- a/sc/source/core/data/table3.cxx
+++ b/sc/source/core/data/table3.cxx
@@ -2308,30 +2308,15 @@ xub_StrLen ScTable::GetMaxNumberStringLen(
         return 0;
 }
 
-void ScTable::UpdateSelectionFunction( ScFunctionData& rData,
-                        SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow,
-                        const ScMarkData& rMark )
+void ScTable::UpdateSelectionFunction( ScFunctionData& rData, const ScMarkData& rMark )
 {
-    //  Cursor neben einer Markierung nicht beruecksichtigen:
-    //! nur noch MarkData uebergeben, Cursorposition ggf. hineinselektieren!!!
-    bool bSingle = ( rMark.IsMarked() || !rMark.IsMultiMarked() );
-
-    // Mehrfachselektion:
-
-    SCCOL nCol;
-    if ( rMark.IsMultiMarked() )
-        for (nCol=0; nCol<=MAXCOL && !rData.bError; nCol++)
-            if ( !pColFlags || !ColHidden(nCol) )
-                aCol[nCol].UpdateSelectionFunction( rMark, rData, *mpHiddenRows,
-                                                    bSingle && ( nCol >= nStartCol && nCol <= nEndCol ),
-                                                    nStartRow, nEndRow );
-
-    //  Einfachselektion (oder Cursor) nur wenn nicht negativ (und s.o.):
-
-    if ( bSingle && !rMark.IsMarkNegative() )
-        for (nCol=nStartCol; nCol<=nEndCol && !rData.bError; nCol++)
-            if ( !pColFlags || !ColHidden(nCol) )
-                aCol[nCol].UpdateAreaFunction( rData, *mpHiddenRows, nStartRow, nEndRow );
+    for (SCCOL nCol = 0; nCol <= MAXCOL && !rData.bError; ++nCol)
+    {
+        if (pColFlags && ColHidden(nCol))
+            continue;
+
+        aCol[nCol].UpdateSelectionFunction(rMark, rData, *mpHiddenRows);
+    }
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */