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

Szabolcs Toth (via logerrit) logerrit at kemper.freedesktop.org
Fri May 29 15:26:47 UTC 2020


 sc/qa/unit/data/xlsx/testShapeRotationImport.xlsx |binary
 sc/qa/unit/subsequent_filters-test.cxx            |   49 ++++++++++++++++++++++
 sc/source/filter/oox/drawingfragment.cxx          |   37 ++++++++++++++--
 3 files changed, 82 insertions(+), 4 deletions(-)

New commits:
commit 130e6a3f4493b987a7d0b177cc84d65219b47d13
Author:     Szabolcs Toth <szabolcs450 at gmail.com>
AuthorDate: Thu May 21 09:06:08 2020 +0200
Commit:     László Németh <nemeth at numbertext.org>
CommitDate: Fri May 29 17:26:08 2020 +0200

    tdf#83593 XLSX DrawingML shape import: fix missing rotation
    
    caused by broken import of xdr:twoCellAnchor.
    
    Co-authored-by: Balázs Regényi
    
    Change-Id: I3f382c3c9b2428e825a3e6d954c65356942f9158
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/94611
    Tested-by: László Németh <nemeth at numbertext.org>
    Reviewed-by: László Németh <nemeth at numbertext.org>

diff --git a/sc/qa/unit/data/xlsx/testShapeRotationImport.xlsx b/sc/qa/unit/data/xlsx/testShapeRotationImport.xlsx
new file mode 100644
index 000000000000..125a3ccaabd0
Binary files /dev/null and b/sc/qa/unit/data/xlsx/testShapeRotationImport.xlsx differ
diff --git a/sc/qa/unit/subsequent_filters-test.cxx b/sc/qa/unit/subsequent_filters-test.cxx
index e56ebd256eb1..3292058ed979 100644
--- a/sc/qa/unit/subsequent_filters-test.cxx
+++ b/sc/qa/unit/subsequent_filters-test.cxx
@@ -77,6 +77,7 @@
 
 #include "helper/qahelper.hxx"
 #include "helper/shared_test_impl.hxx"
+#include <cellsuno.hxx>
 
 namespace com::sun::star::frame { class XModel; }
 
@@ -257,6 +258,7 @@ public:
     void testAutoheight2Rows();
     void testXLSDefColWidth();
     void testPreviewMissingObjLink();
+    void testShapeRotationImport();
 
     CPPUNIT_TEST_SUITE(ScFiltersTest);
     CPPUNIT_TEST(testBooleanFormatXLSX);
@@ -405,6 +407,7 @@ public:
     CPPUNIT_TEST(testAutoheight2Rows);
     CPPUNIT_TEST(testXLSDefColWidth);
     CPPUNIT_TEST(testPreviewMissingObjLink);
+    CPPUNIT_TEST(testShapeRotationImport);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -4445,6 +4448,52 @@ void ScFiltersTest::testPreviewMissingObjLink()
     xDocSh->DoClose();
 }
 
+void ScFiltersTest::testShapeRotationImport()
+{
+    // tdf#83593 Incorrectly calculated bounding rectangles caused shapes to appear as if there
+    // were extra or missing rotations. Hence, we check the sizes of these rectangles.
+    ScDocShellRef xDocSh = loadDoc("testShapeRotationImport.", FORMAT_XLSX);
+    CPPUNIT_ASSERT_MESSAGE("Failed to load testShapeRotationImport.xlsx", xDocSh.is());
+    uno::Reference< drawing::XDrawPagesSupplier > xDoc(xDocSh->GetModel(), uno::UNO_QUERY_THROW);
+    uno::Reference< drawing::XDrawPage > xPage(xDoc->getDrawPages()->getByIndex(0), uno::UNO_QUERY_THROW);
+
+    // The expected values are in the map below. Note that some of the angles are outside of the set which contains
+    // the value of the wrong angles. This is to check the border cases and one value on both sides.
+    std::map<sal_Int32, std::map<std::string, sal_Int32>> aExpectedValues
+    {
+        {  4400, { { "x",  6832 }, { "y", 36893 }, { "width",  8898 }, { "height", 1163 } } },
+        {  4500, { { "x",  4490 }, { "y", 36400 }, { "width",  8898 }, { "height", 1163 } } },
+        {  4600, { { "x",  1673 }, { "y", 36270 }, { "width",  8862 }, { "height", 1144 } } },
+        { 13400, { { "x", 13762 }, { "y", 28403 }, { "width",  8863 }, { "height", 1194 } } },
+        { 13500, { { "x", 10817 }, { "y", 27951 }, { "width",  8863 }, { "height", 1170 } } },
+        { 13600, { { "x",  8449 }, { "y", 28336 }, { "width",  8897 }, { "height", 1164 } } },
+        { 22400, { { "x", 14948 }, { "y", 12978 }, { "width",  8898 }, { "height", 1164 } } },
+        { 22500, { { "x", 11765 }, { "y", 12834 }, { "width",  8898 }, { "height", 1164 } } },
+        { 22600, { { "x",  8253 }, { "y", 12919 }, { "width",  8863 }, { "height", 1171 } } },
+        { 31400, { { "x",  8099 }, { "y",  1160 }, { "width",  9815 }, { "height", 1171 } } },
+        { 31500, { { "x",  4427 }, { "y",  1274 }, { "width", 10238 }, { "height", 1171 } } },
+        { 31600, { { "x",  1964 }, { "y",  1878 }, { "width", 10307 }, { "height", 1164 } } },
+    };
+
+    for (sal_Int32 ind = 0; ind < 12; ++ind)
+    {
+        uno::Reference< drawing::XShape > xShape(xPage->getByIndex(ind), uno::UNO_QUERY_THROW);
+
+        uno::Reference< beans::XPropertySet > xShapeProperties(xShape, uno::UNO_QUERY);
+        uno::Any nRotProp = xShapeProperties->getPropertyValue("RotateAngle");
+        sal_Int32 nRot = nRotProp.get<sal_Int32>();
+
+        awt::Point aPosition = xShape->getPosition();
+        awt::Size aSize = xShape->getSize();
+
+        CPPUNIT_ASSERT(aExpectedValues.find(nRot) != aExpectedValues.end());
+        CPPUNIT_ASSERT_EQUAL(aExpectedValues[nRot]["x"],      aPosition.X);
+        CPPUNIT_ASSERT_EQUAL(aExpectedValues[nRot]["y"],      aPosition.Y);
+        CPPUNIT_ASSERT_EQUAL(aExpectedValues[nRot]["width"],  aSize.Width);
+        CPPUNIT_ASSERT_EQUAL(aExpectedValues[nRot]["height"], aSize.Height);
+    }
+}
+
 ScFiltersTest::ScFiltersTest()
       : ScBootstrapFixture( "sc/qa/unit/data" )
 {
diff --git a/sc/source/filter/oox/drawingfragment.cxx b/sc/source/filter/oox/drawingfragment.cxx
index 9c440d76c0f8..396170be35e3 100644
--- a/sc/source/filter/oox/drawingfragment.cxx
+++ b/sc/source/filter/oox/drawingfragment.cxx
@@ -257,14 +257,43 @@ void DrawingFragment::onEndElement()
         case XDR_TOKEN( twoCellAnchor ):
             if( mxDrawPage.is() && mxShape && mxAnchor )
             {
-                // Rotation is decided by orientation of shape determined
-                // by the anchor position given by 'editAs="oneCell"'
-                if ( mxAnchor->getEditAs() != ShapeAnchor::ANCHOR_ONECELL )
-                        mxShape->setRotation(0);
                 EmuRectangle aShapeRectEmu = mxAnchor->calcAnchorRectEmu( getDrawPageSize() );
                 const bool bIsShapeVisible = mxAnchor->isAnchorValid();
                 if( (aShapeRectEmu.X >= 0) && (aShapeRectEmu.Y >= 0) && (aShapeRectEmu.Width >= 0) && (aShapeRectEmu.Height >= 0) )
                 {
+                    const sal_Int32 aRotation = mxShape->getRotation();
+                    if ((aRotation >= 45  * PER_DEGREE && aRotation < 135 * PER_DEGREE)
+                     || (aRotation >= 225 * PER_DEGREE && aRotation < 315 * PER_DEGREE))
+                    {
+                        // When rotating any shape in MSO Excel within the range of degrees given above,
+                        // Excel changes the cells in which the shape is anchored. The new position of
+                        // the anchors are always calculated using a 90 degrees rotation anticlockwise.
+                        // There is an important result of this operation: the top left point of the shape changes,
+                        // it will be another vertex.
+                        // The anchor position is given in the xml file, it is in the xdr:from and xdr:to elements.
+                        // Let's see what happens in time order:
+                        // We create a shape in Excel, the anchor position is in a given cell, then the rotation happens
+                        // as mentioned above, and excel recalculates the cells in which the anchors are positioned.
+                        // This new cell is exported into the xml elements xdr:from and xdr:to, when Excel exports the document!
+                        // Thus, if we have a 90 degrees rotation and an already rotated point from which we base
+                        // our calculations here in LO, the result is an incorrect 180 degrees rotation.
+                        // Now, we need to create the bounding rectangle of the shape with this in mind.
+                        // (Important to mention that at this point we don't talk about rotations at all, this bounding
+                        // rectangle contains the original not-rotated shape. Rotation happens later in the code.)
+                        // We get the new (x, y) coords, then swap width with height.
+                        // To get the new coords we reflect the rectangle in the line y = x. (This will return the
+                        // correct vertex, which is the actual top left one.)
+                        // Another fact that appears to be true in Excel is that there are only 2 of possible anchor
+                        // positions for a shape that is only rotated (and not resized for example).
+                        // The first position happens in the set of degrees {[45, 135) U [225, 315)} and the second
+                        // set is all the other angles. The two sets partition the circle (of all rotations: 360 degrees).
+                        sal_Int64 nHalfWidth = aShapeRectEmu.Width / 2;
+                        sal_Int64 nHalfHeight = aShapeRectEmu.Height / 2;
+                        aShapeRectEmu.X = aShapeRectEmu.X + nHalfWidth - nHalfHeight;
+                        aShapeRectEmu.Y = aShapeRectEmu.Y + nHalfHeight - nHalfWidth;
+                        std::swap(aShapeRectEmu.Width, aShapeRectEmu.Height);
+                    }
+
                     // TODO: DrawingML implementation expects 32-bit coordinates for EMU rectangles (change that to EmuRectangle)
                     Rectangle aShapeRectEmu32(
                         getLimitedValue< sal_Int32, sal_Int64 >( aShapeRectEmu.X, 0, SAL_MAX_INT32 ),


More information about the Libreoffice-commits mailing list