tdf#121761, tdf#121952 Accurate ellipsequadrant in custom shape
The current solution uses one, bad valued Bezier curve and does not
toggle direction. The patch uses the more accurate solution from
basegfx and simplifies the decision tree. In addition it extracts the
calculation in double from GetPoint, so they can be used directly
instead of double->long->double conversion.
Change-Id: I298f49415d1b7624b36ca561647f2e58af5eb5c6
Reviewed-on: https://gerrit.libreoffice.org/65203
Tested-by: Jenkins
Reviewed-by: Regina Henschel <rb.henschel@t-online.de>
diff --git a/include/svx/EnhancedCustomShape2d.hxx b/include/svx/EnhancedCustomShape2d.hxx
index b286407..7b35083 100644
--- a/include/svx/EnhancedCustomShape2d.hxx
+++ b/include/svx/EnhancedCustomShape2d.hxx
@@ -35,6 +35,7 @@
#include <svx/EnhancedCustomShapeFunctionParser.hxx>
#include <tools/gen.hxx>
#include <o3tl/typed_flags_set.hxx>
#include <basegfx/point/b2dpoint.hxx>
#include <memory>
#include <vector>
@@ -132,6 +133,8 @@ class SVX_DLLPUBLIC EnhancedCustomShape2d : public SfxItemSet
sal_uInt32 nColorCount);
SAL_DLLPRIVATE Point GetPoint( const css::drawing::EnhancedCustomShapeParameterPair&,
const bool bScale = true, const bool bReplaceGeoSize = false ) const;
SAL_DLLPRIVATE basegfx::B2DPoint GetPointAsB2DPoint(const css::drawing::EnhancedCustomShapeParameterPair&,
const bool bScale = true, const bool bReplaceGeoSize = false ) const;
SAL_DLLPRIVATE void CreateSubPath(
sal_Int32& rSrcPt,
diff --git a/svx/qa/unit/customshapes.cxx b/svx/qa/unit/customshapes.cxx
index b5aeea6..d3f63df 100644
--- a/svx/qa/unit/customshapes.cxx
+++ b/svx/qa/unit/customshapes.cxx
@@ -48,9 +48,13 @@ public:
}
void testViewBoxLeftTop();
void testAccuracyCommandX();
void testToggleCommandXY();
CPPUNIT_TEST_SUITE(CustomshapesTest);
CPPUNIT_TEST(testViewBoxLeftTop);
CPPUNIT_TEST(testAccuracyCommandX);
CPPUNIT_TEST(testToggleCommandXY);
CPPUNIT_TEST_SUITE_END();
};
@@ -96,6 +100,67 @@ void CustomshapesTest::testViewBoxLeftTop()
CPPUNIT_ASSERT_LESS(static_cast<long>(3), labs(aFrameRectTB.Y - aBoundRectTB.Y));
}
void CustomshapesTest::testAccuracyCommandX()
{
// 121761 Increase accuracy of quarter circles drawn by command X or Y
// The loaded document has a quarter circle with radius 10000 (unit 1/100 mm)
// which is rotated by 45deg. The test considers the segment.
OUString aURL
= m_directories.getURLFromSrc(sDataDirectory) + "tdf121761_Accuracy_command_X.odp";
mxComponent = loadFromDesktop(aURL, "com.sun.star.comp.presentation.PresentationDocument");
CPPUNIT_ASSERT_MESSAGE("Could not load document", mxComponent.is());
uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(mxComponent,
uno::UNO_QUERY_THROW);
CPPUNIT_ASSERT_MESSAGE("Could not get XDrawPagesSupplier", xDrawPagesSupplier.is());
uno::Reference<drawing::XDrawPages> xDrawPages(xDrawPagesSupplier->getDrawPages());
uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPages->getByIndex(0), uno::UNO_QUERY_THROW);
// Get the shape "arc_45deg_rotated". Error was, that a Bezier curve with bad parameters
// was used, thus the segment height was obviously smaller than for a true circle.
// Math: segment height approx 10000 * ( 1 - sqrt(0.5)) + line width
uno::Reference<drawing::XShape> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY);
CPPUNIT_ASSERT_MESSAGE("Could not get the shape", xShape.is());
uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
CPPUNIT_ASSERT_MESSAGE("Could not get the shape properties", xShapeProps.is());
awt::Rectangle aBoundRect;
xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_BOUNDRECT) >>= aBoundRect;
double fHeight = static_cast<double>(aBoundRect.Height);
// The tolerance is a guess, might be smaller.
CPPUNIT_ASSERT_DOUBLES_EQUAL_MESSAGE("segment height out of tolerance", 2942.0, fHeight, 8.0);
}
void CustomshapesTest::testToggleCommandXY()
{
// 121952 Toggle x- and y-direction if command X has several parameters
// The loaded document has a shape with command X and two parameter placed on a diagonal.
// The radius of the quarter circles are both 10000 (unit 1/100 mm).
// The shape is rotated by 45deg, so you get two segments, one up and one down.
OUString aURL
= m_directories.getURLFromSrc(sDataDirectory) + "tdf121952_Toggle_direction_command_X.odp";
mxComponent = loadFromDesktop(aURL, "com.sun.star.comp.presentation.PresentationDocument");
CPPUNIT_ASSERT_MESSAGE("Could not load document", mxComponent.is());
uno::Reference<drawing::XDrawPagesSupplier> xDrawPagesSupplier(mxComponent,
uno::UNO_QUERY_THROW);
CPPUNIT_ASSERT_MESSAGE("Could not get XDrawPagesSupplier", xDrawPagesSupplier.is());
uno::Reference<drawing::XDrawPages> xDrawPages(xDrawPagesSupplier->getDrawPages());
uno::Reference<drawing::XDrawPage> xDrawPage(xDrawPages->getByIndex(0), uno::UNO_QUERY_THROW);
// Error was, that the second segment was drawn with same direction as first one. If drawn
// correctly, the bounding box height of the segments together is about twice the single
// segment height. Math: segment height approx 10000 * ( 1 - sqrt(0.5)) + line width
uno::Reference<drawing::XShape> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY);
CPPUNIT_ASSERT_MESSAGE("Could not get the shape", xShape.is());
uno::Reference<beans::XPropertySet> xShapeProps(xShape, uno::UNO_QUERY);
CPPUNIT_ASSERT_MESSAGE("Could not get the shape properties", xShapeProps.is());
awt::Rectangle aBoundRect;
xShapeProps->getPropertyValue(UNO_NAME_MISC_OBJ_BOUNDRECT) >>= aBoundRect;
double fHeight = static_cast<double>(aBoundRect.Height);
// The tolerance is a guess, might be smaller.
CPPUNIT_ASSERT_DOUBLES_EQUAL_MESSAGE("segment height out of tolerance", 5871.0, fHeight, 16.0);
}
CPPUNIT_TEST_SUITE_REGISTRATION(CustomshapesTest);
}
diff --git a/svx/qa/unit/data/tdf121761_Accuracy_command_X.odp b/svx/qa/unit/data/tdf121761_Accuracy_command_X.odp
new file mode 100644
index 0000000..1de3917
--- /dev/null
+++ b/svx/qa/unit/data/tdf121761_Accuracy_command_X.odp
Binary files differ
diff --git a/svx/qa/unit/data/tdf121952_Toggle_direction_command_X.odp b/svx/qa/unit/data/tdf121952_Toggle_direction_command_X.odp
new file mode 100644
index 0000000..fbe1b7d
--- /dev/null
+++ b/svx/qa/unit/data/tdf121952_Toggle_direction_command_X.odp
Binary files differ
diff --git a/svx/source/customshapes/EnhancedCustomShape2d.cxx b/svx/source/customshapes/EnhancedCustomShape2d.cxx
index 12448ca..c01c499 100644
--- a/svx/source/customshapes/EnhancedCustomShape2d.cxx
+++ b/svx/source/customshapes/EnhancedCustomShape2d.cxx
@@ -917,40 +917,32 @@ bool EnhancedCustomShape2d::SetAdjustValueAsDouble( const double& rValue, const
return bRetValue;
}
basegfx::B2DPoint EnhancedCustomShape2d::GetPointAsB2DPoint( const css::drawing::EnhancedCustomShapeParameterPair& rPair,
const bool bScale, const bool bReplaceGeoSize ) const
{
double fValX, fValY;
// width
GetParameter(fValX, rPair.First, bReplaceGeoSize, false);
fValX -= nCoordLeft;
if (bScale)
{
fValX *= fXScale;
}
// height
GetParameter(fValY, rPair.Second, false, bReplaceGeoSize);
fValY -= nCoordTop;
if (bScale)
{
fValY *= fYScale;
}
return basegfx::B2DPoint(fValX,fValY);
}
Point EnhancedCustomShape2d::GetPoint( const css::drawing::EnhancedCustomShapeParameterPair& rPair,
const bool bScale, const bool bReplaceGeoSize ) const
{
Point aRetValue;
sal_uInt32 nPass = 0;
do
{
sal_uInt32 nIndex = nPass;
double fVal;
const EnhancedCustomShapeParameter& rParameter = nIndex ? rPair.Second : rPair.First;
if ( nPass ) // height
{
GetParameter( fVal, rParameter, false, bReplaceGeoSize );
fVal -= nCoordTop;
if ( bScale )
{
fVal *= fYScale;
}
aRetValue.setY( static_cast<sal_Int32>(fVal) );
}
else // width
{
GetParameter( fVal, rParameter, bReplaceGeoSize, false );
fVal -= nCoordLeft;
if ( bScale )
{
fVal *= fXScale;
}
aRetValue.setX( static_cast<long>(fVal) );
}
}
while ( ++nPass < 2 );
return aRetValue;
basegfx::B2DPoint aPoint(GetPointAsB2DPoint(rPair, bScale, bReplaceGeoSize));
return Point(static_cast<long>(aPoint.getX()), static_cast<long>(aPoint.getY()));
}
void EnhancedCustomShape2d::GetParameter( double& rRetValue, const EnhancedCustomShapeParameter& rParameter,
@@ -1841,69 +1833,96 @@ void EnhancedCustomShape2d::CreateSubPath(
case ELLIPTICALQUADRANTX :
case ELLIPTICALQUADRANTY :
{
bool bFirstDirection(true);
basegfx::B2DPoint aControlPointA;
basegfx::B2DPoint aControlPointB;
for ( sal_uInt16 i = 0; ( i < nPntCount ) && ( rSrcPt < nCoordSize ); i++ )
if (nPntCount && (rSrcPt < nCoordSize))
{
sal_uInt32 nModT = ( nCommand == ELLIPTICALQUADRANTX ) ? 1 : 0;
Point aCurrent( GetPoint( seqCoordinates[ rSrcPt ], true, true ) );
if ( rSrcPt ) // we need a previous point
// The arc starts at the previous point and ends at the point given in the parameter.
basegfx::B2DPoint aStart;
basegfx::B2DPoint aEnd;
sal_uInt16 i = 0;
if (rSrcPt)
{
Point aPrev( GetPoint( seqCoordinates[ rSrcPt - 1 ], true, true ) );
sal_Int32 nX, nY;
nX = aCurrent.X() - aPrev.X();
nY = aCurrent.Y() - aPrev.Y();
if ( ( nY ^ nX ) & 0x80000000 )
{
if ( !i )
bFirstDirection = true;
else if ( !bFirstDirection )
nModT ^= 1;
}
else
{
if ( !i )
bFirstDirection = false;
else if ( bFirstDirection )
nModT ^= 1;
}
if ( nModT ) // get the right corner
{
nX = aCurrent.X();
nY = aPrev.Y();
}
else
{
nX = aPrev.X();
nY = aCurrent.Y();
}
sal_Int32 nXVec = ( nX - aPrev.X() ) >> 1;
sal_Int32 nYVec = ( nY - aPrev.Y() ) >> 1;
Point aControl1( aPrev.X() + nXVec, aPrev.Y() + nYVec );
aControlPointA = basegfx::B2DPoint(aControl1.X(), aControl1.Y());
nXVec = ( nX - aCurrent.X() ) >> 1;
nYVec = ( nY - aCurrent.Y() ) >> 1;
Point aControl2( aCurrent.X() + nXVec, aCurrent.Y() + nYVec );
aControlPointB = basegfx::B2DPoint(aControl2.X(), aControl2.Y());
aNewB2DPolygon.appendBezierSegment(
aControlPointA,
aControlPointB,
basegfx::B2DPoint(aCurrent.X(), aCurrent.Y()));
aStart = GetPointAsB2DPoint(seqCoordinates[rSrcPt - 1], true, true);
}
else
{
aNewB2DPolygon.append(basegfx::B2DPoint(aCurrent.X(), aCurrent.Y()));
{ // no previous point, path is ill-structured. But we want to show as much as possible.
// Thus make a moveTo to the point given as parameter and continue from there.
aStart = GetPointAsB2DPoint(seqCoordinates[static_cast<sal_uInt16>(rSrcPt)], true, true);
aNewB2DPolygon.append(aStart);
rSrcPt++;
i++;
}
rSrcPt++;
// If there are several points, then the direction changes with every point.
bool bIsXDirection(nCommand == ELLIPTICALQUADRANTX);
basegfx::B2DPolygon aArc;
for ( ; ( i < nPntCount ) && ( rSrcPt < nCoordSize ); i++ )
{
aEnd = GetPointAsB2DPoint(seqCoordinates[rSrcPt], true, true);
basegfx::B2DPoint aCenter;
double fRadiusX = fabs(aEnd.getX() - aStart.getX());
double fRadiusY = fabs(aEnd.getY() - aStart.getY());
if (bIsXDirection)
{
aCenter = basegfx::B2DPoint(aStart.getX(),aEnd.getY());
if (aEnd.getX()<aStart.getX())
{
if (aEnd.getY()<aStart.getY()) // left, up
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, F_PI2, F_PI);
}
else // left, down
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, F_PI, 1.5*F_PI);
aArc.flip();
}
}
else // aEnd.getX()>=aStart.getX()
{
if (aEnd.getY()<aStart.getY()) // right, up
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, 0.0, F_PI2);
aArc.flip();
}
else // right, down
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, 1.5*F_PI, F_2PI);
}
}
}
else // y-direction
{
aCenter = basegfx::B2DPoint(aEnd.getX(),aStart.getY());
if (aEnd.getX()<aStart.getX())
{
if (aEnd.getY()<aStart.getY()) // up, left
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, 1.5*F_PI, F_2PI);
aArc.flip();
}
else // down, left
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, 0.0, F_PI2);
}
}
else // aEnd.getX()>=aStart.getX()
{
if (aEnd.getY()<aStart.getY()) // up, right
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, F_PI, 1.5*F_PI);
}
else // down, right
{
aArc = basegfx::utils::createPolygonFromEllipseSegment(aCenter, fRadiusX, fRadiusY, F_PI2, F_PI);
aArc.flip();
}
}
}
aNewB2DPolygon.append(aArc);
rSrcPt++;
bIsXDirection = !bIsXDirection;
aStart = aEnd;
}
}
// else error in path syntax, do nothing
}
break;