[Libreoffice-commits] core.git: Branch 'distro/nisz/libreoffice-6-4' - sc/qa sc/source
Szabolcs Toth (via logerrit)
logerrit at kemper.freedesktop.org
Fri Jun 12 08:26:53 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 5c62bdca7246d3ee64fa1380c2bdc30d4c8c8481
Author: Szabolcs Toth <szabolcs450 at gmail.com>
AuthorDate: Thu May 21 09:06:08 2020 +0200
Commit: Gabor Kelemen <kelemen.gabor2 at nisz.hu>
CommitDate: Fri Jun 12 10:26:22 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>
(cherry picked from commit 130e6a3f4493b987a7d0b177cc84d65219b47d13)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96136
Reviewed-by: Szabolcs Toth <szabolcs450 at gmail.com>
Reviewed-by: Gabor Kelemen <kelemen.gabor2 at nisz.hu>
Tested-by: Gabor Kelemen <kelemen.gabor2 at nisz.hu>
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 e005b258d979..99840eceaaae 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 { namespace sun { namespace star { namespace frame { class XModel; } } } }
@@ -251,6 +252,7 @@ public:
void testAutoheight2Rows();
void testXLSDefColWidth();
void testPreviewMissingObjLink();
+ void testShapeRotationImport();
CPPUNIT_TEST_SUITE(ScFiltersTest);
CPPUNIT_TEST(testBooleanFormatXLSX);
@@ -393,6 +395,7 @@ public:
CPPUNIT_TEST(testAutoheight2Rows);
CPPUNIT_TEST(testXLSDefColWidth);
CPPUNIT_TEST(testPreviewMissingObjLink);
+ CPPUNIT_TEST(testShapeRotationImport);
CPPUNIT_TEST_SUITE_END();
@@ -4344,6 +4347,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 0eb6b460217d..716c46d1f974 100644
--- a/sc/source/filter/oox/drawingfragment.cxx
+++ b/sc/source/filter/oox/drawingfragment.cxx
@@ -256,14 +256,43 @@ void DrawingFragment::onEndElement()
case XDR_TOKEN( twoCellAnchor ):
if( mxDrawPage.is() && mxShape.get() && mxAnchor.get() )
{
- // 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