tdf#84394: xls load: delay cell-anchoring of object groups

When cell-anchored object groups are loaded, their anchoring must be
delayed until all nested objects have been loaded, lest the invalid
rectangle dimensions lead to incorrect positioning of the object.

To achieve this, we keep track of the DffObjectData of the pending
group, and move the anchoring to a FinalizeObj() method.

Since DffObjectData has a const reference to a DffRecordHeader (which we
need when setting the object anchoring) whose scope has closed by the
time we call FinalizeObj() on the parent object, the stack of pending
DffObjectData has references to clones of the original DffRecordHeader
held in shared pointers. (This is to minimize the invasiveness of this
patch wrt the Import* API.)

Change-Id: Id23f5549dbc82306271cc02afc750f37eeea3ca2
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Reviewed-on: https://gerrit.libreoffice.org/24292
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Markus Mohrhard <markus.mohrhard@googlemail.com>

4 files changed, 83 insertions, 4 deletions

diff --git a/filter/source/msfilter/msdffimp.cxx b/filter/source/msfilter/msdffimp.cxx
index 77c2ecfc258b..b84bae177582 100644
--- a/filter/source/msfilter/msdffimp.cxx
+++ b/filter/source/msfilter/msdffimp.cxx
@@ -4137,6 +4137,12 @@ SdrObject* SvxMSDffManager::ImportGroup( const DffRecordHeader& rHd, SvStream& r
             }
         }
     }
+    if (size_t(nCalledByGroup) < maPendingGroupData.size())
+    {
+        // finalization for this group is pending, do it now
+        pRet = FinalizeObj(maPendingGroupData.back().first, pRet);
+        maPendingGroupData.pop_back();
+    }
     return pRet;
 }
 
@@ -4153,6 +4159,7 @@ SdrObject* SvxMSDffManager::ImportShape( const DffRecordHeader& rHd, SvStream& r
         return pRet;
 
     DffObjData aObjData( rHd, rClientRect, nCalledByGroup );
+
     aObjData.bRotateTextWithShape = ( GetSvxMSDffSettings() & SVXMSDFF_SETTINGS_IMPORT_EXCEL ) == 0;
     maShapeRecords.Consume( rSt );
     if( maShapeRecords.SeekToContent( rSt,
@@ -4812,6 +4819,18 @@ SdrObject* SvxMSDffManager::ImportShape( const DffRecordHeader& rHd, SvStream& r
         pRet->SetDescription( aAltText );
     }
 
+    // If this shape opens a new group, push back its object data because
+    // finalization will be called when nested objects have been imported;
+    // otherwise, just finalize here
+    if (size_t(nCalledByGroup) > maPendingGroupData.size())
+    {
+        std::shared_ptr<DffRecordHeader> rHdClone(new DffRecordHeader(aObjData.rSpHd));
+        maPendingGroupData.push_back( make_pair(DffObjData(rHdClone, aObjData), rHdClone) );
+    }
+    else
+    {
+        pRet = FinalizeObj(aObjData, pRet);
+    }
     return pRet;
 }
 
@@ -5485,6 +5504,12 @@ SdrObject* SvxMSDffManager::ProcessObj(SvStream& rSt,
     return pObj;
 };
 
+SdrObject* SvxMSDffManager::FinalizeObj(DffObjData& /* rObjData */, SdrObject* pObj)
+{
+    return pObj;
+}
+
+
 void SvxMSDffManager::StoreShapeOrder(sal_uLong         nId,
                                       sal_uLong         nTxBx,
                                       SdrObject*    pObject,
diff --git a/include/filter/msfilter/msdffimp.hxx b/include/filter/msfilter/msdffimp.hxx
index e0c23eeda75f..3b594d01b1b4 100644
--- a/include/filter/msfilter/msdffimp.hxx
+++ b/include/filter/msfilter/msdffimp.hxx
@@ -324,6 +324,23 @@ struct DffObjData
         bRotateTextWithShape( true ),
         bPageAnchor( true ),
         nCalledByGroup( nClByGroup ){}
+
+    // Clone a DffObjData _o_ by replacing its rSpHd with a shared_ptr to another one
+    DffObjData( const std::shared_ptr<DffRecordHeader>& rObjHd, const DffObjData& o) :
+        rSpHd( *rObjHd ),
+        aBoundRect( o.aBoundRect ),
+        nShapeId( o.nShapeId ),
+        nSpFlags( o.nSpFlags ),
+        eShapeType( o.eShapeType ),
+        bShapeType( o.bShapeType ),
+        bClientAnchor( o.bClientAnchor ),
+        bClientData( o.bClientData ),
+        bChildAnchor( o.bChildAnchor ),
+        bOpt( o.bOpt ),
+        bOpt2( o.bOpt2 ),
+        bRotateTextWithShape( o.bRotateTextWithShape ),
+        bPageAnchor( o.bPageAnchor ),
+        nCalledByGroup( o.nCalledByGroup ){}
 };
 
 #define DFF_RECORD_MANAGER_BUF_SIZE         64
@@ -407,6 +424,13 @@ protected:
     std::vector<FIDCL> maFidcls;
     OffsetMap       maDgOffsetTable;    ///< array of fileoffsets
 
+    /** When importing Excel files, cell anchor computations for non-page-anchored
+        groups must be done after all nested groups have been processed; for each open
+        group, the pending data is stored here. The storage also holds a shared_ptr to
+        the DffObjData ow DffRecordHeader to avoid it going out of scope except whe needed
+     */
+    std::vector< std::pair<DffObjData, std::shared_ptr<DffRecordHeader> > > maPendingGroupData;
+
     friend class DffPropertyReader;
 
     SvStream&       rStCtrl;
@@ -483,6 +507,13 @@ protected:
                                    void* pData,
                                    Rectangle& rTextRect,
                                    SdrObject* pObj = nullptr);
+
+    /** Object finalization, used by the Excel filter to correctly
+        compute the object anchoring after nested objects have been imported.
+    */
+    virtual SdrObject* FinalizeObj(DffObjData& rData,
+                                   SdrObject* pObj = nullptr);
+
     virtual bool GetColorFromPalette(sal_uInt16 nNum, Color& rColor) const;
 
     // Fontwork objects use a new implementation of ReadObjText because the old
diff --git a/sc/source/filter/excel/xiescher.cxx b/sc/source/filter/excel/xiescher.cxx
index d895e757872b..b76bd75f990e 100644
--- a/sc/source/filter/excel/xiescher.cxx
+++ b/sc/source/filter/excel/xiescher.cxx
@@ -3551,10 +3551,6 @@ SdrObject* XclImpDffConverter::ProcessObj( SvStream& rDffStrm, DffObjData& rDffO
     // process the SdrObject
     if( xSdrObj )
     {
-        // cell anchoring
-        if ( !rDffObjData.bPageAnchor )
-            ScDrawLayer::SetCellAnchoredFromPosition( *xSdrObj,  GetDoc(), xDrawObj->GetTab() );
-
         // filled without color -> set system window color
         if( GetPropertyBool( DFF_Prop_fFilled ) && !IsProperty( DFF_Prop_fillColor ) )
             xSdrObj->SetMergedItem( XFillColorItem( EMPTY_OUSTRING, GetPalette().GetColor( EXC_COLOR_WINDOWBACK ) ) );
@@ -3591,6 +3587,28 @@ SdrObject* XclImpDffConverter::ProcessObj( SvStream& rDffStrm, DffObjData& rDffO
     return xSdrObj.release();
 }
 
+SdrObject* XclImpDffConverter::FinalizeObj(DffObjData& rDffObjData, SdrObject* pOldSdrObj )
+{
+    XclImpDffConvData& rConvData = GetConvData();
+
+    /*  pOldSdrObj passes a generated SdrObject. This function owns this object
+        and can modify it. The function has either to return it back to caller
+        or to delete it by itself. */
+    SdrObjectPtr xSdrObj( pOldSdrObj );
+
+    // find the OBJ record data related to the processed shape
+    XclImpDrawObjRef xDrawObj = rConvData.mrDrawing.FindDrawObj( rDffObjData.rSpHd );
+
+    if( xSdrObj && xDrawObj )
+    {
+        // cell anchoring
+        if ( !rDffObjData.bPageAnchor )
+            ScDrawLayer::SetCellAnchoredFromPosition( *xSdrObj,  GetDoc(), xDrawObj->GetTab() );
+    }
+
+    return xSdrObj.release();
+}
+
 bool XclImpDffConverter::InsertControl( const Reference< XFormComponent >& rxFormComp,
         const css::awt::Size& /*rSize*/, Reference< XShape >* pxShape,
         bool /*bFloatingCtrl*/ )
diff --git a/sc/source/filter/inc/xiescher.hxx b/sc/source/filter/inc/xiescher.hxx
index 7041bbbe4706..35c3a5c8931b 100644
--- a/sc/source/filter/inc/xiescher.hxx
+++ b/sc/source/filter/inc/xiescher.hxx
@@ -980,6 +980,11 @@ private:
                             Rectangle& rTextRect,
                             SdrObject* pOldSdrObj = nullptr ) override;
 
+    /** Finalize a DFF object, sets anchor after nested objs have been loaded. */
+    virtual SdrObject*  FinalizeObj(
+                            DffObjData& rDffObjData,
+                            SdrObject* pOldSdrObj = nullptr ) override;
+
     // virtual functions of SvxMSConvertOCXControls
 
     /** Inserts the passed control rxFComp into the form. Needs call to SetCurrentForm() before. */