[Libreoffice-commits] core.git: Branch 'feature/formula-core-rework' - 3 commits - sc/inc sc/qa sc/source

Kohei Yoshida kohei.yoshida at gmail.com
Fri Jun 21 09:03:45 PDT 2013


 sc/inc/column.hxx                       |   10 ---
 sc/qa/extras/testdocuments/Ranges-3.xls |binary
 sc/source/core/data/column.cxx          |   93 ++++++++++++++++----------------
 sc/source/core/data/column2.cxx         |   18 +++++-
 sc/source/core/data/column3.cxx         |    8 +-
 sc/source/core/data/formulacell.cxx     |   17 -----
 6 files changed, 72 insertions(+), 74 deletions(-)

New commits:
commit ea5a4946c7be75eb9fa6dab62c0bb6a14887adae
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Jun 21 11:54:18 2013 -0400

    Avoid having formula cell directly update text attributes.
    
    This would cause column cell and text attribute arrays to go out of
    sync, because the formula cells may be stored and interpreted in places
    such as change track and conditional formatting.
    
    We still need to find a good way to mark text attributes "obsolete" when
    formula cells are re-calculated.
    
    Change-Id: Ida612806d3afec23c5ae501f2fc8cc7d52e019a8

diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 845fa33..3ce95d4 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -728,11 +728,8 @@ void ScFormulaCell::Compile( const OUString& rFormula, bool bNoListening,
         CompileTokenArray( bNoListening );
     }
     else
-    {
         bChanged = true;
-        pDocument->SetTextWidth(aPos, TEXTWIDTH_DIRTY);
-        pDocument->SetScriptType(aPos, SC_SCRIPTTYPE_UNKNOWN);
-    }
+
     if ( bWasInFormulaTree )
         pDocument->PutInFormulaTree( this );
 }
@@ -822,11 +819,7 @@ void ScFormulaCell::CompileXML( ScProgress& rProgress )
             pDocument->AddSubTotalCell(this);
     }
     else
-    {
         bChanged = true;
-        pDocument->SetTextWidth(aPos, TEXTWIDTH_DIRTY);
-        pDocument->SetScriptType(aPos, SC_SCRIPTTYPE_UNKNOWN);
-    }
 
     //  Same as in Load: after loading, it must be known if ocMacro is in any formula
     //  (for macro warning, CompileXML is called at the end of loading XML file)
@@ -1096,8 +1089,6 @@ void ScFormulaCell::Interpret()
                             pIterCell->bTableOpDirty = false;
                             pIterCell->aResult.SetResultError( errNoConvergence);
                             pIterCell->bChanged = true;
-                            pDocument->SetTextWidth(pIterCell->aPos, TEXTWIDTH_DIRTY);
-                            pDocument->SetScriptType(pIterCell->aPos, SC_SCRIPTTYPE_UNKNOWN);
                         }
                     }
                     // End this iteration and remove entries.
@@ -1389,11 +1380,7 @@ void ScFormulaCell::InterpretTail( ScInterpretTailParameter eTailParam )
             aResult.SetResultError( nErr);
             bChanged = bContentChanged = true;
         }
-        if( bChanged )
-        {
-            pDocument->SetTextWidth(aPos, TEXTWIDTH_DIRTY);
-            pDocument->SetScriptType(aPos, SC_SCRIPTTYPE_UNKNOWN);
-        }
+
         if (bContentChanged && pDocument->IsStreamValid(aPos.Tab()))
         {
             // pass bIgnoreLock=true, because even if called from pending row height update,
commit d0c5ec0220f5dd8fe5285709ae5d6165e6a3027f
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Jun 21 10:57:35 2013 -0400

    Add more calls to CellStorageModified() when it's called for.
    
    Change-Id: Ib7a7abd82060b19f7911f1fef5ccccff5d850055

diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index e287a5b..e7c58c1 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -464,7 +464,7 @@ private:
     sc::CellStoreType::iterator GetPositionToInsert( const sc::CellStoreType::iterator& it, SCROW nRow );
     void ActivateNewFormulaCell( ScFormulaCell* pCell );
     void BroadcastNewCell( SCROW nRow );
-    void UpdateScriptType( sc::CellTextAttr& rAttr, SCROW nRow );
+    bool UpdateScriptType( sc::CellTextAttr& rAttr, SCROW nRow );
 
     const ScFormulaCell* FetchFormulaCell( SCROW nRow ) const;
 
@@ -476,7 +476,7 @@ private:
      * Called whenever the state of cell array gets modified i.e. new cell
      * insertion, cell removal or relocation, cell value update and so on.
      *
-     * Call this only from those methods where maItems is modified directly.
+     * Call this only from those methods where maCells is modified directly.
      */
     void CellStorageModified();
 
diff --git a/sc/qa/extras/testdocuments/Ranges-3.xls b/sc/qa/extras/testdocuments/Ranges-3.xls
index c23b70a..5151fa7 100644
Binary files a/sc/qa/extras/testdocuments/Ranges-3.xls and b/sc/qa/extras/testdocuments/Ranges-3.xls differ
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index 66166be..bd1eeae 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -1369,6 +1369,8 @@ void ScColumn::CopyStaticToDocument(SCROW nRow1, SCROW nRow2, ScColumn& rDestCol
         if (bLastBlock)
             break;
     }
+
+    rDestCol.CellStorageModified();
 }
 
 void ScColumn::CopyCellToDocument( SCROW nSrcRow, SCROW nDestRow, ScColumn& rDestCol )
@@ -1416,6 +1418,8 @@ void ScColumn::CopyCellToDocument( SCROW nSrcRow, SCROW nDestRow, ScColumn& rDes
         rDestCol.maCellTextAttrs.set(nDestRow, maCellTextAttrs.get<sc::CellTextAttr>(nSrcRow));
     else
         rDestCol.maCellTextAttrs.set_empty(nDestRow, nDestRow);
+
+    rDestCol.CellStorageModified();
 }
 
 namespace {
@@ -1918,9 +1922,6 @@ void ScColumn::SwapCol(ScColumn& rCol)
     maCells.swap(rCol.maCells);
     maCellTextAttrs.swap(rCol.maCellTextAttrs);
 
-    CellStorageModified();
-    rCol.CellStorageModified();
-
     ScAttrArray* pTempAttr = rCol.pAttrArray;
     rCol.pAttrArray = pAttrArray;
     pAttrArray = pTempAttr;
@@ -1934,6 +1935,10 @@ void ScColumn::SwapCol(ScColumn& rCol)
     // Reset column positions in formula cells.
     resetColumnPosition(maCells, nCol);
     resetColumnPosition(rCol.maCells, rCol.nCol);
+
+    CellStorageModified();
+    rCol.CellStorageModified();
+
 }
 
 void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol)
@@ -2107,17 +2112,21 @@ class InsertTabUpdater
     SCTAB mnTab;
     SCTAB mnInsPos;
     SCTAB mnNewSheets;
+    bool mbModified;
+
 public:
     InsertTabUpdater(sc::CellTextAttrStoreType& rTextAttrs, SCTAB nTab, SCTAB nInsPos, SCTAB nNewSheets) :
         mrTextAttrs(rTextAttrs),
         miAttrPos(rTextAttrs.begin()),
         mnTab(nTab),
         mnInsPos(nInsPos),
-        mnNewSheets(nNewSheets) {}
+        mnNewSheets(nNewSheets),
+        mbModified(false) {}
 
     void operator() (size_t /*nRow*/, ScFormulaCell* pCell)
     {
         pCell->UpdateInsertTab(mnInsPos, mnNewSheets);
+        mbModified = true;
     }
 
     void operator() (size_t nRow, EditTextObject* pCell)
@@ -2125,7 +2134,10 @@ public:
         editeng::FieldUpdater aUpdater = pCell->GetFieldUpdater();
         aUpdater.updateTableFields(mnTab);
         miAttrPos = mrTextAttrs.set(miAttrPos, nRow, sc::CellTextAttr());
+        mbModified = true;
     }
+
+    bool isModified() const { return mbModified; }
 };
 
 class DeleteTabUpdater
@@ -2136,6 +2148,7 @@ class DeleteTabUpdater
     SCTAB mnSheets;
     SCTAB mnTab;
     bool mbIsMove;
+    bool mbModified;
 public:
     DeleteTabUpdater(sc::CellTextAttrStoreType& rTextAttrs, SCTAB nDelPos, SCTAB nSheets, SCTAB nTab, bool bIsMove) :
         mrTextAttrs(rTextAttrs),
@@ -2143,11 +2156,13 @@ public:
         mnDelPos(nDelPos),
         mnSheets(nSheets),
         mnTab(nTab),
-        mbIsMove(bIsMove) {}
+        mbIsMove(bIsMove),
+        mbModified(false) {}
 
     void operator() (size_t, ScFormulaCell* pCell)
     {
         pCell->UpdateDeleteTab(mnDelPos, mbIsMove, mnSheets);
+        mbModified = true;
     }
 
     void operator() (size_t nRow, EditTextObject* pCell)
@@ -2155,7 +2170,10 @@ public:
         editeng::FieldUpdater aUpdater = pCell->GetFieldUpdater();
         aUpdater.updateTableFields(mnTab);
         miAttrPos = mrTextAttrs.set(miAttrPos, nRow, sc::CellTextAttr());
+        mbModified = true;
     }
+
+    bool isModified() const { return mbModified; }
 };
 
 class InsertAbsTabUpdater
@@ -2164,16 +2182,19 @@ class InsertAbsTabUpdater
     sc::CellTextAttrStoreType::iterator miAttrPos;
     SCTAB mnTab;
     SCTAB mnNewPos;
+    bool mbModified;
 public:
     InsertAbsTabUpdater(sc::CellTextAttrStoreType& rTextAttrs, SCTAB nTab, SCTAB nNewPos) :
         mrTextAttrs(rTextAttrs),
         miAttrPos(rTextAttrs.begin()),
         mnTab(nTab),
-        mnNewPos(nNewPos) {}
+        mnNewPos(nNewPos),
+        mbModified(false) {}
 
     void operator() (size_t /*nRow*/, ScFormulaCell* pCell)
     {
         pCell->UpdateInsertTabAbs(mnNewPos);
+        mbModified = true;
     }
 
     void operator() (size_t nRow, EditTextObject* pCell)
@@ -2181,7 +2202,10 @@ public:
         editeng::FieldUpdater aUpdater = pCell->GetFieldUpdater();
         aUpdater.updateTableFields(mnTab);
         miAttrPos = mrTextAttrs.set(miAttrPos, nRow, sc::CellTextAttr());
+        mbModified = true;
     }
+
+    bool isModified() const { return mbModified; }
 };
 
 class MoveTabUpdater
@@ -2191,17 +2215,20 @@ class MoveTabUpdater
     SCTAB mnTab;
     SCTAB mnOldPos;
     SCTAB mnNewPos;
+    bool mbModified;
 public:
     MoveTabUpdater(sc::CellTextAttrStoreType& rTextAttrs, SCTAB nTab, SCTAB nOldPos, SCTAB nNewPos) :
         mrTextAttrs(rTextAttrs),
         miAttrPos(rTextAttrs.begin()),
         mnTab(nTab),
         mnOldPos(nOldPos),
-        mnNewPos(nNewPos) {}
+        mnNewPos(nNewPos),
+        mbModified(false) {}
 
     void operator() (size_t /*nRow*/, ScFormulaCell* pCell)
     {
         pCell->UpdateMoveTab(mnOldPos, mnNewPos, mnTab);
+        mbModified = true;
     }
 
     void operator() (size_t nRow, EditTextObject* pCell)
@@ -2209,14 +2236,18 @@ public:
         editeng::FieldUpdater aUpdater = pCell->GetFieldUpdater();
         aUpdater.updateTableFields(mnTab);
         miAttrPos = mrTextAttrs.set(miAttrPos, nRow, sc::CellTextAttr());
+        mbModified = true;
     }
+
+    bool isModified() const { return mbModified; }
 };
 
 class UpdateCompileHandler
 {
-    bool mbForceIfNameInUse;
+    bool mbForceIfNameInUse:1;
 public:
-    UpdateCompileHandler(bool bForceIfNameInUse) : mbForceIfNameInUse(bForceIfNameInUse) {}
+    UpdateCompileHandler(bool bForceIfNameInUse) :
+        mbForceIfNameInUse(bForceIfNameInUse) {}
 
     void operator() (size_t /*nRow*/, ScFormulaCell* pCell)
     {
@@ -2541,6 +2572,8 @@ void ScColumn::UpdateInsertTabOnlyCells(SCTAB nInsPos, SCTAB nNewSheets)
 {
     InsertTabUpdater aFunc(maCellTextAttrs, nTab, nInsPos, nNewSheets);
     sc::ProcessFormulaEditText(maCells, aFunc);
+    if (aFunc.isModified())
+        CellStorageModified();
 }
 
 void ScColumn::UpdateDeleteTab(SCTAB nDelPos, bool bIsMove, ScColumn* /*pRefUndo*/, SCTAB nSheets)
@@ -2553,12 +2586,16 @@ void ScColumn::UpdateDeleteTab(SCTAB nDelPos, bool bIsMove, ScColumn* /*pRefUndo
 
     DeleteTabUpdater aFunc(maCellTextAttrs, nDelPos, nSheets, nTab, bIsMove);
     sc::ProcessFormulaEditText(maCells, aFunc);
+    if (aFunc.isModified())
+        CellStorageModified();
 }
 
 void ScColumn::UpdateInsertTabAbs(SCTAB nNewPos)
 {
     InsertAbsTabUpdater aFunc(maCellTextAttrs, nTab, nNewPos);
     sc::ProcessFormulaEditText(maCells, aFunc);
+    if (aFunc.isModified())
+        CellStorageModified();
 }
 
 void ScColumn::UpdateMoveTab( SCTAB nOldPos, SCTAB nNewPos, SCTAB nTabNo )
@@ -2568,6 +2605,8 @@ void ScColumn::UpdateMoveTab( SCTAB nOldPos, SCTAB nNewPos, SCTAB nTabNo )
 
     MoveTabUpdater aFunc(maCellTextAttrs, nTab, nOldPos, nNewPos);
     sc::ProcessFormulaEditText(maCells, aFunc);
+    if (aFunc.isModified())
+        CellStorageModified();
 }
 
 
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 8c52e5c..8332465 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -1624,6 +1624,7 @@ void ScColumn::ResetCellTextAttrs()
     CellTextAttrInitializer aFunc;
     std::for_each(maCells.begin(), maCells.end(), aFunc);
     aFunc.swap(maCellTextAttrs);
+    CellStorageModified();
 }
 
 void ScColumn::SwapCellTextAttrs( SCROW nRow1, SCROW nRow2 )
@@ -1673,6 +1674,8 @@ void ScColumn::SwapCellTextAttrs( SCROW nRow1, SCROW nRow2 )
     sc::CellTextAttr aVal1 = sc::celltextattr_block::at(*it1->data, aPos1.second); // make a copy.
     it1 = maCellTextAttrs.set_empty(it1, nRow1, nRow1);
     maCellTextAttrs.set(it1, nRow2, aVal1);
+
+    CellStorageModified();
 }
 
 SvtBroadcaster* ScColumn::GetBroadcaster(SCROW nRow)
@@ -1713,6 +1716,7 @@ void ScColumn::SetTextWidth(SCROW nRow, sal_uInt16 nWidth)
     sc::CellTextAttr aVal = maCellTextAttrs.get<sc::CellTextAttr>(nRow);
     aVal.mnTextWidth = nWidth;
     maCellTextAttrs.set(nRow, aVal);
+    CellStorageModified();
 }
 
 sal_uInt8 ScColumn::GetScriptType( SCROW nRow ) const
@@ -1736,7 +1740,7 @@ sal_uInt8 ScColumn::GetRangeScriptType(
     itPos = aRet.first; // Track the position of cell text attribute array.
 
     sal_uInt8 nScriptType = 0;
-
+    bool bUpdated = false;
     if (itPos->type == sc::element_type_celltextattr)
     {
         sc::celltextattr_block::iterator it = sc::celltextattr_block::begin(*itPos->data);
@@ -1748,7 +1752,8 @@ sal_uInt8 ScColumn::GetRangeScriptType(
                 return nScriptType;
 
             sc::CellTextAttr& rVal = *it;
-            UpdateScriptType(rVal, nRow);
+            if (UpdateScriptType(rVal, nRow))
+                bUpdated = true;
             nScriptType |= rVal.mnScriptType;
         }
     }
@@ -1779,11 +1784,16 @@ sal_uInt8 ScColumn::GetRangeScriptType(
                 return nScriptType;
 
             sc::CellTextAttr& rVal = *it;
-            UpdateScriptType(rVal, nRow);
+            if (UpdateScriptType(rVal, nRow))
+                bUpdated = true;
+
             nScriptType |= rVal.mnScriptType;
         }
     }
 
+    if (bUpdated)
+        CellStorageModified();
+
     return nScriptType;
 }
 
@@ -1800,12 +1810,14 @@ void ScColumn::SetScriptType( SCROW nRow, sal_uInt8 nType )
         sc::CellTextAttr aVal = maCellTextAttrs.get<sc::CellTextAttr>(nRow);
         aVal.mnScriptType = nType;
         maCellTextAttrs.set(nRow, aVal);
+        CellStorageModified();
     }
     else
     {
         sc::CellTextAttr aVal = maCellTextAttrs.get<sc::CellTextAttr>(nRow);
         aVal.mnScriptType = nType;
         maCellTextAttrs.set(nRow, aVal);
+        CellStorageModified();
     }
 }
 
diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index d2eacdd..10717e2 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -455,17 +455,17 @@ void ScColumn::BroadcastNewCell( SCROW nRow )
     pDocument->Broadcast(aHint);
 }
 
-void ScColumn::UpdateScriptType( sc::CellTextAttr& rAttr, SCROW nRow )
+bool ScColumn::UpdateScriptType( sc::CellTextAttr& rAttr, SCROW nRow )
 {
     if (rAttr.mnScriptType != SC_SCRIPTTYPE_UNKNOWN)
         // Already updated. Nothing to do.
-        return;
+        return false;
 
     // Script type not yet determined. Determine the real script
     // type, and store it.
     const ScPatternAttr* pPattern = GetPattern(nRow);
     if (!pPattern)
-        return;
+        return false;
 
     ScRefCellValue aCell;
     ScAddress aPos(nCol, nRow, nTab);
@@ -490,6 +490,7 @@ void ScColumn::UpdateScriptType( sc::CellTextAttr& rAttr, SCROW nRow )
 
     // Store the real script type to the array.
     rAttr.mnScriptType = pDocument->GetStringScriptType(aStr);
+    return true;
 }
 
 namespace {
@@ -1346,6 +1347,7 @@ void ScColumn::MixData(
     sc::ParseAll(rSrcCol.maCells.begin(), rSrcCol.maCells, nRow1, nRow2, aFunc, aFunc);
 
     aFunc.commit(p);
+    CellStorageModified();
 }
 
 
commit 7d1a6172daec9784e0caa6455f729f8e9877fcef
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Jun 21 09:36:50 2013 -0400

    Remove unused method.
    
    Change-Id: Id49307f8b7050e1f95cd589421f01a85b4e992b2

diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index c0b9c35..e287a5b 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -489,12 +489,6 @@ private:
     void ResetCellTextAttrs();
 
     void SwapCellTextAttrs( SCROW nRow1, SCROW nRow2 );
-
-    /**
-     * Retrieve the cell value and set that slot empty. The ownership of that
-     * cell value moves to the returned cell value object.
-     */
-    void ReleaseCellValue( sc::CellStoreType::iterator& itPos, SCROW nRow, ScCellValue& rVal );
 };
 
 #endif
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index c7ee06b..66166be 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -806,42 +806,6 @@ ScRefCellValue ScColumn::GetCellValue( sc::CellStoreType::const_iterator& itPos,
     return aVal;
 }
 
-void ScColumn::ReleaseCellValue( sc::CellStoreType::iterator& itPos, SCROW nRow, ScCellValue& rVal )
-{
-    std::pair<sc::CellStoreType::iterator,size_t> aPos = maCells.position(itPos, nRow);
-    itPos = aPos.first; // Store it for the next iteration.
-    if (aPos.first == maCells.end())
-        return;
-
-    switch (itPos->type)
-    {
-        case sc::element_type_numeric:
-            // Numeric cell
-            itPos = maCells.release(itPos, nRow, rVal.mfValue);
-            rVal.meType = CELLTYPE_VALUE;
-        break;
-        case sc::element_type_string:
-        {
-            // Make a copy until we implement shared strings...
-            OUString aStr;
-            itPos = maCells.release(itPos, nRow, aStr);
-            rVal.mpString = new OUString(aStr);
-            rVal.meType = CELLTYPE_STRING;
-        }
-        break;
-        case sc::element_type_edittext:
-            itPos = maCells.release(itPos, nRow, rVal.mpEditText);
-            rVal.meType = CELLTYPE_EDIT;
-        break;
-        case sc::element_type_formula:
-            itPos = maCells.release(itPos, nRow, rVal.mpFormula);
-            rVal.meType = CELLTYPE_FORMULA;
-        break;
-        default:
-            ;
-    }
-}
-
 namespace {
 
 ScFormulaCell* cloneFormulaCell(ScDocument* pDoc, const ScAddress& rNewPos, ScFormulaCell& rOldCell)


More information about the Libreoffice-commits mailing list