From c009bdb061d074bbddd58a16bdf65022e2099ba5 Mon Sep 17 00:00:00 2001 From: Kohei Yoshida Date: Mon, 6 Jan 2014 16:12:28 -0500 Subject: 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 (cherry picked from commit 4a7a6b46c0dc779581f271b9e6c13c365eca7ab8) --- sc/source/core/data/column2.cxx | 146 +++++++++++++++------------------------ sc/source/core/data/documen4.cxx | 17 ++--- sc/source/core/data/table3.cxx | 31 +++------ 3 files changed, 70 insertions(+), 124 deletions(-) (limited to 'sc/source/core/data') diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx index 307d42b6ec19..2adf63c83271 100644 --- a/sc/source/core/data/column2.cxx +++ b/sc/source/core/data/column2.cxx @@ -3239,7 +3239,6 @@ namespace { class UpdateSubTotalHandler { ScFunctionData& mrData; - ScFlatBoolRowSegments& mrHiddenRows; void update(double fVal, bool bVal) { @@ -3296,22 +3295,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 @@ -3338,99 +3340,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(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: */ -- cgit v1.2.3