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@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@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 fea94105..6484ae8 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 0000000..36c5062
--- /dev/null
+++ b/oox/qa/unit/data/tdf141463_GroupTransform.pptx
Binary files differ
diff --git a/oox/qa/unit/shape.cxx b/oox/qa/unit/shape.cxx
index a57c779..9d01128 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 8e298bf..a9388e8 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() );
}
