[Libreoffice-commits] core.git: oox/qa oox/source

[Miklos Vajna (via logerrit)]

 oox/qa/unit/data/customshape-position.docx |binary
 oox/qa/unit/shape.cxx                      |   23 +++++++++++++++++++++++
 oox/source/shape/ShapeContextHandler.cxx   |    9 +++++++++
 3 files changed, 32 insertions(+)

New commits:
commit 80aba00e4945b106a276acf4ea28309b16e7c088
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Tue Nov 3 17:55:02 2020 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Tue Nov 3 21:30:48 2020 +0100

    tdf#128877 DOCX import: fix lost position of image cropped to shape
    
    Regression from commit f4ba484183a1e7b9824f10580d633466c266828f (ooxml
    import: supprt cropping to shape, 2019-05-13), which changed the type of
    a cropped-to-shape image from drawing.GraphicObjectShape to
    drawing.CustomShape.
    
    drawing.GraphicObjectShape worked because GraphicImport::lcl_attribute()
    in writerfilter/ had a check for drawing.GraphicObjectShape and did an
    explicit setPosition().
    
    Doing the same for bitmap-filled custom shapes would be an option, but
    it would be ugly: scaling/translation/rotation/mirroring can only work
    together if they are only applied once, and that should happen in oox/,
    that's why we already have a mechanism to send the position from
    writerfilter/ to oox/ for WPS shapes. (<a:xfrm> contains the size, but
    not the position of the shape, so oox/ in itself could not know the
    position.)
    
    Fix the problem by improving ShapeContextHandler instead the pass the
    position from writerfilter/ to oox/ for <pic:pic> as well, the same is
    done for <wps:wsp> already since commit
    6c4f737ec88a4f4dc5da8b2295ca5e7de2d4c24f (DOCX drawingML shape import:
    fix position when CustomShapeGeometry is set, 2013-11-21).
    
    Change-Id: I74a60136d0ca8383e58948711b47858823f42437
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105263
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    Tested-by: Jenkins

diff --git a/oox/qa/unit/data/customshape-position.docx b/oox/qa/unit/data/customshape-position.docx
new file mode 100644
index 000000000000..227928f201e6
Binary files /dev/null and b/oox/qa/unit/data/customshape-position.docx differ
diff --git a/oox/qa/unit/shape.cxx b/oox/qa/unit/shape.cxx
index 3dc173b1ed87..a57c779d00fd 100644
--- a/oox/qa/unit/shape.cxx
+++ b/oox/qa/unit/shape.cxx
@@ -12,6 +12,9 @@
 
 #include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
 #include <com/sun/star/frame/Desktop.hpp>
+#include <com/sun/star/beans/XPropertySet.hpp>
+
+#include <rtl/math.hxx>
 
 using namespace ::com::sun::star;
 
@@ -65,6 +68,26 @@ CPPUNIT_TEST_FIXTURE(OoxShapeTest, testMultipleGroupShapes)
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2), xDrawPage->getCount());
 }
 
+CPPUNIT_TEST_FIXTURE(OoxShapeTest, testCustomshapePosition)
+{
+    load("customshape-position.docx");
+
+    uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+                                                 uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY);
+
+    sal_Int32 nY{};
+    xShape->getPropertyValue("VertOrientPosition") >>= nY;
+    // <wp:posOffset>581025</wp:posOffset> in the document.
+    sal_Int32 nExpected = rtl::math::round(581025.0 / 360);
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 1614
+    // - Actual  : 0
+    // i.e. the position of the shape was lost on import due to the rounded corners.
+    CPPUNIT_ASSERT_EQUAL(nExpected, nY);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/oox/source/shape/ShapeContextHandler.cxx b/oox/source/shape/ShapeContextHandler.cxx
index c1cc28129eb0..2368de0a3629 100644
--- a/oox/source/shape/ShapeContextHandler.cxx
+++ b/oox/source/shape/ShapeContextHandler.cxx
@@ -504,6 +504,15 @@ ShapeContextHandler::getShape()
         else if (mpShape)
         {
             basegfx::B2DHomMatrix aTransformation;
+
+            if (maPosition.X != 0 || maPosition.Y != 0)
+            {
+                // We got a position from writerfilter/, store that in the shape, otherwise the
+                // position won't be set.
+                mpShape->setWps(true);
+                mpShape->setPosition(maPosition);
+            }
+
             mpShape->addShape(*mxFilterBase, mpThemePtr.get(), xShapes, aTransformation, mpShape->getFillProperties() );
             xResult.set(mpShape->getXShape());
             mxGraphicShapeContext.clear( );