svx: fix unexpected large bezier shapes when created via the UNO API

Regression from commit bc886f523872d4f9845c188c7d525d72a1a60946
(tdf#98584 Correct import draw:transform values skewY and matrix,
2020-01-02), then problem is was that now we handle the transform matrix
for path objects the way ODF likes it, but this breaks UNO API clients
with existing code.

In an attempt to make everyone happy, revert the default to ignore
scaling from the transform matrix, but add a way so that xmloff/ can opt
in to handle it. This way the ODF import result doesn't regression and
UNO API compatibility is not broken, either.

Change-Id: I8d525e56954bf68c42d3629eca87fe9b513de507
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111340
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

5 files changed, 116 insertions, 1 deletions

diff --git a/include/svx/svdopath.hxx b/include/svx/svdopath.hxx
index b934d74d3e46..be345a850b7d 100644
--- a/include/svx/svdopath.hxx
+++ b/include/svx/svdopath.hxx
@@ -49,6 +49,7 @@ private:
 
     basegfx::B2DPolyPolygon maPathPolygon;
     SdrObjKind                  meKind;
+    bool mbHandleScale = false;
 
     // for isolation of old Drag/Create code
     std::unique_ptr<ImpPathForDragAndCreate> mpDAC;
@@ -163,6 +164,12 @@ public:
     // If it's an SdrPathObj it will use the provided geometry information. The Polygon has
     // to use (0,0) as upper left and will be scaled to the given size in the matrix.
     virtual void TRSetBaseGeometry(const basegfx::B2DHomMatrix& rMatrix, const basegfx::B2DPolyPolygon& rPolyPolygon) override;
+
+    /**
+     * Scaling is ignored from the transform matrix by default, to not break compatibility. One can
+     * opt in to handle scaling if matching ODF behavior is more important.
+     */
+    void SetHandleScale(bool bHandleScale);
 };
 
 #endif // INCLUDED_SVX_SVDOPATH_HXX
diff --git a/svx/qa/unit/svdraw.cxx b/svx/qa/unit/svdraw.cxx
index c96ccbb7aa97..15c220d53e07 100644
--- a/svx/qa/unit/svdraw.cxx
+++ b/svx/qa/unit/svdraw.cxx
@@ -16,6 +16,10 @@
 #include <com/sun/star/drawing/XDrawPage.hpp>
 #include <com/sun/star/frame/Desktop.hpp>
 #include <com/sun/star/text/XTextRange.hpp>
+#include <com/sun/star/drawing/FillStyle.hpp>
+#include <com/sun/star/drawing/LineStyle.hpp>
+#include <com/sun/star/drawing/PolyPolygonBezierCoords.hpp>
+#include <com/sun/star/drawing/HomogenMatrix3.hpp>
 
 #include <drawinglayer/tools/primitive2dxmldump.hxx>
 #include <rtl/ustring.hxx>
@@ -105,6 +109,84 @@ CPPUNIT_TEST_FIXTURE(SvdrawTest, testSemiTransparentText)
     CPPUNIT_ASSERT_EQUAL(nTransparence,
                          static_cast<sal_Int16>(basegfx::fround(fTransparence * 100)));
 }
+
+CPPUNIT_TEST_FIXTURE(SvdrawTest, testHandlePathObjScale)
+{
+    // Given a path object:
+    getComponent() = loadFromDesktop("private:factory/sdraw");
+    uno::Reference<lang::XMultiServiceFactory> xFactory(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XShape> xShape(
+        xFactory->createInstance("com.sun.star.drawing.ClosedBezierShape"), uno::UNO_QUERY);
+
+    // When setting its scale by both using setSize() and scaling in a transform matrix:
+    // Set size and basic properties.
+    xShape->setPosition(awt::Point(2512, 6062));
+    xShape->setSize(awt::Size(112, 112));
+    uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
+    xShapeProps->setPropertyValue("FillStyle", uno::makeAny(drawing::FillStyle_SOLID));
+    xShapeProps->setPropertyValue("LineStyle", uno::makeAny(drawing::LineStyle_SOLID));
+    xShapeProps->setPropertyValue("FillColor", uno::makeAny(static_cast<sal_Int32>(0)));
+    // Add it to the draw page.
+    uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+                                                 uno::UNO_QUERY);
+    xDrawPage->add(xShape);
+    // Set polygon coordinates.
+    drawing::PolyPolygonBezierCoords aPolyPolygonBezierCoords;
+    aPolyPolygonBezierCoords.Coordinates = {
+        {
+            awt::Point(2624, 6118),
+            awt::Point(2624, 6087),
+            awt::Point(2599, 6062),
+            awt::Point(2568, 6062),
+            awt::Point(2537, 6062),
+            awt::Point(2512, 6087),
+            awt::Point(2512, 6118),
+            awt::Point(2512, 6149),
+            awt::Point(2537, 6175),
+            awt::Point(2568, 6174),
+            awt::Point(2599, 6174),
+            awt::Point(2625, 6149),
+            awt::Point(2624, 6118),
+        },
+    };
+    aPolyPolygonBezierCoords.Flags = {
+        {
+            drawing::PolygonFlags_NORMAL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_NORMAL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_NORMAL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_NORMAL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_CONTROL,
+            drawing::PolygonFlags_NORMAL,
+        },
+    };
+    xShapeProps->setPropertyValue("PolyPolygonBezier", uno::makeAny(aPolyPolygonBezierCoords));
+    drawing::HomogenMatrix3 aMatrix;
+    aMatrix.Line1.Column1 = 56;
+    aMatrix.Line2.Column1 = -97;
+    aMatrix.Line3.Column1 = 0;
+    aMatrix.Line1.Column2 = 97;
+    aMatrix.Line2.Column2 = 56;
+    aMatrix.Line3.Column2 = 0;
+    aMatrix.Line1.Column3 = 3317;
+    aMatrix.Line2.Column3 = 5583;
+    aMatrix.Line3.Column3 = 1;
+    xShapeProps->setPropertyValue("Transformation", uno::makeAny(aMatrix));
+
+    // Then make sure the scaling is only applied once:
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 113
+    // - Actual  : 12566
+    // i.e. the scaling was applied twice.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(113), xShape->getSize().Width);
+}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svx/source/svdraw/svdopath.cxx b/svx/source/svdraw/svdopath.cxx
index c3b1995e2b16..be0fbf786a11 100644
--- a/svx/source/svdraw/svdopath.cxx
+++ b/svx/source/svdraw/svdopath.cxx
@@ -2877,6 +2877,11 @@ bool SdrPathObj::TRGetBaseGeometry(basegfx::B2DHomMatrix& rMatrix, basegfx::B2DP
     return true;
 }
 
+void SdrPathObj::SetHandleScale(bool bHandleScale)
+{
+    mbHandleScale = bHandleScale;
+}
+
 // Sets the base geometry of the object using infos contained in the homogeneous 3x3 matrix.
 // If it's an SdrPathObj it will use the provided geometry information. The Polygon has
 // to use (0,0) as upper left and will be scaled to the given size in the matrix.
@@ -2929,7 +2934,7 @@ void SdrPathObj::TRSetBaseGeometry(const basegfx::B2DHomMatrix& rMatrix, const b
     // be applied to the polygon too, so aScale cannot be ignored while loading.
     // I use "maSnapRect.IsEmpty() && GetPathPoly().count()" to detect this case. Any better
     // idea? The behavior in other cases is the same as it was before this fix.
-    if (maSnapRect.IsEmpty() && GetPathPoly().count())
+    if (maSnapRect.IsEmpty() && GetPathPoly().count() && mbHandleScale)
     {
         // In case of a Writer document, the scaling factors were converted to twips. That is not
         // correct here, because width and height are already in the points coordinates and aScale
diff --git a/svx/source/unodraw/unoshape.cxx b/svx/source/unodraw/unoshape.cxx
index 965df6b63cbd..a84795b4b06b 100644
--- a/svx/source/unodraw/unoshape.cxx
+++ b/svx/source/unodraw/unoshape.cxx
@@ -85,6 +85,7 @@
 #include <vcl/wmf.hxx>
 #include <svx/sdtfsitm.hxx>
 #include <svx/svdoutl.hxx>
+#include <svx/svdopath.hxx>
 
 #include <memory>
 #include <vector>
@@ -1611,6 +1612,20 @@ void SvxShape::_setPropertyValue( const OUString& rPropertyName, const uno::Any&
         return;
     }
 
+    if (rPropertyName == "HandlePathObjScale")
+    {
+        auto pPathObj = dynamic_cast<SdrPathObj*>(GetSdrObject());
+        if (pPathObj)
+        {
+            bool bHandleScale{};
+            if (rVal >>= bHandleScale)
+            {
+                pPathObj->SetHandleScale(bHandleScale);
+            }
+        }
+        return;
+    }
+
     if (!pMap)
     {
         // reduce log noise by ignoring two properties that higher level code queries for on all objects
diff --git a/xmloff/source/draw/shapeimport.cxx b/xmloff/source/draw/shapeimport.cxx
index 69f97973913f..792daaa5db3f 100644
--- a/xmloff/source/draw/shapeimport.cxx
+++ b/xmloff/source/draw/shapeimport.cxx
@@ -471,6 +471,12 @@ void XMLShapeImportHelper::addShape( uno::Reference< drawing::XShape >& rShape,
     {
         // add new shape to parent
         rShapes->add( rShape );
+
+        uno::Reference<beans::XPropertySet> xPropertySet(rShape, uno::UNO_QUERY);
+        if (xPropertySet.is())
+        {
+            xPropertySet->setPropertyValue("HandlePathObjScale", uno::makeAny(true));
+        }
     }
 }