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>
diff --git a/include/xmloff/shapeimport.hxx b/include/xmloff/shapeimport.hxx
index 4936b36..efc00a2 100644
--- a/include/xmloff/shapeimport.hxx
+++ b/include/xmloff/shapeimport.hxx
@@ -319,6 +319,9 @@ public:
void shapeWithZIndexAdded( css::uno::Reference< css::drawing::XShape >& rShape,
sal_Int32 nZIndex );
/// Updates the z-order of other shapes to be consistent again, needed due
/// to the removal of rShape.
void shapeRemoved(const css::uno::Reference<css::drawing::XShape>& rShape);
void addShapeConnection( css::uno::Reference< css::drawing::XShape >& rConnectorShape,
bool bStart,
diff --git a/include/xmloff/xmlmultiimagehelper.hxx b/include/xmloff/xmlmultiimagehelper.hxx
index 391f8a6..4a14d1a 100644
--- a/include/xmloff/xmlmultiimagehelper.hxx
+++ b/include/xmloff/xmlmultiimagehelper.hxx
@@ -33,7 +33,7 @@ private:
protected:
/// helper to get the created xShape instance, override this
virtual OUString getGraphicURLFromImportContext(const SvXMLImportContext& rContext) const = 0;
virtual void removeGraphicFromImportContext(const SvXMLImportContext& rContext) const = 0;
virtual void removeGraphicFromImportContext(const SvXMLImportContext& rContext) = 0;
public:
MultiImageImportHelper();
diff --git a/sw/qa/extras/odfimport/data/tdf107392.odt b/sw/qa/extras/odfimport/data/tdf107392.odt
new file mode 100644
index 0000000..c8a05a9
--- /dev/null
+++ b/sw/qa/extras/odfimport/data/tdf107392.odt
Binary files differ
diff --git a/sw/qa/extras/odfimport/odfimport.cxx b/sw/qa/extras/odfimport/odfimport.cxx
index 9572e7f..ce4a69b 100644
--- a/sw/qa/extras/odfimport/odfimport.cxx
+++ b/sw/qa/extras/odfimport/odfimport.cxx
@@ -772,5 +772,17 @@ DECLARE_ODFIMPORT_TEST(testTdf101729, "tdf101729.odt")
CPPUNIT_ASSERT( x < l + 3 * w / 4);
}
DECLARE_ODFIMPORT_TEST(testTdf107392, "tdf107392.odt")
{
// Shapes from bottom to top were Frame, SVG, Bitmap, i.e. in the order as
// they appeared in the document, not according to their requested z-index,
// as sorting failed.
// So instead of 0, 1, 2 these were 2, 0, 1.
CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(getShapeByName("Bitmap"), "ZOrder"));
CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getProperty<sal_Int32>(getShapeByName("Frame"), "ZOrder"));
CPPUNIT_ASSERT_EQUAL(sal_Int32(2), getProperty<sal_Int32>(getShapeByName("SVG"), "ZOrder"));
}
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/source/draw/shapeimport.cxx b/xmloff/source/draw/shapeimport.cxx
index 44c648f..4f55f1e 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 8f71968..19bdfbf 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 6fa574c..56bdd7f 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 20ee384..c61c597 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 8f19eda..2f82ea5 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:
