tdf#118662 Cleanup old hack with cloned SdrCaptionObj

XclObjComment formally cloned the SdrCaptionObj for a single
reason - to suppress functionality of the UNO API implementation
in SvxShape::GetBitmap - non-inserted SdrObjects did not create
Graphic return values.

Changed this to use an exclusive flag at SdrCaptionObj, only
accessible for XclObjComment. Due to bad/undefined behaviour
of SdrObjects that are not iinserted anywhere (see old comment
in XclObjComment) there is no way to return to cloning the
SdrObjects just to have them without being added to a SdrPage.

Also improved the time eater UNO API implementation SvxShape::GetBitmap
to use more modern stuff to create the Graphics needed. All the
time constructing a full E3DView and setting SdrObjects selected
and getting the selection as graphic is way too expensive. That
way save may even get somewhat faster.

Last was to cleanup the bInserted flag in SdrObject. It is no
longer needed, being inserted now depends on being a member
of an SdrObjList (Group or Page) - sounds normal anyways and
is a synergy effect of already done AW080 cleanups.

Checked now on linux. Problem is UnitTest 'testN777345' which
checks file "n777345.docx".

First point is that this only happens
  #if !defined(MACOSX)
  #if !defined(_WIN32)
so it's clear why I detected no problem on Windows.

Second point is that this test takes a checksum of a Graphic
that is created using getReplacementGraphic() this value *will*
change - of course - every time creation of that graphic is
even *slightly* modified, so from my POV this UnitTest is
defined to fill quite often. It may even create different
results on different systems (!). Adaption of the test value
 will be needed quite often and makes this test questionable.

Change-Id: If0918831a9cbd61b31298aeac7342e1913ee6c7a
Reviewed-on: https://gerrit.libreoffice.org/57758
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@cib.de>

8 files changed, 130 insertions, 69 deletions

diff --git a/include/svx/svdobj.hxx b/include/svx/svdobj.hxx
index a7a80105122b..52183e78ecc1 100644
--- a/include/svx/svdobj.hxx
+++ b/include/svx/svdobj.hxx
@@ -790,11 +790,14 @@ public:
     // is invalid, if this is a group object
     bool IsClosedObj() const { return bClosedObj;}
 
+    // tdf#118662 reorganize inserted state, no local bool needed anymore,
+    // it depends on being a member of a SdrObjList
+    void InsertedStateChange();
+    bool IsInserted() const { return nullptr != getParentSdrObjListFromSdrObject(); }
+
     bool IsEdgeObj() const { return bIsEdge;}
     bool Is3DObj() const { return bIs3DObj;}
     bool IsUnoObj() const { return bIsUnoObj;}
-    void SetInserted(bool bIns);
-    bool IsInserted() const { return bInserted;}
     void SetMoveProtect(bool bProt);
     bool IsMoveProtect() const { return bMovProt;}
     void SetResizeProtect(bool bProt);
@@ -1004,7 +1007,6 @@ private:
     // Position in the navigation order. SAL_MAX_UINT32 when not used.
     sal_uInt32                        mnNavigationPosition;
     SdrLayerID                        mnLayerID;
-    bool                              bInserted : 1;  // only if set to true, there are RepaintBroadcast & SetModify
     bool                              bNoPrint : 1;   // if true, the object is not printed.
     bool                              mbVisible : 1;  // if false, the object is not visible on screen (but maybe on printer, depending on bNoprint
     bool                              bMarkProt : 1;  // marking forbidden, persistent
diff --git a/include/svx/svdocapt.hxx b/include/svx/svdocapt.hxx
index b75d4867b8e1..124e83c206b2 100644
--- a/include/svx/svdocapt.hxx
+++ b/include/svx/svdocapt.hxx
@@ -46,15 +46,23 @@ private:
     friend class sdr::properties::CaptionProperties;
     friend class                SdrTextObj; // for ImpRecalcTail() during AutoGrow
 
+    // tdf#118662 exclusive friend function and setter for SuppressGetBitmap
+    friend void setSuppressGetBitmapFromXclObjComment(SdrCaptionObj* pSdrCaptionObj, bool bValue);
+    void setSuppressGetBitmap(bool bNew)
+    {
+        mbSuppressGetBitmap = bNew;
+    }
+
 protected:
     virtual std::unique_ptr<sdr::properties::BaseProperties> CreateObjectSpecificProperties() override;
     virtual std::unique_ptr<sdr::contact::ViewContact> CreateObjectSpecificViewContact() override;
 
 private:
-    tools::Polygon aTailPoly;  // the whole tail polygon
-    bool                        mbSpecialTextBoxShadow; // for calc special shadow, default FALSE
-    bool                        mbFixedTail; // for calc note box fixed tail, default FALSE
-    Point                       maFixedTailPos; // for calc note box fixed tail position.
+    tools::Polygon  aTailPoly;              // the whole tail polygon
+    bool            mbSpecialTextBoxShadow; // for calc special shadow, default FALSE
+    bool            mbFixedTail;            // for calc note box fixed tail, default FALSE
+    bool            mbSuppressGetBitmap;    // tdf#118662
+    Point           maFixedTailPos;         // for calc note box fixed tail position.
 
     SVX_DLLPRIVATE void ImpGetCaptParams(ImpCaptParams& rPara) const;
     SVX_DLLPRIVATE static void ImpCalcTail1(const ImpCaptParams& rPara, tools::Polygon& rPoly, tools::Rectangle const & rRect);
@@ -74,6 +82,9 @@ public:
         const tools::Rectangle& rRect,
         const Point& rTail);
 
+    // tdf#118662 getter for SuppressGetBitmap
+    bool isSuppressGetBitmap() const { return mbSuppressGetBitmap; }
+
     virtual void TakeObjInfo(SdrObjTransformInfoRec& rInfo) const override;
     virtual sal_uInt16 GetObjIdentifier() const override;
     virtual SdrCaptionObj* CloneSdrObject(SdrModel& rTargetModel) const override;
diff --git a/sc/source/filter/xcl97/xcl97rec.cxx b/sc/source/filter/xcl97/xcl97rec.cxx
index 2e3041662f1a..fa8390f41da6 100644
--- a/sc/source/filter/xcl97/xcl97rec.cxx
+++ b/sc/source/filter/xcl97/xcl97rec.cxx
@@ -487,29 +487,32 @@ void XclObj::SaveTextRecs( XclExpStream& rStrm )
         pTxo->Save( rStrm );
 }
 
-  // --- class XclObjComment ------------------------------------------
+// --- class XclObjComment ------------------------------------------
+
+// tdf#118662 static helper to allow single function access as friend in SdrCaptionObj
+void setSuppressGetBitmapFromXclObjComment(SdrCaptionObj* pSdrCaptionObj, bool bValue)
+{
+    if(nullptr != pSdrCaptionObj)
+    {
+        pSdrCaptionObj->setSuppressGetBitmap(bValue);
+    }
+}
 
 XclObjComment::XclObjComment( XclExpObjectManager& rObjMgr, const tools::Rectangle& rRect, const EditTextObject& rEditObj, SdrCaptionObj* pCaption, bool bVisible, const ScAddress& rAddress, const tools::Rectangle &rFrom, const tools::Rectangle &rTo ) :
     XclObj( rObjMgr, EXC_OBJTYPE_NOTE, true )
             , maScPos( rAddress )
-
-            // No need to CloneSdrObject(...) here, the SdrCaptionObj will exist
-            // during the whole im/export time. Seems that this was done
-            // initially to make UnitTest CppunitTest_sc_subsequent_export_test
-            // work (better: not crash) which had not added emfio/emfio to
-            // CppunitTest_sc_subsequent_export_test.mk and thus failed.
-            // Probably the Graphic created from the Clone was wrong.
-            // Problem with creating a Clone here is that it gets cloned, but not inserted to a
-            // SdrPage. In deeper export layers this then goes wrong since without being inserted
-            // to a Page, no SvxPage/UnoApiPage can be accessed. This was different in previous
-            // revisions of the code in that a SdrObject could be *not* inserted, but have a
-            // SdrPage*. That again was redundant, wrong and inconsequent.
             , mpCaption( pCaption )
-
             , mbVisible( bVisible )
             , maFrom ( rFrom )
             , maTo ( rTo )
 {
+    // tdf#118662 due to no longer cloning the SdrCaptionObj an old 'hack' using the
+    // fact that no Graphics gets created when a SdrObject is not inserted in a SdrPage
+    // does not work anymore. In SvxShape::GetBitmap that info was used, and here the
+    // SdrCaptionObj was cloned for the only reason to have one not added to a SdrPage.
+    // To emulate old behaviour, use a boolean flag at the SdrCaptionObj.
+    setSuppressGetBitmapFromXclObjComment(mpCaption, true);
+
     ProcessEscherObj( rObjMgr.GetRoot(), rRect, pCaption, bVisible);
     // TXO
     pTxo .reset(new XclTxo( rObjMgr.GetRoot(), rEditObj, pCaption ));
@@ -590,6 +593,8 @@ void XclObjComment::ProcessEscherObj( const XclExpRoot& rRoot, const tools::Rect
 
 XclObjComment::~XclObjComment()
 {
+    // tdf#118662 reset flag
+    setSuppressGetBitmapFromXclObjComment(mpCaption, false);
 }
 
 void XclObjComment::Save( XclExpStream& rStrm )
diff --git a/svx/source/svdraw/svdobj.cxx b/svx/source/svdraw/svdobj.cxx
index fdfd176b0011..89cee38cde37 100644
--- a/svx/source/svdraw/svdobj.cxx
+++ b/svx/source/svdraw/svdobj.cxx
@@ -354,7 +354,6 @@ SdrObject::SdrObject(SdrModel& rSdrModel)
 {
     bVirtObj         =false;
     bSnapRectDirty   =true;
-    bInserted        =false;
     bMovProt         =false;
     bSizProt         =false;
     bNoPrint         =false;
@@ -1715,7 +1714,7 @@ bool SdrObject::Equals(const SdrObject& rOtherObj) const
             bIs3DObj == rOtherObj.bIs3DObj && bIsEdge == rOtherObj.bIsEdge && bClosedObj == rOtherObj.bClosedObj &&
             bNotVisibleAsMaster == rOtherObj.bNotVisibleAsMaster && bEmptyPresObj == rOtherObj.bEmptyPresObj &&
             mbVisible == rOtherObj.mbVisible && bNoPrint == rOtherObj.bNoPrint && bSizProt == rOtherObj.bSizProt &&
-            bMovProt == rOtherObj.bMovProt && bInserted == rOtherObj.bInserted && bVirtObj == rOtherObj.bVirtObj &&
+            bMovProt == rOtherObj.bMovProt && bVirtObj == rOtherObj.bVirtObj &&
             mnLayerID == rOtherObj.mnLayerID && GetMergedItemSet().Equals(rOtherObj.GetMergedItemSet(), false) );
 }
 
@@ -2569,18 +2568,24 @@ SdrObject* SdrObject::DoConvertToPolyObj(bool /*bBezier*/, bool /*bAddText*/) co
 }
 
 
-void SdrObject::SetInserted(bool bIns)
+void SdrObject::InsertedStateChange()
 {
-    if (bIns!=IsInserted()) {
-        bInserted=bIns;
-        tools::Rectangle aBoundRect0(GetLastBoundRect());
-        if (bIns) SendUserCall(SdrUserCallType::Inserted,aBoundRect0);
-        else SendUserCall(SdrUserCallType::Removed,aBoundRect0);
+    const bool bIsInserted(nullptr != getParentSdrObjListFromSdrObject());
+    const tools::Rectangle aBoundRect0(GetLastBoundRect());
 
-        if (pPlusData!=nullptr && pPlusData->pBroadcast!=nullptr) {
-            SdrHint aHint(bIns?SdrHintKind::ObjectInserted:SdrHintKind::ObjectRemoved, *this);
-            pPlusData->pBroadcast->Broadcast(aHint);
-        }
+    if(bIsInserted)
+    {
+        SendUserCall(SdrUserCallType::Inserted, aBoundRect0);
+    }
+    else
+    {
+        SendUserCall(SdrUserCallType::Removed, aBoundRect0);
+    }
+
+    if(nullptr != pPlusData && nullptr != pPlusData->pBroadcast)
+    {
+        SdrHint aHint(bIsInserted ? SdrHintKind::ObjectInserted : SdrHintKind::ObjectRemoved, *this);
+        pPlusData->pBroadcast->Broadcast(aHint);
     }
 }
 
diff --git a/svx/source/svdraw/svdocapt.cxx b/svx/source/svdraw/svdocapt.cxx
index 4af2f1a3dfe4..7acb0bd4015c 100644
--- a/svx/source/svdraw/svdocapt.cxx
+++ b/svx/source/svdraw/svdocapt.cxx
@@ -193,7 +193,9 @@ SdrCaptionObj::SdrCaptionObj(SdrModel& rSdrModel)
 :   SdrRectObj(rSdrModel, OBJ_TEXT),
     aTailPoly(3),  // default size: 3 points = 2 lines
     mbSpecialTextBoxShadow(false),
-    mbFixedTail(false)
+    mbFixedTail(false),
+    mbSuppressGetBitmap(false),
+    maFixedTailPos()
 {
 }
 
@@ -204,7 +206,9 @@ SdrCaptionObj::SdrCaptionObj(
 :   SdrRectObj(rSdrModel, OBJ_TEXT,rRect),
     aTailPoly(3),  // default size: 3 points = 2 lines
     mbSpecialTextBoxShadow(false),
-    mbFixedTail(false)
+    mbFixedTail(false),
+    mbSuppressGetBitmap(false),
+    maFixedTailPos()
 {
     aTailPoly[0]=maFixedTailPos=rTail;
 }
diff --git a/svx/source/svdraw/svdpage.cxx b/svx/source/svdraw/svdpage.cxx
index 2573b58f0505..aefd4955a680 100644
--- a/svx/source/svdraw/svdpage.cxx
+++ b/svx/source/svdraw/svdpage.cxx
@@ -323,7 +323,7 @@ void SdrObjList::NbcInsertObject(SdrObject* pObj, size_t nPos)
         maSdrObjListOutRect.Union(pObj->GetCurrentBoundRect());
         maSdrObjListSnapRect.Union(pObj->GetSnapRect());
     }
-    pObj->SetInserted(true); // calls the UserCall (among others)
+    pObj->InsertedStateChange(); // calls the UserCall (among others)
 }
 
 void SdrObjList::InsertObject(SdrObject* pObj, size_t nPos)
@@ -384,7 +384,7 @@ SdrObject* SdrObjList::NbcRemoveObject(size_t nObjNum)
         pObj->GetViewContact().flushViewObjectContacts();
 
         DBG_ASSERT(pObj->IsInserted(),"The object does not have the status Inserted.");
-        pObj->SetInserted(false); // calls UserCall, among other
+        pObj->InsertedStateChange(); // calls UserCall, among other
         SetParentAtSdrObjectFromSdrObjList(*pObj, nullptr);
         if (!mbObjOrdNumsDirty)
         {
@@ -426,7 +426,7 @@ SdrObject* SdrObjList::RemoveObject(size_t nObjNum)
 
         pObj->getSdrModelFromSdrObject().SetChanged();
 
-        pObj->SetInserted(false); // calls, among other things, the UserCall
+        pObj->InsertedStateChange(); // calls, among other things, the UserCall
         SetParentAtSdrObjectFromSdrObjList(*pObj, nullptr);
 
         if (!mbObjOrdNumsDirty)
@@ -463,7 +463,7 @@ SdrObject* SdrObjList::NbcReplaceObject(SdrObject* pNewObj, size_t nObjNum)
     DBG_ASSERT(pObj!=nullptr,"SdrObjList::ReplaceObject: Could not find object to remove.");
     if (pObj!=nullptr) {
         DBG_ASSERT(pObj->IsInserted(),"SdrObjList::ReplaceObject: the object does not have status Inserted.");
-        pObj->SetInserted(false);
+        pObj->InsertedStateChange();
         SetParentAtSdrObjectFromSdrObjList(*pObj, nullptr);
         ReplaceObjectInContainer(*pNewObj,nObjNum);
 
@@ -477,7 +477,7 @@ SdrObject* SdrObjList::NbcReplaceObject(SdrObject* pNewObj, size_t nObjNum)
         // evtl. existing parent visualisations
         impChildInserted(*pNewObj);
 
-        pNewObj->SetInserted(true);
+        pNewObj->InsertedStateChange();
         SetSdrObjListRectsDirty();
     }
     return pObj;
@@ -508,7 +508,7 @@ SdrObject* SdrObjList::ReplaceObject(SdrObject* pNewObj, size_t nObjNum)
             pObj->getSdrModelFromSdrObject().Broadcast(aHint);
         }
 
-        pObj->SetInserted(false);
+        pObj->InsertedStateChange();
         SetParentAtSdrObjectFromSdrObjList(*pObj, nullptr);
         ReplaceObjectInContainer(*pNewObj,nObjNum);
 
@@ -522,7 +522,7 @@ SdrObject* SdrObjList::ReplaceObject(SdrObject* pNewObj, size_t nObjNum)
         // evtl. existing parent visualisations
         impChildInserted(*pNewObj);
 
-        pNewObj->SetInserted(true);
+        pNewObj->InsertedStateChange();
 
         // TODO: We need a different broadcast here.
         if (pNewObj->getSdrPageFromSdrObject()!=nullptr) {
diff --git a/svx/source/unodraw/unoshape.cxx b/svx/source/unodraw/unoshape.cxx
index a8fbab8db6c7..3ac01d5df9aa 100644
--- a/svx/source/unodraw/unoshape.cxx
+++ b/svx/source/unodraw/unoshape.cxx
@@ -85,6 +85,8 @@
 
 #include <svx/lathe3d.hxx>
 #include <svx/extrud3d.hxx>
+#include <svx/sdr/contact/viewcontact.hxx>
+#include <drawinglayer/geometry/viewinformation2d.hxx>
 
 #include <vcl/wmf.hxx>
 
@@ -690,48 +692,80 @@ uno::Any SvxShape::GetBitmap( bool bMetaFile /* = false */ ) const
     DBG_TESTSOLARMUTEX();
     uno::Any aAny;
 
-    if( !HasSdrObject() || !GetSdrObject()->IsInserted() || nullptr == GetSdrObject()->getSdrPageFromSdrObject() )
+    if(!HasSdrObject() || nullptr == GetSdrObject()->getSdrPageFromSdrObject())
+    {
+        return aAny;
+    }
+
+    // tdf#118662 Emulate old behaviour of XclObjComment (see there)
+    const SdrCaptionObj* pSdrCaptionObj(dynamic_cast<SdrCaptionObj*>(GetSdrObject()));
+    if(nullptr != pSdrCaptionObj && pSdrCaptionObj->isSuppressGetBitmap())
+    {
         return aAny;
+    }
 
+    // tdf#118662 instead of creating an E3dView instance every time to paint
+    // a single SdrObject, use the existing SdrObject::SingleObjectPainter to
+    // use less ressources and runtime
     ScopedVclPtrInstance< VirtualDevice > pVDev;
-    pVDev->SetMapMode(MapMode(MapUnit::Map100thMM));
-    SdrPage* pPage = GetSdrObject()->getSdrPageFromSdrObject();
+    const tools::Rectangle aBoundRect(GetSdrObject()->GetCurrentBoundRect());
 
-    std::unique_ptr<E3dView> pView(
-        new E3dView(
-            GetSdrObject()->getSdrModelFromSdrObject(),
-            pVDev.get()));
-    pView->hideMarkHandles();
-    SdrPageView* pPageView = pView->ShowSdrPage(pPage);
+    if(bMetaFile)
+    {
+        GDIMetaFile aMtf;
 
-    SdrObject *pTempObj = GetSdrObject();
-    pView->MarkObj(pTempObj,pPageView);
+        pVDev->SetMapMode(MapMode(MapUnit::Map100thMM));
+        pVDev->EnableOutput(false);
+        aMtf.Record(pVDev);
+        GetSdrObject()->SingleObjectPainter(*pVDev.get());
+        aMtf.Stop();
+        aMtf.WindStart();
+        aMtf.Move(-aBoundRect.Left(), -aBoundRect.Top());
+        aMtf.SetPrefMapMode(MapMode(MapUnit::Map100thMM));
+        aMtf.SetPrefSize(aBoundRect.GetSize());
 
-    tools::Rectangle aRect(pTempObj->GetCurrentBoundRect());
-    aRect.Justify();
-    Size aSize(aRect.GetSize());
+        SvMemoryStream aDestStrm(65535, 65535);
+
+        ConvertGDIMetaFileToWMF(
+            aMtf,
+            aDestStrm,
+            nullptr,
+            false);
 
-    GDIMetaFile aMtf( pView->GetMarkedObjMetaFile() );
-    if( bMetaFile )
-    {
-        SvMemoryStream aDestStrm( 65535, 65535 );
-        ConvertGDIMetaFileToWMF( aMtf, aDestStrm, nullptr, false );
         const uno::Sequence<sal_Int8> aSeq(
             static_cast< const sal_Int8* >(aDestStrm.GetData()),
             aDestStrm.GetEndOfData());
+
         aAny <<= aSeq;
     }
     else
     {
-        Graphic aGraph(aMtf);
-        aGraph.SetPrefSize(aSize);
-        aGraph.SetPrefMapMode(MapMode(MapUnit::Map100thMM));
+        const drawinglayer::primitive2d::Primitive2DContainer xPrimitives(
+            GetSdrObject()->GetViewContact().getViewIndependentPrimitive2DContainer());
 
-        Reference< awt::XBitmap > xBmp( aGraph.GetXGraphic(), UNO_QUERY );
-        aAny <<= xBmp;
-    }
+        if(!xPrimitives.empty())
+        {
+            const drawinglayer::geometry::ViewInformation2D aViewInformation2D;
+            const basegfx::B2DRange aRange(
+                xPrimitives.getB2DRange(aViewInformation2D));
+
+            if(!aRange.isEmpty())
+            {
+                const BitmapEx aBmp(
+                    convertPrimitive2DSequenceToBitmapEx(
+                        xPrimitives,
+                        aRange));
+
+                Graphic aGraph(aBmp);
 
-    pView->UnmarkAll();
+                aGraph.SetPrefSize(aBmp.GetPrefSize());
+                aGraph.SetPrefMapMode(aBmp.GetPrefMapMode());
+
+                Reference< awt::XBitmap > xBmp( aGraph.GetXGraphic(), UNO_QUERY );
+                aAny <<= xBmp;
+            }
+        }
+    }
 
     return aAny;
 }
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index 6f86c484bd42..2140f4c1dccb 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -400,7 +400,7 @@ DECLARE_OOXMLIMPORT_TEST(testN777345, "n777345.docx")
     Graphic aGraphic(xGraphic);
     // If this changes later, feel free to update it, but make sure it's not
     // the checksum of a white/transparent placeholder rectangle.
-    CPPUNIT_ASSERT_EQUAL(BitmapChecksum(SAL_CONST_UINT64(18203404956065762943)), aGraphic.GetChecksum());
+    CPPUNIT_ASSERT_EQUAL(BitmapChecksum(SAL_CONST_UINT64(12796261976794711810)), aGraphic.GetChecksum());
 #endif
 #endif
 }