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>

4 files changed, 169 insertions, 85 deletions

diff --git a/svx/qa/unit/customshapes.cxx b/svx/qa/unit/customshapes.cxx
index b5aeea670ae2..d3f63dffe2a9 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 000000000000..1de391758e4c
--- /dev/null
+++ b/svx/qa/unit/data/tdf121761_Accuracy_command_X.odp
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 000000000000..fbe1b7d4032e
--- /dev/null
+++ b/svx/qa/unit/data/tdf121952_Toggle_direction_command_X.odp
diff --git a/svx/source/customshapes/EnhancedCustomShape2d.cxx b/svx/source/customshapes/EnhancedCustomShape2d.cxx
index 12448ca95054..c01c49949f94 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;
 }
 
-Point EnhancedCustomShape2d::GetPoint( const css::drawing::EnhancedCustomShapeParameterPair& rPair,
+basegfx::B2DPoint EnhancedCustomShape2d::GetPointAsB2DPoint( const css::drawing::EnhancedCustomShapeParameterPair& rPair,
                                         const bool bScale, const bool bReplaceGeoSize ) const
 {
-    Point       aRetValue;
-    sal_uInt32  nPass = 0;
-    do
+    double fValX, fValY;
+    // width
+    GetParameter(fValX, rPair.First, bReplaceGeoSize, false);
+    fValX -= nCoordLeft;
+    if (bScale)
     {
-        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) );
-        }
+        fValX *= fXScale;
     }
-    while ( ++nPass < 2 );
-    return aRetValue;
+    // 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
+{
+    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
+                            aStart = GetPointAsB2DPoint(seqCoordinates[rSrcPt - 1], true, true);
+                        }
+                        else
+                        {   // 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++;
+                        }
+                        // 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)
                             {
-                                nX = aCurrent.X();
-                                nY = aPrev.Y();
+                                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
+                            else // y-direction
                             {
-                                nX = aPrev.X();
-                                nY = aCurrent.Y();
+                                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();
+                                    }
+                                }
                             }
-                            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()));
-                        }
-                        else
-                        {
-                            aNewB2DPolygon.append(basegfx::B2DPoint(aCurrent.X(), aCurrent.Y()));
+                            aNewB2DPolygon.append(aArc);
+                            rSrcPt++;
+                            bIsXDirection = !bIsXDirection;
+                            aStart = aEnd;
                         }
-
-                        rSrcPt++;
                     }
+                    // else error in path syntax, do nothing
                 }
                 break;