[Libreoffice-commits] core.git: Branch 'libreoffice-7-1' - include/oox oox/qa oox/source
Regina Henschel (via logerrit)
logerrit at kemper.freedesktop.org
Mon May 17 19:04:10 UTC 2021
include/oox/drawingml/shape.hxx | 4
oox/qa/unit/data/tdf141463_GroupTransform.pptx |binary
oox/qa/unit/shape.cxx | 55 +++++
oox/source/drawingml/shape.cxx | 234 +++++++++++++++----------
4 files changed, 204 insertions(+), 89 deletions(-)
New commits:
commit ac0eb504a38321849b854889598a28d0687196ea
Author: Regina Henschel <rb.henschel at t-online.de>
AuthorDate: Mon May 10 00:47:13 2021 +0200
Commit: Xisco Fauli <xiscofauli at libreoffice.org>
CommitDate: Mon May 17 21:02:27 2021 +0200
tdf#141463 avoid skew in shape group in ooxml import ..
and implement special resize handling for rotation angles larger 45deg.
This solves tdf#93952 and tdf#141953 too.
Change-Id: I798f6d2cea29c4a5285f530e9cf7bb10e7f6c41d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115296
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli at libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115715
diff --git a/include/oox/drawingml/shape.hxx b/include/oox/drawingml/shape.hxx
index fea94105b65d..6484ae8530c5 100644
--- a/include/oox/drawingml/shape.hxx
+++ b/include/oox/drawingml/shape.hxx
@@ -187,7 +187,7 @@ public:
const basegfx::B2DHomMatrix& aTransformation,
FillProperties& rShapeOrParentShapeFillProps,
ShapeIdMap* pShapeMap = nullptr,
- bool bInGroup = false);
+ oox::drawingml::ShapePtr pParentGroupShape = nullptr);
const css::uno::Reference< css::drawing::XShape > &
getXShape() const { return mxShape; }
@@ -264,7 +264,7 @@ protected:
bool bDoNotInsertEmptyTextBody,
basegfx::B2DHomMatrix& aTransformation,
FillProperties& rShapeOrParentShapeFillProps,
- bool bInGroup = false
+ oox::drawingml::ShapePtr pParentGroupShape = nullptr
);
void addChildren(
diff --git a/oox/qa/unit/data/tdf141463_GroupTransform.pptx b/oox/qa/unit/data/tdf141463_GroupTransform.pptx
new file mode 100644
index 000000000000..36c506262333
Binary files /dev/null and b/oox/qa/unit/data/tdf141463_GroupTransform.pptx differ
diff --git a/oox/qa/unit/shape.cxx b/oox/qa/unit/shape.cxx
index a57c779d00fd..9d011282ad65 100644
--- a/oox/qa/unit/shape.cxx
+++ b/oox/qa/unit/shape.cxx
@@ -10,6 +10,8 @@
#include <test/bootstrapfixture.hxx>
#include <unotest/macros_test.hxx>
+#include <com/sun/star/awt/Point.hpp>
+#include <com/sun/star/awt/Size.hpp>
#include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
#include <com/sun/star/frame/Desktop.hpp>
#include <com/sun/star/beans/XPropertySet.hpp>
@@ -18,6 +20,24 @@
using namespace ::com::sun::star;
+namespace
+{
+/// Gets one child of xShape, which one is specified by nIndex.
+uno::Reference<drawing::XShape> getChildShape(const uno::Reference<drawing::XShape>& xShape,
+ sal_Int32 nIndex)
+{
+ uno::Reference<container::XIndexAccess> xGroup(xShape, uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xGroup.is());
+
+ CPPUNIT_ASSERT(xGroup->getCount() > nIndex);
+
+ uno::Reference<drawing::XShape> xRet(xGroup->getByIndex(nIndex), uno::UNO_QUERY);
+ CPPUNIT_ASSERT(xRet.is());
+
+ return xRet;
+}
+}
+
char const DATA_DIRECTORY[] = "/oox/qa/unit/data/";
/// oox shape tests.
@@ -54,6 +74,41 @@ void OoxShapeTest::load(const OUString& rFileName)
mxComponent = loadFromDesktop(aURL);
}
+CPPUNIT_TEST_FIXTURE(OoxShapeTest, testGroupTransform)
+{
+ load(u"tdf141463_GroupTransform.pptx");
+
+ uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(getComponent(), uno::UNO_QUERY);
+ uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPagesSupplier->getDrawPages()->getByIndex(0),
+ uno::UNO_QUERY);
+ uno::Reference<drawing::XShape> xGroup(xDrawPage->getByIndex(0), uno::UNO_QUERY);
+ uno::Reference<drawing::XShape> xShape(getChildShape(xGroup, 0), uno::UNO_QUERY);
+ uno::Reference<beans::XPropertySet> xPropSet(xShape, uno::UNO_QUERY);
+ // Without the accompanying fix in place, this test would have failed in several properties.
+
+ sal_Int32 nAngle;
+ xPropSet->getPropertyValue("ShearAngle") >>= nAngle;
+ // Failed with - Expected: 0
+ // - Actual : -810
+ // i.e. the shape was sheared although shearing does not exist in oox
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(0), nAngle);
+
+ xPropSet->getPropertyValue("RotateAngle") >>= nAngle;
+ // Failed with - Expected: 26000 (is in 1/100deg)
+ // - Actual : 26481 (is in 1/100deg)
+ // 100deg in PowerPoint UI = 360deg - 100deg in LO.
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(26000), nAngle);
+
+ sal_Int32 nActual = xShape->getSize().Width;
+ // The group has ext.cy=2880000 and chExt.cy=4320000 resulting in Y-scale=2/3.
+ // The child has ext 2880000 x 1440000. Because of rotation angle 80deg, the Y-scale has to be
+ // applied to the width, resulting in 2880000 * 2/3 = 1920000EMU = 5333Hmm
+ // ToDo: Expected value currently 1 off.
+ // Failed with - Expected: 5332
+ // - Actual : 5432
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(5332), nActual);
+}
+
CPPUNIT_TEST_FIXTURE(OoxShapeTest, testMultipleGroupShapes)
{
load("multiple-group-shapes.docx");
diff --git a/oox/source/drawingml/shape.cxx b/oox/source/drawingml/shape.cxx
index 8e298bf6bd56..a9388e8a4a70 100644
--- a/oox/source/drawingml/shape.cxx
+++ b/oox/source/drawingml/shape.cxx
@@ -84,6 +84,7 @@
#include <basegfx/point/b2dpoint.hxx>
#include <basegfx/polygon/b2dpolygon.hxx>
#include <basegfx/matrix/b2dhommatrix.hxx>
+#include <basegfx/matrix/b2dhommatrixtools.hxx>
#include <com/sun/star/document/XActionLockable.hpp>
#include <com/sun/star/chart2/data/XDataReceiver.hpp>
#include <svx/svdtrans.hxx>
@@ -269,7 +270,7 @@ void Shape::addShape(
const basegfx::B2DHomMatrix& aTransformation,
FillProperties& rShapeOrParentShapeFillProps,
ShapeIdMap* pShapeMap,
- bool bInGroup )
+ oox::drawingml::ShapePtr pParentGroupShape)
{
SAL_INFO("oox.drawingml", "Shape::addShape: id='" << msId << "'");
@@ -279,7 +280,7 @@ void Shape::addShape(
if( !sServiceName.isEmpty() )
{
basegfx::B2DHomMatrix aMatrix( aTransformation );
- Reference< XShape > xShape( createAndInsert( rFilterBase, sServiceName, pTheme, rxShapes, false, false, aMatrix, rShapeOrParentShapeFillProps, bInGroup ) );
+ Reference< XShape > xShape( createAndInsert( rFilterBase, sServiceName, pTheme, rxShapes, false, false, aMatrix, rShapeOrParentShapeFillProps, pParentGroupShape) );
if( pShapeMap && !msId.isEmpty() )
{
@@ -392,41 +393,10 @@ void Shape::addChildren(
ShapeIdMap* pShapeMap,
const basegfx::B2DHomMatrix& aTransformation )
{
- basegfx::B2DHomMatrix aChildTransformation;
-
- aChildTransformation.translate(-maChPosition.X, -maChPosition.Y);
- aChildTransformation.scale(1/(maChSize.Width ? maChSize.Width : 1.0), 1/(maChSize.Height ? maChSize.Height : 1.0));
-
- // Child position and size is typically non-zero, but it's allowed to have
- // it like that, and in that case Word ignores the parent transformation
- // (excluding translate component).
- if (!mbWps || maChPosition.X || maChPosition.Y || maChSize.Width || maChSize.Height)
- {
- aChildTransformation *= aTransformation;
- }
- else
- {
- basegfx::B2DVector aScale, aTranslate;
- double fRotate, fShearX;
- aTransformation.decompose(aScale, aTranslate, fRotate, fShearX);
- aChildTransformation.translate(aTranslate.getX(), aTranslate.getY());
- }
-
- SAL_INFO("oox.drawingml", "Shape::addChildren: parent matrix:\n"
- << aChildTransformation.get(0, 0) << " "
- << aChildTransformation.get(0, 1) << " "
- << aChildTransformation.get(0, 2) << "\n"
- << aChildTransformation.get(1, 0) << " "
- << aChildTransformation.get(1, 1) << " "
- << aChildTransformation.get(1, 2) << "\n"
- << aChildTransformation.get(2, 0) << " "
- << aChildTransformation.get(2, 1) << " "
- << aChildTransformation.get(2, 2));
-
for (auto const& child : rMaster.maChildren)
{
child->setMasterTextListStyle( mpMasterTextListStyle );
- child->addShape( rFilterBase, pTheme, rxShapes, aChildTransformation, getFillProperties(), pShapeMap, true );
+ child->addShape( rFilterBase, pTheme, rxShapes, aTransformation, getFillProperties(), pShapeMap, rMaster.shared_from_this());
}
}
@@ -651,6 +621,53 @@ static void lcl_createPresetShape(const uno::Reference<drawing::XShape>& xShape,
uno::makeAny(comphelper::containerToSequence(aGeomPropVec)));
}
+// Some helper methods for createAndInsert
+namespace
+{
+// mirrors aTransformation at its center axis
+// only valid if neither rotation or shear included
+void lcl_mirrorAtCenter(basegfx::B2DHomMatrix& aTransformation, bool bFlipH, bool bFlipV)
+{
+ if (!bFlipH && !bFlipV)
+ return;
+ basegfx::B2DPoint aCenter(0.5, 0.5);
+ aCenter *= aTransformation;
+ aTransformation.translate(-aCenter);
+ aTransformation.scale(bFlipH ? -1.0 : 1.0, bFlipV ? -1.0 : 1.0);
+ aTransformation.translate(aCenter);
+ return;
+}
+
+// only valid if neither rotation or shear included
+void lcl_doSpecialMSOWidthHeightToggle(basegfx::B2DHomMatrix& aTransformation)
+{
+ // The values are directly set at the matrix without any matrix multiplication.
+ // That way it is valid for lines too. Those have zero width or height.
+ const double fSx(aTransformation.get(0, 0));
+ const double fSy(aTransformation.get(1, 1));
+ const double fTx(aTransformation.get(0, 2));
+ const double fTy(aTransformation.get(1, 2));
+ aTransformation.set(0, 0, fSy);
+ aTransformation.set(1, 1, fSx);
+ aTransformation.set(0, 2, fTx + 0.5 * (fSx - fSy));
+ aTransformation.set(1, 2, fTy + 0.5 * (fSy - fSx));
+ return;
+}
+
+void lcl_RotateAtCenter(basegfx::B2DHomMatrix& aTransformation, const sal_Int32& rMSORotationAngle)
+{
+ if (rMSORotationAngle == 0)
+ return;
+ double fRad = basegfx::deg2rad(rMSORotationAngle / 60000.0);
+ basegfx::B2DPoint aCenter(0.5, 0.5);
+ aCenter *= aTransformation;
+ aTransformation.translate(-aCenter);
+ aTransformation.rotate(fRad);
+ aTransformation.translate(aCenter);
+ return;
+}
+}
+
Reference< XShape > const & Shape::createAndInsert(
::oox::core::XmlFilterBase& rFilterBase,
const OUString& rServiceName,
@@ -660,7 +677,7 @@ Reference< XShape > const & Shape::createAndInsert(
bool bDoNotInsertEmptyTextBody,
basegfx::B2DHomMatrix& aParentTransformation,
FillProperties& rShapeOrParentShapeFillProps,
- bool bInGroup )
+ oox::drawingml::ShapePtr pParentGroupShape)
{
bool bIsEmbMedia = false;
SAL_INFO("oox.drawingml", "Shape::createAndInsert: id='" << msId << "' service='" << rServiceName << "'");
@@ -726,6 +743,7 @@ Reference< XShape > const & Shape::createAndInsert(
(mpCustomShapePropertiesPtr->getShapePresetType() >= 0 || mpCustomShapePropertiesPtr->getPath2DList().size() > 0) &&
mpCustomShapePropertiesPtr->getShapePresetType() != XML_Rect &&
mpCustomShapePropertiesPtr->getShapePresetType() != XML_rect);
+ // ToDo: Why is ConnectorShape here treated as custom shape, but below with start and end point?
bool bIsCustomShape = ( aServiceName == "com.sun.star.drawing.CustomShape" ||
aServiceName == "com.sun.star.drawing.ConnectorShape" ||
bIsCroppedGraphic);
@@ -740,81 +758,125 @@ Reference< XShape > const & Shape::createAndInsert(
mbFlipH ||
mbFlipV );
- basegfx::B2DHomMatrix aTransformation;
+ basegfx::B2DHomMatrix aTransformation; // will be cumulative transformation of this object
+ // Special for SmartArt import. Rotate diagram's shape around object's center before sizing.
if (bUseRotationTransform && mnDiagramRotation != 0)
{
- // rotate diagram's shape around object's center before sizing
aTransformation.translate(-0.5, -0.5);
aTransformation.rotate(basegfx::deg2rad(mnDiagramRotation / 60000.0));
aTransformation.translate(0.5, 0.5);
}
- if( maSize.Width != 1 || maSize.Height != 1)
+ // 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);
+ else
{
- // take care there are no zeros used by error
- aTransformation.scale(
- maSize.Width ? maSize.Width : 1.0,
- maSize.Height ? maSize.Height : 1.0 );
+ aTransformation.scale(maSize.Width ? maSize.Width : 1.0,
+ maSize.Height ? maSize.Height : 1.0);
}
- bool bNoTranslation = !aParentTransformation.isIdentity();
- if( mbFlipH || mbFlipV || mnRotation != 0 || bNoTranslation )
+ // Evaluate object flip. Other shapes than custom shapes have no attribute for flip but use
+ // negative scale. Flip in MSO is at object center.
+ if (!bIsCustomShape && (mbFlipH || mbFlipV))
+ lcl_mirrorAtCenter(aTransformation, mbFlipH, mbFlipV);
+
+ // Evaluate parent flip.
+ // A CustomShape has mirror not as negative scale, but as attributes.
+ basegfx::B2DVector aParentScale(1.0, 1.0);
+ basegfx::B2DVector aParentTranslate(0.0, 0.0);
+ double fParentRotate(0.0);
+ double fParentShearX(0.0);
+ if (pParentGroupShape)
{
- // calculate object's center
- basegfx::B2DPoint aCenter(0.5, 0.5);
- aCenter *= aTransformation;
-
- // center object at origin
- aTransformation.translate( -aCenter.getX(), -aCenter.getY() );
-
- if( !bIsCustomShape && ( mbFlipH || mbFlipV ) )
+ aParentTransformation.decompose(aParentScale, aParentTranslate, fParentRotate, fParentShearX);
+ if (bIsCustomShape)
{
- // mirror around object's center
- aTransformation.scale( mbFlipH ? -1.0 : 1.0, mbFlipV ? -1.0 : 1.0 );
+ lcl_mirrorAtCenter(aTransformation, aParentScale.getX() < 0, aParentScale.getY() < 0);
+ if(aParentScale.getX() < 0)
+ mbFlipH = !mbFlipH;
+ if(aParentScale.getY() < 0)
+ mbFlipV = !mbFlipV;
}
-
- if( bUseRotationTransform )
- {
- // OOXML flips shapes before rotating them.
- if( bIsCustomShape )
- {
- basegfx::B2DVector aScale, aTranslate;
- double fRotate, fShearX;
- aParentTransformation.decompose(aScale, aTranslate, fRotate, fShearX);
- // A negative scale means that the shape needs to be flipped
- if(aScale.getX() < 0)
- {
- mbFlipH = !mbFlipH;
- aTransformation.scale(-1, 1);
- }
- if(aScale.getY() < 0)
- {
- mbFlipV = !mbFlipV;
- aTransformation.scale(1, -1);
- }
- }
- // rotate around object's center
- aTransformation.rotate(basegfx::deg2rad(static_cast<double>(mnRotation) / 60000.0));
- }
-
- // move object back from center
- aTransformation.translate( aCenter.getX(), aCenter.getY() );
}
- if( maPosition.X != 0 || maPosition.Y != 0)
+ if (maPosition.X != 0 || maPosition.Y != 0)
{
// if global position is used, add it to transformation
- if (mbWps && !bInGroup)
+ if (mbWps && pParentGroupShape == nullptr)
aTransformation.translate( maPosition.X * EMU_PER_HMM, maPosition.Y * EMU_PER_HMM);
else
- aTransformation.translate( maPosition.X, maPosition.Y );
+ aTransformation.translate(maPosition.X, maPosition.Y);
}
- aTransformation = aParentTransformation*aTransformation;
+ // Apply further parent transformations. First scale object then rotate. Other way round would
+ // introduce shearing.
+
+ // The attributes chExt and chOff of the group in oox file contain the values on which the size
+ // and position of the child is based on. If they differ from the actual size of the group as
+ // given in its ext and off attributes, the child has to be transformed according the new values.
+ if (pParentGroupShape)
+ {
+ // ToDo: A diagram in a group might need special handling because it cannot flip and only
+ // resize uniformly. But currently it is imported with zero size, see tdf#139575. That needs
+ // to be fixed beforehand.
+
+ // Scaling is done from left/top edges of the group. So these need to become coordinate axes.
+ aTransformation.translate(-pParentGroupShape->maChPosition.X,
+ -pParentGroupShape->maChPosition.Y);
+
+ // oox allows zero or missing attribute chExt. In that case the scaling factor is 1.
+ // Transform2DContext::onCreateContext has set maChSize to maSize for groups in oox file in
+ // such cases. For own made groups (e.g. diagrams) that is missing.
+ // The factors cumulate on the way through the parent groups, so we do not use maSize of the
+ // direct parent group but the cumulated value from aParentScale.
+ double fFactorX = 1.0;
+ double fFactorY = 1.0;
+ if (pParentGroupShape->maChSize.Width != 0)
+ fFactorX = aParentScale.getX() / pParentGroupShape->maChSize.Width;
+ if (pParentGroupShape->maChSize.Height != 0)
+ fFactorY = aParentScale.getY() / pParentGroupShape->maChSize.Height;
+ if (fFactorX != 1 || fFactorY != 1)
+ {
+ // It depends on the object rotation angle whether scaling is applied to switched
+ // width and height. MSO acts strange in that case (as of May 2021).
+ const sal_Int32 nDeg(mnRotation / 60000);
+ const bool bNeedsMSOWidhtHeightToggle
+ = (nDeg >= 45 && nDeg < 135) || (nDeg >= 225 && nDeg < 315);
+ if (bNeedsMSOWidhtHeightToggle)
+ lcl_doSpecialMSOWidthHeightToggle(aTransformation);
+
+ aTransformation.scale(fFactorX, fFactorY);
+
+ if (bNeedsMSOWidhtHeightToggle)
+ {
+ lcl_doSpecialMSOWidthHeightToggle(aTransformation);
+ // In case of flip the special case needs an additional 180deg rotation.
+ if ((aParentScale.getX() < 0) != (aParentScale.getY() < 0))
+ lcl_RotateAtCenter(aTransformation, 10800000);
+ }
+ }
+ }
+
+ // Apply object rotation at current object center
+ // The flip contained in aParentScale will affect orientation of object rotation angle.
+ sal_Int16 nOrientation = ((aParentScale.getX() < 0) != (aParentScale.getY() < 0)) ? -1 : 1;
+ // ToDo: Not sure about the restrictions given by bUseRotationTransform.
+ if (bUseRotationTransform && mnRotation != 0)
+ lcl_RotateAtCenter(aTransformation, nOrientation * mnRotation);
+
+ if (fParentRotate != 0.0)
+ aTransformation.rotate(fParentRotate);
+ if (!aParentTranslate.equalZero())
+ aTransformation.translate(aParentTranslate);
+
aParentTransformation = aTransformation;
aTransformation.scale(1/double(EMU_PER_HMM), 1/double(EMU_PER_HMM));
+ // OOXML flips shapes before rotating them, so the rotation needs to be inverted
if( bIsCustomShape && mbFlipH != mbFlipV )
{
basegfx::B2DVector aScale, aTranslate;
@@ -823,11 +885,9 @@ Reference< XShape > const & Shape::createAndInsert(
if(fRotate != 0)
{
- // calculate object's center
basegfx::B2DPoint aCenter(0.5, 0.5);
aCenter *= aTransformation;
aTransformation.translate( -aCenter.getX(), -aCenter.getY() );
- // OOXML flips shapes before rotating them, so the rotation needs to be inverted
aTransformation.rotate( fRotate * -2.0 );
aTransformation.translate( aCenter.getX(), aCenter.getY() );
}
More information about the Libreoffice-commits
mailing list