tdf#107392 ODF import: fix z-order sorting of SVG images

The problem was that in case the document has shapes where the order
does not match the z-index order, so sorting is needed, then sorting
failed to take the multi-image feature into account. E.g. SVG images
have a PNG fallback, but at the end of the shape import the PNG
fallback is removed, which means the "actual" (not the "wished") z-index
of the shapes after the SVG image has to be adjusted.

Without this happening SvxDrawPage::getByIndex() (or in case of Writer,
SwTextBoxHelper::getByIndex()) will throw when the importer calls
getByIndex(3) but we only have 3 shapes. This results in not honoring
the z-index request of the remaining shapes.

Regression from commit 44cfc7cb6533d827fd2d6e586d92c61d7d7f7a70 (re-base
on ALv2 code. Includes (at least) relevant parts of:, 2012-10-09), from
the

        Svg: Reintegrated Svg replacement from /branches/alg/svgreplavement
        http://svn.apache.org/viewvc?view=revision&revision=1220836

part.

Change-Id: Ibe880e5c6c74b728b4a760498720ee31f052b726
Reviewed-on: https://gerrit.libreoffice.org/36998
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Jenkins <ci@libreoffice.org>

5 files changed, 46 insertions, 7 deletions

diff --git a/xmloff/source/draw/shapeimport.cxx b/xmloff/source/draw/shapeimport.cxx
index 44c648fa641f..4f55f1ec2726 100644
--- a/xmloff/source/draw/shapeimport.cxx
+++ b/xmloff/source/draw/shapeimport.cxx
@@ -698,6 +698,8 @@ struct ZOrderHint
 {
     sal_Int32 nIs;
     sal_Int32 nShould;
+    /// The hint is for this shape.
+    uno::Reference<drawing::XShape> xShape;
 
     bool operator<(const ZOrderHint& rComp) const { return nShould < rComp.nShould; }
 };
@@ -832,22 +834,23 @@ void XMLShapeImportHelper::popGroupAndSort()
     {
         mpImpl->mpSortContext->popGroupAndSort();
     }
-    catch( uno::Exception& )
+    catch( const uno::Exception& rException )
     {
-        OSL_FAIL("exception while sorting shapes, sorting failed!");
+        SAL_WARN("xmloff", "exception while sorting shapes, sorting failed: " << rException.Message);
     }
 
     // put parent on top and drop current context, we are done
     mpImpl->mpSortContext = mpImpl->mpSortContext->mpParentContext;
 }
 
-void XMLShapeImportHelper::shapeWithZIndexAdded( css::uno::Reference< css::drawing::XShape >&, sal_Int32 nZIndex )
+void XMLShapeImportHelper::shapeWithZIndexAdded( css::uno::Reference< css::drawing::XShape >& xShape, sal_Int32 nZIndex )
 {
     if( mpImpl->mpSortContext)
     {
         ZOrderHint aNewHint;
         aNewHint.nIs = mpImpl->mpSortContext->mnCurrentZ++;
         aNewHint.nShould = nZIndex;
+        aNewHint.xShape = xShape;
 
         if( nZIndex == -1 )
         {
@@ -862,6 +865,37 @@ void XMLShapeImportHelper::shapeWithZIndexAdded( css::uno::Reference< css::drawi
     }
 }
 
+void XMLShapeImportHelper::shapeRemoved(const uno::Reference<drawing::XShape>& xShape)
+{
+    auto it = std::find_if(mpImpl->mpSortContext->maZOrderList.begin(), mpImpl->mpSortContext->maZOrderList.end(), [&xShape](const ZOrderHint& rHint)
+    {
+        return rHint.xShape == xShape;
+    });
+    if (it == mpImpl->mpSortContext->maZOrderList.end())
+        // Part of the unsorted list, nothing to do.
+        return;
+
+    sal_Int32 nZIndex = it->nIs;
+
+    for (it = mpImpl->mpSortContext->maZOrderList.begin(); it != mpImpl->mpSortContext->maZOrderList.end();)
+    {
+        if (it->nIs == nZIndex)
+        {
+            // This is xShape: remove it and adjust the max of indexes
+            // accordingly.
+            it = mpImpl->mpSortContext->maZOrderList.erase(it);
+            mpImpl->mpSortContext->mnCurrentZ--;
+            continue;
+        }
+        else if (it->nIs > nZIndex)
+            // On top of xShape: adjust actual index to reflect removal.
+            it->nIs--;
+
+        // On top of or below xShape.
+        ++it;
+    }
+}
+
 void XMLShapeImportHelper::addShapeConnection( css::uno::Reference< css::drawing::XShape >& rConnectorShape,
                          bool bStart,
                          const OUString& rDestShapeId,
diff --git a/xmloff/source/draw/ximpshap.cxx b/xmloff/source/draw/ximpshap.cxx
index 8f7196801da5..19bdfbf08b5a 100644
--- a/xmloff/source/draw/ximpshap.cxx
+++ b/xmloff/source/draw/ximpshap.cxx
@@ -3335,7 +3335,7 @@ SdXMLFrameShapeContext::~SdXMLFrameShapeContext()
 {
 }
 
-void SdXMLFrameShapeContext::removeGraphicFromImportContext(const SvXMLImportContext& rContext) const
+void SdXMLFrameShapeContext::removeGraphicFromImportContext(const SvXMLImportContext& rContext)
 {
     const SdXMLGraphicObjectShapeContext* pSdXMLGraphicObjectShapeContext = dynamic_cast< const SdXMLGraphicObjectShapeContext* >(&rContext);
 
diff --git a/xmloff/source/draw/ximpshap.hxx b/xmloff/source/draw/ximpshap.hxx
index 6fa574c9e3d9..56bdd7f84934 100644
--- a/xmloff/source/draw/ximpshap.hxx
+++ b/xmloff/source/draw/ximpshap.hxx
@@ -550,7 +550,7 @@ private:
 protected:
     /// helper to get the created xShape instance, needs to be overridden
     virtual OUString getGraphicURLFromImportContext(const SvXMLImportContext& rContext) const override;
-    virtual void removeGraphicFromImportContext(const SvXMLImportContext& rContext) const override;
+    virtual void removeGraphicFromImportContext(const SvXMLImportContext& rContext) override;
 
 public:
 
diff --git a/xmloff/source/text/XMLTextFrameContext.cxx b/xmloff/source/text/XMLTextFrameContext.cxx
index 20ee3842f933..c61c597fb7f9 100644
--- a/xmloff/source/text/XMLTextFrameContext.cxx
+++ b/xmloff/source/text/XMLTextFrameContext.cxx
@@ -728,7 +728,7 @@ void XMLTextFrameContext_Impl::Create( bool /*bHRefOrBase64*/ )
     }
 }
 
-void XMLTextFrameContext::removeGraphicFromImportContext(const SvXMLImportContext& rContext) const
+void XMLTextFrameContext::removeGraphicFromImportContext(const SvXMLImportContext& rContext)
 {
     const XMLTextFrameContext_Impl* pXMLTextFrameContext_Impl = dynamic_cast< const XMLTextFrameContext_Impl* >(&rContext);
 
@@ -739,6 +739,11 @@ void XMLTextFrameContext::removeGraphicFromImportContext(const SvXMLImportContex
             // just dispose to delete
             uno::Reference< lang::XComponent > xComp(pXMLTextFrameContext_Impl->GetPropSet(), UNO_QUERY);
 
+            // Inform shape importer about the removal so it can adjust
+            // z-indxes.
+            uno::Reference<drawing::XShape> xShape(xComp, uno::UNO_QUERY);
+            GetImport().GetShapeImport()->shapeRemoved(xShape);
+
             if(xComp.is())
             {
                 xComp->dispose();
diff --git a/xmloff/source/text/XMLTextFrameContext.hxx b/xmloff/source/text/XMLTextFrameContext.hxx
index 8f19eda2d70e..2f82ea50ffac 100644
--- a/xmloff/source/text/XMLTextFrameContext.hxx
+++ b/xmloff/source/text/XMLTextFrameContext.hxx
@@ -60,7 +60,7 @@ class XMLTextFrameContext : public SvXMLImportContext, public MultiImageImportHe
 protected:
     /// helper to get the created xShape instance, needs to be overridden
     virtual OUString getGraphicURLFromImportContext(const SvXMLImportContext& rContext) const override;
-    virtual void removeGraphicFromImportContext(const SvXMLImportContext& rContext) const override;
+    virtual void removeGraphicFromImportContext(const SvXMLImportContext& rContext) override;
 
 public: