[Libreoffice-commits] core.git: svx/CppunitTest_svx_unit.mk svx/qa svx/source xmloff/source
Regina Henschel (via logerrit)
logerrit at kemper.freedesktop.org
Fri Jan 17 17:04:12 UTC 2020
svx/CppunitTest_svx_unit.mk | 1
svx/qa/unit/classicshapes.cxx | 189 ++++++++++++++++++++++++++
svx/qa/unit/data/tdf98583_ShearHorizontal.odp |binary
svx/qa/unit/data/tdf98584_ShearVertical.odg |binary
svx/source/svdraw/svdopath.cxx | 28 +++
xmloff/source/draw/xexptran.cxx | 10 +
xmloff/source/draw/ximpshap.cxx | 53 +++++--
7 files changed, 260 insertions(+), 21 deletions(-)
New commits:
commit bc886f523872d4f9845c188c7d525d72a1a60946
Author: Regina Henschel <rb.henschel at t-online.de>
AuthorDate: Thu Jan 2 17:37:32 2020 +0100
Commit: Regina Henschel <rb.henschel at t-online.de>
CommitDate: Fri Jan 17 18:03:38 2020 +0100
tdf#98584 Correct import draw:transform values skewY and matrix
Covers tdf#98583 and tdf#98565 too.
TRBaseGeomety (API) uses for skewX the same angle orientation as
written to file. But that results in mathematically wrong matrices.
Change sign where needed.
Vertical shearing is converted and written to file by LO as sequence
rotation * shear horizontal * scale. Same should happen on reading.
Because LO does not write skewY itself, I have used the angle
orientation, that was used in OOo1.1.5 and that is used in
Scribus 1.5.4.
Import generates a transformation matrix from the draw:transform
attribute. That is a mathematically correct matrix. It is applied
to the shape via TRSetBaseGeometry. But that uses a wrong sign in
the shear angle. Therefore conversion of mathematical matrix to
TRBaseGeometry matrix is needed.
The draw:transform attribute can generate a scaling, which needs to
be applied on top of the scaling made from svg:width and svg:height.
Such happens on import of skewY() and might happen with matrix().
SdrPathObject puts scaling form svg:width and svg:height directly
into the coordinates of its points. It had ignored any additional
scaling. I have add a part to detect and apply it.
Change-Id: I7636b9feec432cf403e7c6ef8dbd6a769793d144
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86244
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
Reviewed-by: Regina Henschel <rb.henschel at t-online.de>
diff --git a/svx/CppunitTest_svx_unit.mk b/svx/CppunitTest_svx_unit.mk
index 8514f438c8f0..92feb45d6578 100644
--- a/svx/CppunitTest_svx_unit.mk
+++ b/svx/CppunitTest_svx_unit.mk
@@ -25,6 +25,7 @@ $(eval $(call gb_CppunitTest_set_include,svx_unit,\
$(eval $(call gb_CppunitTest_add_exception_objects,svx_unit, \
svx/qa/unit/svdraw/test_SdrTextObject \
svx/qa/unit/customshapes \
+ svx/qa/unit/classicshapes \
svx/qa/unit/svdraw \
svx/qa/unit/unodraw \
svx/qa/unit/xoutdev \
diff --git a/svx/qa/unit/classicshapes.cxx b/svx/qa/unit/classicshapes.cxx
new file mode 100755
index 000000000000..b03210d12c20
--- /dev/null
+++ b/svx/qa/unit/classicshapes.cxx
@@ -0,0 +1,189 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <test/bootstrapfixture.hxx>
+#include <unotest/macros_test.hxx>
+#include <rtl/ustring.hxx>
+#include <editeng/unoprnms.hxx>
+
+#include <cppunit/TestAssert.h>
+
+#include <com/sun/star/beans/XPropertySet.hpp>
+#include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
+#include <com/sun/star/drawing/XDrawPage.hpp>
+#include <com/sun/star/awt/Rectangle.hpp>
+#include <com/sun/star/frame/Desktop.hpp>
+
+using namespace ::com::sun::star;
+
+namespace
+{
+const OUString sDataDirectory("svx/qa/unit/data/");
+
+/// Tests not about special features of custom shapes, but about shapes in general.
+class ClassicshapesTest : public test::BootstrapFixture, public unotest::MacrosTest
+{
+protected:
+ uno::Reference<lang::XComponent> mxComponent;
+ uno::Reference<drawing::XShape> getShape(sal_uInt8 nShapeIndex, sal_uInt8 nPageIndex);
+
+public:
+ virtual void setUp() override
+ {
+ test::BootstrapFixture::setUp();
+ mxDesktop.set(frame::Desktop::create(m_xContext));
+ }
+
+ virtual void tearDown() override
+ {
+ if (mxComponent.is())
+ {
+ mxComponent->dispose();
+ }
+ test::BootstrapFixture::tearDown();
+ }
+};
+
+uno::Reference<drawing::XShape> ClassicshapesTest::getShape(sal_uInt8 nShapeIndex,
+ sal_uInt8 nPageIndex)
+{
+ uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(mxComponent,
+ uno::UNO_QUERY_THROW);
+ CPPUNIT_ASSERT_MESSAGE("Could not get XDrawPagesSupplier", xDrawPagesSupplier.is());
+ uno::Reference<drawing::XDrawPages> xDrawPages(xDrawPagesSupplier->getDrawPages());
+ uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPages->getByIndex(nPageIndex),
+ uno::UNO_QUERY_THROW);
+ CPPUNIT_ASSERT_MESSAGE("Could not get xDrawPage", xDrawPage.is());
+ uno::Reference<drawing::XShape> xShape(xDrawPage->getByIndex(nShapeIndex), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_MESSAGE("Could not get xShape", xShape.is());
+ return xShape;
+}
+
+CPPUNIT_TEST_FIXTURE(ClassicshapesTest, testTdf98584ShearVertical)
+{
+ // The document contains draw:rect, draw:polygon and draw:path objects.
+ // They are vertical sheared by skewY(-0.927295218002) or by matrix(1 2 0 1 1cm 1cm).
+ // Notice, skewY and matrix are interpreted on file open, but not written on file save.
+ // They are converted to rotate * shear horizontal * scale.
+ // Besides using a wrong sign in shear angle, error was, that TRSetGeometry of SdrPathObj did
+ // not consider the additional scaling (tdf#98565).
+ const OUString sFileName("tdf98584_ShearVertical.odg");
+ const OUString sURL(m_directories.getURLFromSrc(sDataDirectory) + sFileName);
+ mxComponent = loadFromDesktop(sURL, "com.sun.star.comp.drawing.DrawingDocument");
+ CPPUNIT_ASSERT_MESSAGE("Could not load document", mxComponent.is());
+
+ OUString sErrors; // sErrors collects the errors and should be empty in case all is OK.
+ // All tests have a small tolerance for to avoid failing because of rounding errors.
+
+ // Tests skewY
+ sal_Int32 nShearE = -5313; // expected angle for horizontal shear
+ sal_Int32 nRotE = 30687; // = -5313 expected angle for generated rotation
+ // expected width of shape, should not change on vertical shearing
+ sal_Int32 nWidthE = 5001;
+
+ for (sal_uInt8 nPageIndex = 0; nPageIndex < 3; ++nPageIndex)
+ {
+ awt::Rectangle aFrameRect;
+ uno::Reference<drawing::XShape> xShape(getShape(0, nPageIndex));
+ uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
+ CPPUNIT_ASSERT_MESSAGE("Could not get the shape properties", xShapeProps.is());
+ xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_FRAMERECT) >>= aFrameRect;
+ const sal_Int32 nWidthA(aFrameRect.Width);
+ if (abs(nWidthE - nWidthA) > 2)
+ sErrors += "skewY page " + OUString::number(nPageIndex) + " width expected "
+ + OUString::number(nWidthE) + ", actual " + OUString::number(nWidthA) + "\n";
+ sal_Int32 nShearA(0);
+ xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_SHEARANGLE) >>= nShearA;
+ if (abs(nShearE - nShearA) > 2)
+ sErrors += "skewY page" + OUString::number(nPageIndex) + " shear angle expected "
+ + OUString::number(nShearE) + ", actual " + OUString::number(nShearA) + "\n";
+ sal_Int32 nRotA(0);
+ xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_ROTATEANGLE) >>= nRotA;
+ if (abs(nRotE - nRotA) > 2)
+ sErrors += "skewY page" + OUString::number(nPageIndex) + " rotate angle expected "
+ + OUString::number(nRotE) + ", actual " + OUString::number(nRotA) + "\n";
+ }
+
+ // Tests matrix
+ nShearE = -6343;
+ nRotE = 29657;
+ nWidthE = 5001;
+
+ for (sal_uInt8 nPageIndex = 3; nPageIndex < 6; ++nPageIndex)
+ {
+ awt::Rectangle aFrameRect;
+ uno::Reference<drawing::XShape> xShape(getShape(0, nPageIndex));
+ uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
+ CPPUNIT_ASSERT_MESSAGE("Could not get the shape properties", xShapeProps.is());
+ xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_FRAMERECT) >>= aFrameRect;
+ const sal_Int32 nWidthA(aFrameRect.Width);
+ if (abs(nWidthE - nWidthA) > 2)
+ sErrors += "matrix page " + OUString::number(nPageIndex) + " width expected "
+ + OUString::number(nWidthE) + ", actual " + OUString::number(nWidthA) + "\n";
+ sal_Int32 nShearA(0);
+ xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_SHEARANGLE) >>= nShearA;
+ if (abs(nShearE - nShearA) > 2)
+ sErrors += "matrix page" + OUString::number(nPageIndex) + " shear angle expected "
+ + OUString::number(nShearE) + ", actual " + OUString::number(nShearA) + "\n";
+ sal_Int32 nRotA(0);
+ xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_ROTATEANGLE) >>= nRotA;
+ if (abs(nRotE - nRotA) > 2)
+ sErrors += "matrix page" + OUString::number(nPageIndex) + " rotate angle expected "
+ + OUString::number(nRotE) + ", actual " + OUString::number(nRotA) + "\n";
+ }
+
+ CPPUNIT_ASSERT_EQUAL(OUString(), sErrors);
+}
+
+CPPUNIT_TEST_FIXTURE(ClassicshapesTest, testTdf98583ShearHorizontal)
+{
+ // The document contains rectangles with LT 3000,5000 and RB 5000,9000.
+ // skewX (-0.78539816339744830961) = skewX(-45deg) is applied on the first page
+ // matrix(1 0 1 1 0cm 0cm) on the second page. Both should result in a parallelogram with
+ // LT 8000,5000 and RB 14000, 9000, which means width 6001, height 4001.
+ // Error was, that not the mathematical matrix was used, but the API matrix, which has
+ // wrong sign in shear angle.
+ const OUString sFileName("tdf98583_ShearHorizontal.odp");
+ const OUString sURL(m_directories.getURLFromSrc(sDataDirectory) + sFileName);
+ mxComponent = loadFromDesktop(sURL, "com.sun.star.comp.presentation.PresentationDocument");
+ CPPUNIT_ASSERT_MESSAGE("Could not load document", mxComponent.is());
+
+ OUString sErrors; // sErrors collects the errors and should be empty in case all is OK.
+ // All tests have a small tolerance for to avoid failing because of rounding errors.
+ const sal_Int32 nLeftE(8000); // expected values
+ const sal_Int32 nTopE(5000);
+ const sal_Int32 nWidthE(6001);
+ const sal_Int32 nHeightE(4001);
+ for (sal_uInt8 nPageIndex = 0; nPageIndex < 2; ++nPageIndex)
+ {
+ awt::Rectangle aFrameRect;
+ uno::Reference<drawing::XShape> xShape(getShape(0, nPageIndex));
+ uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
+ CPPUNIT_ASSERT_MESSAGE("Could not get the shape properties", xShapeProps.is());
+ xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_FRAMERECT) >>= aFrameRect;
+ const sal_Int32 nLeftA(aFrameRect.X);
+ const sal_Int32 nTopA(aFrameRect.Y);
+ const sal_Int32 nWidthA(aFrameRect.Width);
+ const sal_Int32 nHeightA(aFrameRect.Height);
+ if (abs(nLeftE - nLeftA) > 2 || abs(nTopE - nTopA) > 2)
+ sErrors += "page " + OUString::number(nPageIndex) + " LT expected "
+ + OUString::number(nLeftE) + " | " + OUString::number(nTopE) + ", actual "
+ + OUString::number(nLeftA) + " | " + OUString::number(nTopA) + "\n";
+ if (abs(nWidthE - nWidthA) > 2 || abs(nHeightE - nHeightA) > 2)
+ sErrors += "page " + OUString::number(nPageIndex) + " width x height expected "
+ + OUString::number(nWidthE) + " x " + OUString::number(nHeightE)
+ + ", actual " + OUString::number(nWidthA) + " x "
+ + OUString::number(nHeightA) + "\n";
+ }
+
+ CPPUNIT_ASSERT_EQUAL(OUString(), sErrors);
+}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svx/qa/unit/data/tdf98583_ShearHorizontal.odp b/svx/qa/unit/data/tdf98583_ShearHorizontal.odp
new file mode 100644
index 000000000000..37719e825c82
Binary files /dev/null and b/svx/qa/unit/data/tdf98583_ShearHorizontal.odp differ
diff --git a/svx/qa/unit/data/tdf98584_ShearVertical.odg b/svx/qa/unit/data/tdf98584_ShearVertical.odg
new file mode 100644
index 000000000000..457521d50394
Binary files /dev/null and b/svx/qa/unit/data/tdf98584_ShearVertical.odg differ
diff --git a/svx/source/svdraw/svdopath.cxx b/svx/source/svdraw/svdopath.cxx
index b586d83b3079..a897887ec708 100644
--- a/svx/source/svdraw/svdopath.cxx
+++ b/svx/source/svdraw/svdopath.cxx
@@ -2929,13 +2929,31 @@ void SdrPathObj::TRSetBaseGeometry(const basegfx::B2DHomMatrix& rMatrix, const b
// #i75086#
// Given polygon is already scaled (for historical reasons), but not mirrored yet.
// Thus, when scale is negative in X or Y, apply the needed mirroring accordingly.
- if(basegfx::fTools::less(aScale.getX(), 0.0) || basegfx::fTools::less(aScale.getY(), 0.0))
- {
- aTransform.scale(
- basegfx::fTools::less(aScale.getX(), 0.0) ? -1.0 : 1.0,
- basegfx::fTools::less(aScale.getY(), 0.0) ? -1.0 : 1.0);
+ double fScaleX(basegfx::fTools::less(aScale.getX(), 0.0) ? -1.0 : 1.0);
+ double fScaleY(basegfx::fTools::less(aScale.getY(), 0.0) ? -1.0 : 1.0);
+
+ // tdf#98565, tdf#98584. While loading a shape, svg:width and svg:height is used to scale
+ // the polygon. But draw:transform might introduce additional scaling factors, which need to
+ // 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())
+ {
+ // 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
+ // is no length but only a factor here. Convert back.
+ if (getSdrModelFromSdrObject().IsWriter())
+ {
+ aScale.setX(aScale.getX() * 127.0 / 72.0);
+ aScale.setY(aScale.getY() * 127.0 / 72.0);
+ }
+ fScaleX *= fabs(aScale.getX());
+ fScaleY *= fabs(aScale.getY());
}
+ if (fScaleX != 1.0 || fScaleY != 1.0)
+ aTransform.scale(fScaleX, fScaleY);
+
if(!basegfx::fTools::equalZero(fShearX))
{
aTransform.shearX(tan(-atan(fShearX)));
diff --git a/xmloff/source/draw/xexptran.cxx b/xmloff/source/draw/xexptran.cxx
index eb7cb4a634d5..0137166871bc 100644
--- a/xmloff/source/draw/xexptran.cxx
+++ b/xmloff/source/draw/xexptran.cxx
@@ -510,12 +510,18 @@ void SdXMLImExTransform2D::GetFullTransform(::basegfx::B2DHomMatrix& rFullTrans)
}
case IMP_SDXMLEXP_TRANSOBJ2D_SKEWX :
{
- rFullTrans.shearX(tan(static_cast<ImpSdXMLExpTransObj2DSkewX*>(pObj)->mfSkewX));
+ // For to get a mathematical correct matrix from already existing documents,
+ // mirror the value here. ODF spec is unclear about direction.
+ rFullTrans.shearX(-tan(static_cast<ImpSdXMLExpTransObj2DSkewX*>(pObj)->mfSkewX));
break;
}
case IMP_SDXMLEXP_TRANSOBJ2D_SKEWY :
{
- rFullTrans.shearY(tan(static_cast<ImpSdXMLExpTransObj2DSkewY*>(pObj)->mfSkewY));
+ // LibreOffice does not write skewY, OOo neither. Such files are foreign documents
+ // or manually set transformations. OOo had used the value as -tan(value) before
+ // errors were introduced, Scribus 1.5.4 uses it as -tan(value) too, MS Office does
+ // not shear at all. ODF spec is unclear about direction.
+ rFullTrans.shearY(-tan(static_cast<ImpSdXMLExpTransObj2DSkewY*>(pObj)->mfSkewY));
break;
}
case IMP_SDXMLEXP_TRANSOBJ2D_MATRIX :
diff --git a/xmloff/source/draw/ximpshap.cxx b/xmloff/source/draw/ximpshap.cxx
index 8bee4b376ba1..3594d1605363 100644
--- a/xmloff/source/draw/ximpshap.cxx
+++ b/xmloff/source/draw/ximpshap.cxx
@@ -575,21 +575,38 @@ void SdXMLShapeContext::SetTransformation()
}
// now set transformation for this object
- drawing::HomogenMatrix3 aMatrix;
- aMatrix.Line1.Column1 = maUsedTransformation.get(0, 0);
- aMatrix.Line1.Column2 = maUsedTransformation.get(0, 1);
- aMatrix.Line1.Column3 = maUsedTransformation.get(0, 2);
+ // maUsedTransformtion contains the mathematical correct matrix, which if
+ // applied to a unit square would generate the transformed shape. But the property
+ // "Transformation" contains a matrix, which can be used in TRSetBaseGeometry
+ // and would be created by TRGetBaseGeometry. And those use a mathematically wrong
+ // sign for the shearing angle. So we need to adapt the matrix here.
+ basegfx::B2DTuple aScale;
+ basegfx::B2DTuple aTranslate;
+ double fRotate;
+ double fShearX;
+ maUsedTransformation.decompose(aScale, aTranslate, fRotate, fShearX);
+ basegfx::B2DHomMatrix aB2DHomMatrix;
+ aB2DHomMatrix = basegfx::utils::createScaleShearXRotateTranslateB2DHomMatrix(
+ aScale,
+ basegfx::fTools::equalZero(fShearX) ? 0.0 : -fShearX,
+ basegfx::fTools::equalZero(fRotate) ? 0.0 : fRotate,
+ aTranslate);
+ drawing::HomogenMatrix3 aUnoMatrix;
- aMatrix.Line2.Column1 = maUsedTransformation.get(1, 0);
- aMatrix.Line2.Column2 = maUsedTransformation.get(1, 1);
- aMatrix.Line2.Column3 = maUsedTransformation.get(1, 2);
+ aUnoMatrix.Line1.Column1 = aB2DHomMatrix.get(0, 0);
+ aUnoMatrix.Line1.Column2 = aB2DHomMatrix.get(0, 1);
+ aUnoMatrix.Line1.Column3 = aB2DHomMatrix.get(0, 2);
- aMatrix.Line3.Column1 = maUsedTransformation.get(2, 0);
- aMatrix.Line3.Column2 = maUsedTransformation.get(2, 1);
- aMatrix.Line3.Column3 = maUsedTransformation.get(2, 2);
+ aUnoMatrix.Line2.Column1 = aB2DHomMatrix.get(1, 0);
+ aUnoMatrix.Line2.Column2 = aB2DHomMatrix.get(1, 1);
+ aUnoMatrix.Line2.Column3 = aB2DHomMatrix.get(1, 2);
- xPropSet->setPropertyValue("Transformation", Any(aMatrix));
+ aUnoMatrix.Line3.Column1 = aB2DHomMatrix.get(2, 0);
+ aUnoMatrix.Line3.Column2 = aB2DHomMatrix.get(2, 1);
+ aUnoMatrix.Line3.Column3 = aB2DHomMatrix.get(2, 2);
+
+ xPropSet->setPropertyValue("Transformation", Any(aUnoMatrix));
}
}
}
@@ -1093,9 +1110,9 @@ void SdXMLLineShapeContext::StartElement(const uno::Reference< xml::sax::XAttrib
xPropSet->setPropertyValue("Geometry", Any(aPolyPoly));
}
- // set sizes for transformation
- maSize.Width = o3tl::saturating_sub(aBottomRight.X, aTopLeft.X);
- maSize.Height = o3tl::saturating_sub(aBottomRight.Y, aTopLeft.Y);
+ // Size is included in point coordinates
+ maSize.Width = 1;
+ maSize.Height = 1;
maPosition.X = aTopLeft.X;
maPosition.Y = aTopLeft.Y;
@@ -1322,6 +1339,10 @@ void SdXMLPolygonShapeContext::StartElement(const uno::Reference< xml::sax::XAtt
css::drawing::PointSequenceSequence aPointSequenceSequence;
basegfx::utils::B2DPolyPolygonToUnoPointSequenceSequence(basegfx::B2DPolyPolygon(aPolygon), aPointSequenceSequence);
xPropSet->setPropertyValue("Geometry", Any(aPointSequenceSequence));
+ // Size is now contained in the point coordinates, adapt maSize for
+ // to use the correct transformation matrix in SetTransformation()
+ maSize.Width = 1;
+ maSize.Height = 1;
}
}
}
@@ -1471,6 +1492,10 @@ void SdXMLPathShapeContext::StartElement(const uno::Reference< xml::sax::XAttrib
}
xPropSet->setPropertyValue("Geometry", aAny);
+ // Size is now contained in the point coordinates, adapt maSize for
+ // to use the correct transformation matrix in SetTransformation()
+ maSize.Width = 1;
+ maSize.Height = 1;
}
// set pos, size, shear and rotate
More information about the Libreoffice-commits
mailing list