diff options
author | Miklos Vajna <vmiklos@collabora.co.uk> | 2017-04-26 17:52:27 +0200 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.co.uk> | 2017-04-26 18:35:56 +0200 |
commit | 1b471124df251011b0053900cb82ceb0f3d8be86 (patch) | |
tree | d2a6a2abc6c6d57b26b43d74a88ef1dd15293ec3 | |
parent | ad6515f48ced36ab9d10840aa60670fc41d25c6b (diff) |
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>
-rw-r--r-- | include/xmloff/shapeimport.hxx | 3 | ||||
-rw-r--r-- | include/xmloff/xmlmultiimagehelper.hxx | 2 | ||||
-rw-r--r-- | sw/qa/extras/odfimport/data/tdf107392.odt | bin | 0 -> 43338 bytes | |||
-rw-r--r-- | sw/qa/extras/odfimport/odfimport.cxx | 12 | ||||
-rw-r--r-- | xmloff/source/draw/shapeimport.cxx | 40 | ||||
-rw-r--r-- | xmloff/source/draw/ximpshap.cxx | 2 | ||||
-rw-r--r-- | xmloff/source/draw/ximpshap.hxx | 2 | ||||
-rw-r--r-- | xmloff/source/text/XMLTextFrameContext.cxx | 7 | ||||
-rw-r--r-- | xmloff/source/text/XMLTextFrameContext.hxx | 2 |
9 files changed, 62 insertions, 8 deletions
diff --git a/include/xmloff/shapeimport.hxx b/include/xmloff/shapeimport.hxx index 4936b36a7068..efc00a289355 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 391f8a67a6a4..4a14d1ac4a77 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 Binary files differnew file mode 100644 index 000000000000..c8a05a9eef94 --- /dev/null +++ b/sw/qa/extras/odfimport/data/tdf107392.odt diff --git a/sw/qa/extras/odfimport/odfimport.cxx b/sw/qa/extras/odfimport/odfimport.cxx index 9572e7f4dba4..ce4a69b7b369 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 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: |