oox/qa/unit/data/toplevel-line-hori-offset.docx |binary
 oox/qa/unit/drawingml.cxx                       |   51 ++++++++++++++++++++++++
 oox/source/drawingml/shape.cxx                  |   34 +++++++++++++---
 sw/qa/extras/ooxmlexport/ooxmlexport3.cxx       |    2 
 sw/qa/writerfilter/dmapper/GraphicImport.cxx    |    4 -
 5 files changed, 82 insertions(+), 9 deletions(-)

New commits:
commit 568782f2b6a6abb1f63f5c31d8c3db1dacd0ec01
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Jun 26 09:21:17 2024 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Jun 26 13:19:57 2024 +0200

    tdf#161779 DOCX import, drawingML: fix handling of translation for lines
    
    Open the bugdoc, it has a line with a non-zero horizontal offset from
    the anchor paragraph, it shows up as a horizontal line, while it should
    be vertical.
    
    Checking how the ODT import and the DOCX import works for lines, one
    obvious difference is that the ODT import at
    SdXMLLineShapeContext::startFastElement() only considers the size /
    scaling for the individual points, everything else goes to the transform
    matrix of the containing shape, set in
    SdXMLShapeContext::SetTransformation(). The drawingML import is way more
    complex, but it effectively tries to not set any transformation on the
    shape and just transorms the points of the line instead.
    
    Fix the problem by changing Shape::createAndInsert() to also not put any
    scaling to the transform matrix, to not transform the points of the line
    and finally to apply the transform matrix to lines as well.
    
    Do this only for toplevel Writer lines, that's enough to fix the bugdoc
    and group shapes / Calc shapes need more investigation, so leave those
    unchanged for now. Tests which were failing while working on this
    change:
    - CppunitTest_sc_shapetest's testTdf144242_Line_noSwapWH: do this for
      Writer shapes only, for now
    - CppunitTest_sw_ooxmlimport's lineRotation: this is already broken
      partially, now looks perfect
    - CppunitTest_sw_ooxmlimport's testTdf85232 / group shape: this points
      out that lines in group shapes are some additional complexity, so
      leave that case unchanged, for now
    - CppunitTest_sw_ooxmlexport3's testArrowPosition: manual testing shows
      this is still OK
    - CppunitTest_sw_writerfilter_dmapper's testTdf141540GroupLinePosSize:
      manual testing shows this is still OK
    
    Change-Id: I246430148e3b3c927e010f360fa317e8429c82d2
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169533
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>
    Tested-by: Jenkins
    (cherry picked from commit 6c09c85ec384e88c89bff0817e7fe9889d7ed68e)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169550

diff --git a/oox/qa/unit/data/toplevel-line-hori-offset.docx 
b/oox/qa/unit/data/toplevel-line-hori-offset.docx
new file mode 100644
index 000000000000..9f6ba561d4fe
Binary files /dev/null and b/oox/qa/unit/data/toplevel-line-hori-offset.docx 
differ
diff --git a/oox/qa/unit/drawingml.cxx b/oox/qa/unit/drawingml.cxx
index 923758ee28a7..35cb7528452e 100644
--- a/oox/qa/unit/drawingml.cxx
+++ b/oox/qa/unit/drawingml.cxx
@@ -29,6 +29,8 @@
 #include <com/sun/star/text/XTextRange.hpp>
 #include <com/sun/star/table/XCellRange.hpp>
 #include <com/sun/star/util/XTheme.hpp>
+#include <com/sun/star/drawing/HomogenMatrix3.hpp>
+#include <com/sun/star/drawing/PointSequenceSequence.hpp>
 
 #include <docmodel/uno/UnoGradientTools.hxx>
 #include <docmodel/uno/UnoComplexColor.hxx>
@@ -698,6 +700,55 @@ CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, 
testTdf125085WordArtFontText)
     CPPUNIT_ASSERT_EQUAL(u"he"_ustr, aLocal.Language);
 }
 
+CPPUNIT_TEST_FIXTURE(OoxDrawingmlTest, testToplevelLineHorOffsetDOCX)
+{
+    // Given a toplevel line shape from DOCX:
+    loadFromFile(u"toplevel-line-hori-offset.docx");
+
+    // When checking the transform and the points of the shape:
+    uno::Reference<drawing::XDrawPagesSupplier> 
xDrawPagesSupplier(mxComponent, 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);
+    drawing::HomogenMatrix3 aTransformation;
+    xShape->getPropertyValue(u"Transformation"_ustr) >>= aTransformation;
+    basegfx::B2DHomMatrix aMatrix;
+    aMatrix.set(0, 0, aTransformation.Line1.Column1);
+    aMatrix.set(0, 1, aTransformation.Line1.Column2);
+    aMatrix.set(0, 2, aTransformation.Line1.Column3);
+    aMatrix.set(1, 0, aTransformation.Line2.Column1);
+    aMatrix.set(1, 1, aTransformation.Line2.Column2);
+    aMatrix.set(1, 2, aTransformation.Line2.Column3);
+    drawing::PointSequenceSequence aPolyPoly;
+    xShape->getPropertyValue(u"Geometry"_ustr) >>= aPolyPoly;
+
+    // Then make sure we get a vertical line, not a horizontal one:
+    basegfx::B2DTuple aScale;
+    basegfx::B2DTuple aTranslate;
+    double fRotate = 0;
+    double fShearX = 0;
+    aMatrix.decompose(aScale, aTranslate, fRotate, fShearX);
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 1
+    // - Actual  : 4094.76362560479
+    // i.e. this was a horizontal line, not a vertical one.
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(1, aScale.getX(), 0.01);
+    // 1473682 EMUs in mm100 is 4093.56.
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(4094, aScale.getY(), 0.01);
+    // 343535 EMUs in mm100 is 954.27.
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(954, aTranslate.getX(), 2);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(0, aTranslate.getY(), 0.01);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(0, fRotate, 0);
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(0, fShearX, 0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), aPolyPoly.getLength());
+    drawing::PointSequence aPoly = aPolyPoly[0];
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2), aPoly.getLength());
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), aPoly[0].X);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), aPoly[0].Y);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), aPoly[1].X);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(4094), aPoly[1].Y);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index 2513db943b73..2fcca550608c 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -1031,11 +1031,19 @@ Reference< XShape > const & Shape::createAndInsert(
         aTransformation.translate(0.5, 0.5);
     }
 
+    bool bLineShape = aServiceName == "com.sun.star.drawing.LineShape";
+    bool bTopWriterLine = !pParentGroupShape && mbWps && bLineShape;
     // Build object matrix from shape size and position; corresponds to MSO 
ext and off
     // Only LineShape and ConnectorShape may have zero width or height.
     if (aServiceName == "com.sun.star.drawing.LineShape"
         || aServiceName == "com.sun.star.drawing.ConnectorShape")
-        aTransformation.scale(maSize.Width, maSize.Height);
+    {
+        // For toplevel Writer lines, size is included in the point 
coordinates.
+        if (!bTopWriterLine)
+        {
+            aTransformation.scale(maSize.Width, maSize.Height);
+        }
+    }
     else
     {
         aTransformation.scale(maSize.Width ? maSize.Width : 1.0,
@@ -1143,7 +1151,10 @@ Reference< XShape > const & Shape::createAndInsert(
     aParentTransformation = aTransformation;
 
     constexpr double fEmuToMm100 = o3tl::convert(1.0, o3tl::Length::emu, 
o3tl::Length::mm100);
-    aTransformation.scale(fEmuToMm100, fEmuToMm100);
+    if (!bTopWriterLine)
+    {
+        aTransformation.scale(fEmuToMm100, fEmuToMm100);
+    }
 
     // OOXML flips shapes before rotating them, so the rotation needs to be 
inverted
     if( bIsCustomShape && mbFlipH != mbFlipV )
@@ -1167,8 +1178,19 @@ Reference< XShape > const & Shape::createAndInsert(
     {
         ::basegfx::B2DPolygon aPoly;
         aPoly.insert( 0, ::basegfx::B2DPoint( 0, 0 ) );
-        aPoly.insert( 1, ::basegfx::B2DPoint( maSize.Width ? 1 : 0, 
maSize.Height ? 1 : 0 ) );
-        aPoly.transform( aTransformation );
+        if (bTopWriterLine)
+        {
+            // No transform of individual points, everything apart from size 
is part of the
+            // transform matrix.
+            sal_Int32 nMM100Width = o3tl::convert(maSize.Width, 
o3tl::Length::emu, o3tl::Length::mm100);
+            sal_Int32 nMM100Height = o3tl::convert(maSize.Height, 
o3tl::Length::emu, o3tl::Length::mm100);
+            aPoly.insert(1, ::basegfx::B2DPoint(nMM100Width, nMM100Height));
+        }
+        else
+        {
+            aPoly.insert( 1, ::basegfx::B2DPoint( maSize.Width ? 1 : 0, 
maSize.Height ? 1 : 0 ) );
+            aPoly.transform( aTransformation );
+        }
 
         // now creating the corresponding PolyPolygon
         sal_Int32 i, nNumPoints = aPoly.count();
@@ -1195,7 +1217,7 @@ Reference< XShape > const & Shape::createAndInsert(
 
         maShapeProperties.setProperty(PROP_PolyPolygon, aPolyPolySequence);
     }
-    else if ( aServiceName == "com.sun.star.drawing.ConnectorShape" )
+    if ( aServiceName == "com.sun.star.drawing.ConnectorShape" )
     {
         ::basegfx::B2DPolygon aPoly;
         aPoly.insert( 0, ::basegfx::B2DPoint( 0, 0 ) );
@@ -1210,7 +1232,7 @@ Reference< XShape > const & Shape::createAndInsert(
         maShapeProperties.setProperty(PROP_StartPosition, aAWTStartPosition);
         maShapeProperties.setProperty(PROP_EndPosition, aAWTEndPosition);
     }
-    else
+    else if (!bLineShape || bTopWriterLine)
     {
         // now set transformation for this object
         HomogenMatrix3 aMatrix;
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport3.cxx 
b/sw/qa/extras/ooxmlexport/ooxmlexport3.cxx
index d30d607061d3..14170bf3f23f 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport3.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport3.cxx
@@ -1153,7 +1153,7 @@ CPPUNIT_TEST_FIXTURE(Test, testArrowPosition)
 
     // This is the correct Y coordinate, the incorrect was 817880.
     assertXPathContent(pXmlDocument, 
"/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/wp:anchor"
-        "/wp:positionV/wp:posOffset"_ostr, u"516255"_ustr);
+        "/wp:positionV/wp:posOffset"_ostr, u"516890"_ustr);
 }
 
 CPPUNIT_TEST_FIXTURE(Test, testArrowMarker)
diff --git a/sw/qa/writerfilter/dmapper/GraphicImport.cxx 
b/sw/qa/writerfilter/dmapper/GraphicImport.cxx
index 35c1988c9b02..54358ed07052 100644
--- a/sw/qa/writerfilter/dmapper/GraphicImport.cxx
+++ b/sw/qa/writerfilter/dmapper/GraphicImport.cxx
@@ -167,9 +167,9 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf141540GroupLinePosSize)
     // Without fix in place, you had got Position = (19|6498), Size = 5001 x 2
     // i.e. the line was nearly horizontal instead of vertical
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(5022), aPosition.X);
-    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2963), aPosition.Y);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2965), aPosition.Y);
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), aSize.Width);
-    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(7073), aSize.Height);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(7071), aSize.Height);
 
     // Test group
     uno::Reference<drawing::XShape> xGroupShape(xDrawPage->getByIndex(1), 
uno::UNO_QUERY);

Reply via email to