DOCX drawingML shape import: fix position when CustomShapeGeometry is set

DOCX drawingML shapes had wrong position if they had their
CustomShapeGeometry set (e.g. flipped). This wasn't a problem for VML
shapes, as there the shape knows its position, and position was always
set in oox as well, not in writerfilter. However, in case of WPS shapes,
oox created the shape, and previously writerfilter set the position
after-the-fact. This leads to incorrect results if CustomShapeGeometry
is involved. Fix this by passing the position from writerfilter to oox,
and call setPosition() after creation, but before CustomShapeGeometry is
set.

The other problem was that normally writerfilter learns the position of
the shape when relevant token in GraphicImport::lcl_sprm() arrives, but
this happens after OOXMLFastContextHandlerShape::sendShape() needs that
information. Work around this by accessing the PositionHandler directly.

Change-Id: Iced35dc9467ef77c41f1897f124729f686bd045e

12 files changed, 56 insertions, 1 deletions

diff --git a/include/oox/drawingml/shape.hxx b/include/oox/drawingml/shape.hxx
index 9c8faa9c95db..5fcb91c1d5a4 100644
--- a/include/oox/drawingml/shape.hxx
+++ b/include/oox/drawingml/shape.hxx
@@ -173,6 +173,8 @@ public:
     void                addExtDrawingRelId( const OUString &rRelId ) { maExtDrawings.push_back( rRelId ); }
     void                setLockedCanvas(bool bLockedCanvas);
     bool                getLockedCanvas();
+    void                setWps(bool bWps);
+    bool                getWps();
     const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> &
                         getDiagramDoms() { return maDiagramDoms; }
     void                setDiagramDoms(const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue>& rDiagramDoms) { maDiagramDoms = rDiagramDoms; }
@@ -272,6 +274,7 @@ private:
                                                          // we need separate flag because we don't want
                                                          // to propagate it when applying reference shape
     bool mbLockedCanvas; ///< Is this shape part of a locked canvas?
+    bool mbWps; ///< Is this a wps shape?
 
     com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> maDiagramDoms;
 };
diff --git a/offapi/com/sun/star/xml/sax/XFastShapeContextHandler.idl b/offapi/com/sun/star/xml/sax/XFastShapeContextHandler.idl
index bd58164b6f0e..5ec7c88de1d6 100644
--- a/offapi/com/sun/star/xml/sax/XFastShapeContextHandler.idl
+++ b/offapi/com/sun/star/xml/sax/XFastShapeContextHandler.idl
@@ -42,6 +42,7 @@ interface XFastShapeContextHandler: com::sun::star::xml::sax::XFastContextHandle
     [attribute] com::sun::star::io::XInputStream InputStream;
     [attribute] string RelationFragmentPath;
     [attribute] long StartToken;
+    [attribute] com::sun::star::awt::Point Position;
   };
 
 
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index 54db7bb73fb8..f93a4c4e980c 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -96,6 +96,7 @@ Shape::Shape( const sal_Char* pServiceName )
 , mbHidden( false )
 , mbHiddenMasterShape( false )
 , mbLockedCanvas( false )
+, mbWps( false )
 , maDiagramDoms( 0 )
 {
     if ( pServiceName )
@@ -132,6 +133,7 @@ Shape::Shape( const ShapePtr& pSourceShape )
 , mbHidden( pSourceShape->mbHidden )
 , mbHiddenMasterShape( pSourceShape->mbHiddenMasterShape )
 , mbLockedCanvas( pSourceShape->mbLockedCanvas )
+, mbWps( pSourceShape->mbWps )
 , maDiagramDoms( pSourceShape->maDiagramDoms )
 {}
 
@@ -259,6 +261,16 @@ bool Shape::getLockedCanvas()
     return mbLockedCanvas;
 }
 
+void Shape::setWps(bool bWps)
+{
+    mbWps = bWps;
+}
+
+bool Shape::getWps()
+{
+    return mbWps;
+}
+
 void Shape::applyShapeReference( const Shape& rReferencedShape, bool bUseText )
 {
     SAL_INFO("oox", OSL_THIS_FUNC << "apply shape reference: " << rReferencedShape.msId << " to shape id: " << msId);
@@ -607,6 +619,12 @@ Reference< XShape > Shape::createAndInsert(
             }
         }
 
+        // These can have a custom geometry, so position should be set here,
+        // after creation but before custom shape handling, using the position
+        // we got from the caller.
+        if (mbWps)
+            mxShape->setPosition(maPosition);
+
         if( bIsCustomShape )
         {
             if ( mbFlipH )
diff --git a/oox/source/shape/ShapeContextHandler.cxx b/oox/source/shape/ShapeContextHandler.cxx
index 4af1ab04ff2f..bcf43c85542d 100644
--- a/oox/source/shape/ShapeContextHandler.cxx
+++ b/oox/source/shape/ShapeContextHandler.cxx
@@ -426,6 +426,7 @@ ShapeContextHandler::getShape() throw (uno::RuntimeException)
             if (pShape)
             {
                 basegfx::B2DHomMatrix aMatrix;
+                pShape->setPosition(maPosition);
                 pShape->addShape(*mxFilterBase, mpThemePtr.get(), xShapes, aMatrix, pShape->getFillProperties());
                 xResult = pShape->getXShape();
                 mxWpsContext.clear();
@@ -512,6 +513,16 @@ void SAL_CALL ShapeContextHandler::setStartToken( ::sal_Int32 _starttoken ) thro
 
 }
 
+awt::Point SAL_CALL ShapeContextHandler::getPosition() throw (uno::RuntimeException)
+{
+    return maPosition;
+}
+
+void SAL_CALL ShapeContextHandler::setPosition(const awt::Point& rPosition) throw (uno::RuntimeException)
+{
+    maPosition = rPosition;
+}
+
 OUString ShapeContextHandler::getImplementationName()
     throw (css::uno::RuntimeException)
 {
diff --git a/oox/source/shape/ShapeContextHandler.hxx b/oox/source/shape/ShapeContextHandler.hxx
index 11fdc0b5d6bb..0c213131fee4 100644
--- a/oox/source/shape/ShapeContextHandler.hxx
+++ b/oox/source/shape/ShapeContextHandler.hxx
@@ -134,11 +134,15 @@ public:
     virtual ::sal_Int32 SAL_CALL getStartToken() throw (::com::sun::star::uno::RuntimeException);
     virtual void SAL_CALL setStartToken( ::sal_Int32 _starttoken ) throw (::com::sun::star::uno::RuntimeException);
 
+    virtual css::awt::Point SAL_CALL getPosition() throw (css::uno::RuntimeException);
+    virtual void SAL_CALL setPosition(const css::awt::Point& rPosition) throw (css::uno::RuntimeException);
+
 private:
     ShapeContextHandler(ShapeContextHandler &); // not defined
     void operator =(ShapeContextHandler &); // not defined
 
     ::sal_uInt32 mnStartToken;
+    css::awt::Point maPosition;
 
     css::uno::Reference< css::uno::XComponentContext > m_xContext;
     drawingml::ShapePtr mpShape;
diff --git a/oox/source/shape/WpsContext.cxx b/oox/source/shape/WpsContext.cxx
index 8f4987ab09a0..92c1a088f237 100644
--- a/oox/source/shape/WpsContext.cxx
+++ b/oox/source/shape/WpsContext.cxx
@@ -19,6 +19,7 @@ WpsContext::WpsContext(ContextHandler2Helper& rParent)
 : ContextHandler2(rParent)
 {
     mpShape.reset(new oox::drawingml::Shape("com.sun.star.drawing.CustomShape"));
+    mpShape->setWps(true);
 }
 
 WpsContext::~WpsContext()
diff --git a/sw/qa/extras/ooxmlimport/data/line-wps-only.docx b/sw/qa/extras/ooxmlimport/data/line-wps-only.docx
new file mode 100644
index 000000000000..465f4bd6ed2c
--- /dev/null
+++ b/sw/qa/extras/ooxmlimport/data/line-wps-only.docx
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index d8ca06f9cafa..90956106eb1f 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -1526,6 +1526,13 @@ DECLARE_OOXMLIMPORT_TEST(testWpsOnly, "wps-only.docx")
     CPPUNIT_ASSERT_EQUAL(false, bool(getProperty<sal_Bool>(getShape(2), "Opaque")));
 }
 
+DECLARE_OOXMLIMPORT_TEST(lineWpsOnly, "line-wps-only.docx")
+{
+    uno::Reference<drawing::XShape> xShape = getShape(1);
+    // Check position, it was -7223 as it was set after the CustomShapeGeometry property.
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(210), xShape->getPosition().X);
+}
+
 DECLARE_OOXMLIMPORT_TEST(testFdo70457, "fdo70457.docx")
 {
     // The document contains a rotated bitmap
diff --git a/writerfilter/source/dmapper/GraphicHelpers.cxx b/writerfilter/source/dmapper/GraphicHelpers.cxx
index d2e64d23e143..433e30e37eeb 100644
--- a/writerfilter/source/dmapper/GraphicHelpers.cxx
+++ b/writerfilter/source/dmapper/GraphicHelpers.cxx
@@ -165,6 +165,14 @@ void PositionHandler::setPositionOffset(const OUString & sText, bool vertical)
         savedPositionOffsetH = ConversionHelper::convertEMUToMM100( sText.toInt32());
 }
 
+int PositionHandler::getPositionOffset(bool vertical)
+{
+    if (vertical)
+        return savedPositionOffsetV;
+    else
+        return savedPositionOffsetH;
+}
+
 void PositionHandler::setAlignH(const OUString & sText)
 {
     if( sText == "left")
diff --git a/writerfilter/source/dmapper/GraphicHelpers.hxx b/writerfilter/source/dmapper/GraphicHelpers.hxx
index e3e4be66a2d6..97b2652bed52 100644
--- a/writerfilter/source/dmapper/GraphicHelpers.hxx
+++ b/writerfilter/source/dmapper/GraphicHelpers.hxx
@@ -37,6 +37,7 @@ public:
     PositionHandler( bool vertical );
     ~PositionHandler( );
     static void setPositionOffset(const OUString & sText, bool vertical);
+    static int getPositionOffset(bool vertical);
     static void setAlignH(const OUString & sText);
     static void setAlignV(const OUString & sText);
     sal_Int16 orientation() const;
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 7c3de0075137..8d0d0cb6d975 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -1000,7 +1000,6 @@ void GraphicImport::lcl_attribute(Id nName, Value & val)
                         // This needs to be AT_PARAGRAPH and not AT_CHARACTER, otherwise shape will move when the user inserts a new paragraph.
                         xShapeProps->setPropertyValue("AnchorType", uno::makeAny(text::TextContentAnchorType_AT_PARAGRAPH));
 
-                        m_xShape->setPosition(awt::Point(m_pImpl->nLeftPosition, m_pImpl->nTopPosition));
                         m_pImpl->applyMargins(xShapeProps);
                         bool bOpaque = m_pImpl->bOpaque && !m_pImpl->rDomainMapper.IsInHeaderFooter();
                         if (!bOpaque)
diff --git a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
index 9854d697ed4e..2b9be07b420e 100644
--- a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
+++ b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
@@ -2102,6 +2102,8 @@ void OOXMLFastContextHandlerShape::sendShape( Token_t Element )
 {
     if ( mrShapeContext.is() && !m_bShapeSent )
     {
+        awt::Point aPosition(writerfilter::dmapper::PositionHandler::getPositionOffset(false), writerfilter::dmapper::PositionHandler::getPositionOffset(true));
+        mrShapeContext->setPosition(aPosition);
         uno::Reference<drawing::XShape> xShape(mrShapeContext->getShape());
         if (xShape.is())
         {