tdf#112450 correct points and size for polyline in VML import

The points in file source might have units. Decode was missing.

maWidth and maHeight are used in ShapeBase::convertAndInsert(), and
moCoordSize is used in PolyLineShape::implConvertAndInsert(). So
ShapeContext needs to provide both in case of importing a polyline.
That was missing.

Word writes the size into the coordsize attribute of the v:polyline
element. But from VML specification it need not exist, but bounding
rectangle of points has to be used. That is added too.

Unclosed polyline cannot be filled in LO and ODF, a fill is only
possible for a closed polygon. LO defines a sequence of points as
being closed, if first point == last point. The import is adapted
to that behavior.

Word allows an unclosed polyline to have filling. That cannot be
represented with a simple PolyPolygonShape but would need a
CustomShape. Because the user cannot yet edit points in a CustomShape
but can do that in a PolyPolygonShape, the v:polyline element is
not converted to a CustomShape on import and not to DML on export.
LO would first need an UI for editing points of a CustomShape.

Change-Id: I23d08fda2005f8b36488e1d9ba32e565d8a0f803
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121042
Tested-by: Jenkins
Reviewed-by: Regina Henschel <rb.henschel@t-online.de>

6 files changed, 157 insertions, 10 deletions

diff --git a/include/oox/vml/vmlformatting.hxx b/include/oox/vml/vmlformatting.hxx
index 4bdabbffb97b..78a2b7f27578 100644
--- a/include/oox/vml/vmlformatting.hxx
+++ b/include/oox/vml/vmlformatting.hxx
@@ -130,6 +130,18 @@ namespace ConversionHelper
                             bool bPixelX,
                             bool bDefaultAsPixel );
 
+/** Converts the passed VML measure string to Twip.
+
+        @param rGraphicHelper  See above.
+        @param rValue  See above.
+        @param nRefValue  See above.
+        @param bPixelX  See above.
+        @param bDefaultAsPixel  See above.
+     */
+OOX_DLLPUBLIC sal_Int32 decodeMeasureToTwip(const GraphicHelper& rGraphicHelper,
+                                            const OUString& rValue, sal_Int32 nRefValue,
+                                            bool bPixelX, bool bDefaultAsPixel);
+
     /** Converts VML color attributes to a DrawingML color.
 
         @param roVmlColor  The VML string representation of the color. If
diff --git a/oox/qa/unit/data/tdf112450_vml_polyline.docx b/oox/qa/unit/data/tdf112450_vml_polyline.docx
new file mode 100644
index 000000000000..a31124ee77db
--- /dev/null
+++ b/oox/qa/unit/data/tdf112450_vml_polyline.docx
diff --git a/oox/qa/unit/vml.cxx b/oox/qa/unit/vml.cxx
index d3b84712a459..6d8a5cf93912 100644
--- a/oox/qa/unit/vml.cxx
+++ b/oox/qa/unit/vml.cxx
@@ -16,6 +16,9 @@
 
 #include <com/sun/star/beans/XPropertySet.hpp>
 #include <com/sun/star/drawing/LineStyle.hpp>
+#include <com/sun/star/drawing/PointSequence.hpp>
+#include <com/sun/star/drawing/PointSequenceSequence.hpp>
+#include <com/sun/star/drawing/PolygonKind.hpp>
 #include <com/sun/star/drawing/PolyPolygonBezierCoords.hpp>
 #include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
 #include <com/sun/star/frame/Desktop.hpp>
@@ -59,6 +62,69 @@ void OoxVmlTest::load(std::u16string_view rFileName)
     mxComponent = loadFromDesktop(aURL);
 }
 
+CPPUNIT_TEST_FIXTURE(OoxVmlTest, tdf112450_vml_polyline)
+{
+    // Load a document with v:polyline shapes. Error was, that the size was set to zero and the
+    // points were zero because of missing decode from length with unit.
+    load(u"tdf112450_vml_polyline.docx");
+    uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+                                                 uno::UNO_QUERY);
+    {
+        // This tests a polyline shape which is not closed.
+        uno::Reference<drawing::XShape> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY);
+        uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
+        // Without fix in place, Geometry had 2 points, both 0|0.
+        drawing::PointSequenceSequence aGeometry;
+        xShapeProps->getPropertyValue("Geometry") >>= aGeometry;
+        drawing::PointSequence aPolygon = aGeometry[0];
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(6879), aPolygon[3].X, 1);
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(487), aPolygon[3].Y, 1);
+        // Without fix in place, width and height were zero
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(6879), xShape->getSize().Width, 1);
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(1926), xShape->getSize().Height, 1);
+        // After the fix the shape has still to be PolygonKind_PLIN
+        drawing::PolygonKind ePolygonKind;
+        xShapeProps->getPropertyValue("PolygonKind") >>= ePolygonKind;
+        CPPUNIT_ASSERT_EQUAL(drawing::PolygonKind_PLIN, ePolygonKind);
+    }
+    {
+        uno::Reference<drawing::XShape> xShape(xDrawPage->getByIndex(1), uno::UNO_QUERY);
+        uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
+        // Without fix in place, Geometry had 2 points, both 0|0.
+        drawing::PointSequenceSequence aGeometry;
+        xShapeProps->getPropertyValue("Geometry") >>= aGeometry;
+        drawing::PointSequence aPolygon = aGeometry[0];
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(5062), aPolygon[2].X, 1);
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(2247), aPolygon[2].Y, 1);
+        // Without fix in place, width and height were zero
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(6163), xShape->getSize().Width, 1);
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(2247), xShape->getSize().Height, 1);
+        // Without fix in place the shape was not closed, it had PolygonKind_PLIN
+        drawing::PolygonKind ePolygonKind;
+        xShapeProps->getPropertyValue("PolygonKind") >>= ePolygonKind;
+        CPPUNIT_ASSERT_EQUAL(drawing::PolygonKind_POLY, ePolygonKind);
+    }
+    {
+        // This tests a filled shape where v:polyline does not have attribute coordsize
+        uno::Reference<drawing::XShape> xShape(xDrawPage->getByIndex(2), uno::UNO_QUERY);
+        uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
+        // Without fix in place, Geometry had 2 points, both 0|0.
+        drawing::PointSequenceSequence aGeometry;
+        xShapeProps->getPropertyValue("Geometry") >>= aGeometry;
+        drawing::PointSequence aPolygon = aGeometry[0];
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(2095), aPolygon[3].X, 1);
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(608), aPolygon[3].Y, 1);
+        // Without fix in place, width and height were zero
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(5634), xShape->getSize().Width, 1);
+        CPPUNIT_ASSERT_DOUBLES_EQUAL(sal_Int32(2485), xShape->getSize().Height, 1);
+        // Without fix in place the shape was not closed, it had PolygonKind_PLIN
+        drawing::PolygonKind ePolygonKind;
+        xShapeProps->getPropertyValue("PolygonKind") >>= ePolygonKind;
+        CPPUNIT_ASSERT_EQUAL(drawing::PolygonKind_POLY, ePolygonKind);
+    }
+}
+
 CPPUNIT_TEST_FIXTURE(OoxVmlTest, tdf137314_vml_rotation_unit_fd)
 {
     // Load a document with a 30deg rotated arc on a drawing canvas. Rotation is given
diff --git a/oox/source/vml/vmlformatting.cxx b/oox/source/vml/vmlformatting.cxx
index ef7e692838fe..94f87a772c00 100644
--- a/oox/source/vml/vmlformatting.cxx
+++ b/oox/source/vml/vmlformatting.cxx
@@ -210,6 +210,15 @@ sal_Int32 ConversionHelper::decodeMeasureToHmm( const GraphicHelper& rGraphicHel
     return ::oox::drawingml::convertEmuToHmm( decodeMeasureToEmu( rGraphicHelper, rValue, nRefValue, bPixelX, bDefaultAsPixel ) );
 }
 
+sal_Int32 ConversionHelper::decodeMeasureToTwip(const GraphicHelper& rGraphicHelper,
+                                                const OUString& rValue, sal_Int32 nRefValue,
+                                                bool bPixelX, bool bDefaultAsPixel)
+{
+    return ::o3tl::convert(
+        decodeMeasureToEmu(rGraphicHelper, rValue, nRefValue, bPixelX, bDefaultAsPixel),
+        o3tl::Length::emu, o3tl::Length::twip);
+}
+
 Color ConversionHelper::decodeColor( const GraphicHelper& rGraphicHelper,
         const OptValue< OUString >& roVmlColor, const OptValue< double >& roVmlOpacity,
         ::Color nDefaultRgb, ::Color nPrimaryRgb )
diff --git a/oox/source/vml/vmlshape.cxx b/oox/source/vml/vmlshape.cxx
index 2e2abfd5c32b..4d99a0014e88 100644
--- a/oox/source/vml/vmlshape.cxx
+++ b/oox/source/vml/vmlshape.cxx
@@ -1036,14 +1036,27 @@ PolyLineShape::PolyLineShape( Drawing& rDrawing ) :
 
 Reference< XShape > PolyLineShape::implConvertAndInsert( const Reference< XShapes >& rxShapes, const awt::Rectangle& rShapeRect ) const
 {
-    Reference< XShape > xShape = SimpleShape::implConvertAndInsert( rxShapes, rShapeRect );
-    // polygon path
+    ::std::vector<awt::Point> aAbsPoints;
     awt::Rectangle aCoordSys = getCoordSystem();
-    if( !maShapeModel.maPoints.empty() && (aCoordSys.Width > 0) && (aCoordSys.Height > 0) )
+    if (!maShapeModel.maPoints.empty() && (aCoordSys.Width > 0) && (aCoordSys.Height > 0))
     {
-        ::std::vector< awt::Point > aAbsPoints;
         for (auto const& point : maShapeModel.maPoints)
-            aAbsPoints.push_back( lclGetAbsPoint( point, rShapeRect, aCoordSys ) );
+            aAbsPoints.push_back(lclGetAbsPoint(point, rShapeRect, aCoordSys));
+        // A polyline cannot be filled but only a polygon. We treat first point == last point as
+        // indicator for being closed. In that case we force to type PolyPolygonShape.
+        if (aAbsPoints.size() > 2 && aAbsPoints.front().X == aAbsPoints.back().X
+            && aAbsPoints.front().Y == aAbsPoints.back().Y)
+        {
+            const_cast<PolyLineShape*>(this)->setService("com.sun.star.drawing.PolyPolygonShape");
+        }
+    }
+
+    Reference<XShape> xShape = SimpleShape::implConvertAndInsert(rxShapes, rShapeRect);
+
+    // polygon path
+
+    if (!aAbsPoints.empty())
+    {
         PointSequenceSequence aPointSeq( 1 );
         aPointSeq[ 0 ] = ContainerHelper::vectorToSequence( aAbsPoints );
         PropertySet aPropSet( xShape );
diff --git a/oox/source/vml/vmlshapecontext.cxx b/oox/source/vml/vmlshapecontext.cxx
index 82ca210e7588..d6051356ff14 100644
--- a/oox/source/vml/vmlshapecontext.cxx
+++ b/oox/source/vml/vmlshapecontext.cxx
@@ -562,16 +562,63 @@ ContextHandlerRef ShapeContext::onCreateContext( sal_Int32 nElement, const Attri
     return ShapeTypeContext::onCreateContext( nElement, rAttribs );
 }
 
-void ShapeContext::setPoints( const OUString& rPoints )
+void ShapeContext::setPoints(const OUString& rPoints)
 {
     mrShapeModel.maPoints.clear();
     sal_Int32 nIndex = 0;
 
-    while( nIndex >= 0 )
+    while (nIndex >= 0)
+    {
+        sal_Int32 nX = ConversionHelper::decodeMeasureToTwip(
+            mrShape.getDrawing().getFilter().getGraphicHelper(), rPoints.getToken(0, ',', nIndex),
+            0, true, true);
+        sal_Int32 nY = ConversionHelper::decodeMeasureToTwip(
+            mrShape.getDrawing().getFilter().getGraphicHelper(), rPoints.getToken(0, ',', nIndex),
+            0, false, true);
+        mrShapeModel.maPoints.emplace_back(nX, nY);
+    }
+    // VML polyline has no size in its style attribute. Word writes the size to attribute
+    // coordsize with values in twip but without unit. For others we get size from points.
+    if (mrShape.getTypeModel().maWidth.isEmpty() && mrShape.getTypeModel().maHeight.isEmpty())
     {
-        sal_Int32 nX = rPoints.getToken( 0, ',', nIndex ).toInt32();
-        sal_Int32 nY = rPoints.getToken( 0, ',', nIndex ).toInt32();
-        mrShapeModel.maPoints.emplace_back( nX, nY );
+        if (mrShape.getTypeModel().moCoordSize.has())
+        {
+            double fWidth = mrShape.getTypeModel().moCoordSize.get().first;
+            fWidth = o3tl::convert(fWidth, o3tl::Length::twip, o3tl::Length::pt);
+            double fHeight = mrShape.getTypeModel().moCoordSize.get().second;
+            fHeight = o3tl::convert(fHeight, o3tl::Length::twip, o3tl::Length::pt);
+            mrShape.getTypeModel().maWidth = OUString::number(fWidth) + "pt";
+            mrShape.getTypeModel().maHeight = OUString::number(fHeight) + "pt";
+        }
+        else if (mrShapeModel.maPoints.size())
+        {
+            double fMinX = mrShapeModel.maPoints[0].X;
+            double fMaxX = mrShapeModel.maPoints[0].X;
+            double fMinY = mrShapeModel.maPoints[0].Y;
+            double fMaxY = mrShapeModel.maPoints[0].Y;
+            for (const auto& rPoint : mrShapeModel.maPoints)
+            {
+                if (rPoint.X < fMinX)
+                    fMinX = rPoint.X;
+                else if (rPoint.X > fMaxX)
+                    fMaxX = rPoint.X;
+                if (rPoint.Y < fMinY)
+                    fMinY = rPoint.Y;
+                else if (rPoint.Y > fMaxY)
+                    fMaxY = rPoint.Y;
+            }
+            mrShape.getTypeModel().maWidth
+                = OUString::number(
+                      o3tl::convert(fMaxX - fMinX, o3tl::Length::twip, o3tl::Length::pt))
+                  + "pt";
+            mrShape.getTypeModel().maHeight
+                = OUString::number(
+                      o3tl::convert(fMaxY - fMinY, o3tl::Length::twip, o3tl::Length::pt))
+                  + "pt";
+            // Set moCoordSize, otherwise default (1000,1000) is used.
+            mrShape.getTypeModel().moCoordSize.set(
+                Int32Pair(basegfx::fround(fMaxX - fMinX), basegfx::fround(fMaxY - fMinY)));
+        }
     }
 }