[Libreoffice-commits] core.git: 2 commits - sc/inc sc/source

[Tor Lillqvist]

 sc/inc/tokenarray.hxx               |    1 +
 sc/source/core/tool/token.cxx       |    6 ++++++
 sc/source/filter/excel/excform8.cxx |   11 ++++++++++-
 sc/source/filter/inc/tokstack.hxx   |    2 +-
 4 files changed, 18 insertions(+), 2 deletions(-)

New commits:
commit ddc1f7d9a816e2cc970d48d2ccc2c0cd256e6e03
Author: Tor Lillqvist <tml at collabora.com>
Date:   Tue Mar 10 09:04:57 2015 +0200

    Fix issue in re-use of ScTokenArray objects from a TokenPool
    
    The TokenPool::operator[] is used to initialise and take into use an object
    from the pool. Which is a fascinating thing as such and probably not entirely
    in good style. Anyway, the objects in the pool are of type ScTokenArray, a
    class derived from FormulaTokenArray. The operator[] called the
    FormulaTokenArray::Clear() function to initialise the object. This left the
    fields added in ScTokenArray uninitialised, having whatever value the previous
    use of the object had set. Which of course is bad.
    
    In practice, this showed up in the handling of formulas in the .xls input
    filter. If an earlier (or the first?) formula had happened to be one for which
    we don't want to use OpenCL, the meVectorState of its ScTokenArray object in
    the pool had been set to FormulaVectorDisabled. When the same pool object was
    later re-used for another formula, it kept that same meVectorState, even if
    there was no reason to. Thus formula groups that should have been OpenCL
    accelerated weren't. This can have a significant impact on performance of
    document loading and recalculation for large documents.
    
    I added a function to ScTokenArray to clear (initialise) such an object, both
    the FormulaTokenArray part and the ScTokenArray-specific part, and call that
    instead. This fixes the issue.
    
    I named the added function ClearScTokenArray() to make it clear that it is a
    separate function. Sure, possibly Clear() should be made into a virtual of
    FormulaTokenArry and overridden in ScTokenArray, and the overriding Clear()
    would first call the base class's Clear(). But I can't be sure that there
    aren't other calls of FormulaTokenArray::Clear() that *must* mean the base
    class one. Better safe than sorry.
    
    And of course, I did *not* want to name the function in ScTokenArray also
    "Clear()", like in the base class, without it being virtual. That is horrible
    style in my opinion, even if there certainly is precedence for such even in
    the very same classes, i.e. the Clone() function...
    
    Change-Id: I0e0e13e5ca705603005a1e0a46866f095cd2ac4d

diff --git a/sc/inc/tokenarray.hxx b/sc/inc/tokenarray.hxx
index 4068745..0d3fa02 100644
--- a/sc/inc/tokenarray.hxx
+++ b/sc/inc/tokenarray.hxx
@@ -58,6 +58,7 @@ public:
     /// Assignment with references to FormulaToken entries (not copied!)
     ScTokenArray( const ScTokenArray& );
     virtual ~ScTokenArray();
+    void ClearScTokenArray();
     ScTokenArray* Clone() const;    /// True copy!
 
     // An estimate of the number of cells referenced by the token array
diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx
index 55a0601..203f378 100644
--- a/sc/source/core/tool/token.cxx
+++ b/sc/source/core/tool/token.cxx
@@ -1651,6 +1651,12 @@ ScTokenArray& ScTokenArray::operator=( const ScTokenArray& rArr )
     return *this;
 }
 
+void ScTokenArray::ClearScTokenArray()
+{
+    Clear();
+    meVectorState = FormulaVectorEnabled;
+}
+
 ScTokenArray* ScTokenArray::Clone() const
 {
     ScTokenArray* p = new ScTokenArray();
diff --git a/sc/source/filter/inc/tokstack.hxx b/sc/source/filter/inc/tokstack.hxx
index 7513d48..9e458d7 100644
--- a/sc/source/filter/inc/tokstack.hxx
+++ b/sc/source/filter/inc/tokstack.hxx
@@ -362,7 +362,7 @@ inline const TokenId TokenPool::LastId( void ) const
 
 const inline ScTokenArray* TokenPool::operator []( const TokenId& rId )
 {
-    pScToken->Clear();
+    pScToken->ClearScTokenArray();
 
     if( rId )
     {//...only if rId > 0!
commit bb0c05cebed131805d04919fac2b83030087451b
Author: Tor Lillqvist <tml at collabora.com>
Date:   Tue Mar 10 00:48:28 2015 +0200

    Also relative row references need to wrap around, like fdo#84556 for columns
    
    Change-Id: I07400d6dead66ec437436b5ea8b49491f8048335

diff --git a/sc/source/filter/excel/excform8.cxx b/sc/source/filter/excel/excform8.cxx
index ff4bd6c..ebe3b64 100644
--- a/sc/source/filter/excel/excform8.cxx
+++ b/sc/source/filter/excel/excform8.cxx
@@ -1479,7 +1479,16 @@ void ExcelToSc8::ExcRelToScRel8( sal_uInt16 nRow, sal_uInt16 nC, ScSingleRefData
 
         // R O W
         if( bRowRel )
-            rSRD.SetRelRow(static_cast<sal_Int16>(nRow));
+        {
+            SCROW nRelRow = static_cast<sal_Int16>(nRow);
+            sal_Int32 nDiff = aEingPos.Row() + nRelRow;
+            if (nDiff < 0)
+            {
+                // relative row references wrap around
+                nRelRow = 65536 + nRelRow;
+            }
+            rSRD.SetRelRow(nRelRow);
+        }
         else
             rSRD.SetAbsRow(std::min( static_cast<SCROW>(nRow), MAXROW));
     }