RFR: 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box [v2]

[Laurent Bourgès]

On Fri, 5 Nov 2021 04:41:34 GMT, Jeremy <duke at openjdk.java.net> wrote:

>> This removes code that relied on consulting the Bezier control points to calculate the Rectangle2D bounding box. Instead it's pretty straight-forward to convert the Bezier control points into the x & y parametric equations. At their most complex these equations are cubic polynomials, so calculating their extrema is just a matter of applying the quadratic formula to calculate their extrema. (Or in path segments that are quadratic/linear/constant: we do even less work.)
>> 
>> The bug writeup indicated they wanted Path2D#getBounds2D() to be more accurate/concise. They didn't explicitly say they wanted CubicCurve2D and QuadCurve2D to become more accurate too. But a preexisting unit test failed when Path2D#getBounds2D() was updated and those other classes weren't. At this point I considered either:
>> A. Updating CubicCurve2D and QuadCurve2D to use the new more accurate getBounds2D() or
>> B. Updating the unit test to forgive the discrepancy.
>> 
>> I chose A. Which might technically be seen as scope creep, but it feels like a more holistic/better approach.
>> 
>> Other shapes in java.awt.geom should not require updating, because they already identify concise bounds.
>> 
>> This also includes a new unit test (in Path2D/UnitTest.java) that fails without the changes in this commit.
>
> Jeremy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box
>   
>   Addressing some of Laurent's code review recommendations/comments:
>   
>   1. use the convention t for the parametric variable x(t),y(t)
>   2. solve the quadratic equation using QuadCurve2d.solveQuadratic() or like Helpers.quadraticRoots()
>   3. always use braces for x = (a < b) ? ...
>   4. always use double-precision constants in math or logical operations: (2 * x => 2.0 * x) and (coefficients[3] != 0) => (coefficients[3] != 0.0)
>   
>   (There are two additional recommendations not in this commit that I'll ask about shortly.)
>   
>   See https://github.com/openjdk/jdk/pull/6227#issuecomment-959757954

src/java.desktop/share/classes/java/awt/geom/Path2D.java line 2171:

> 2169:                 definedParametricEquations = true;
> 2170: 
> 2171:                 x_coeff[3] = -lastX + 3.0 * coords[0] - 3.0 * coords[2] + coords[4];

To improve coefficient accuracy ~ few ulps, it should be written explicitely as differences (= vector notation).
See Marlin DCurve formula:

    void set(final double x1, final double y1,
             final double x2, final double y2,
             final double x3, final double y3,
             final double x4, final double y4)
    {
        final double dx32 = 3.0 * (x3 - x2);
        final double dx21 = 3.0 * (x2 - x1);
        ax = (x4 - x1) - dx32;  // A = P3 - P0 - 3 (P2 - P1) = (P3 - P0) + 3 (P1 - P2)
        bx = (dx32 - dx21);     // B = 3 (P2 - P1) - 3(P1 - P0) = 3 (P2 + P0) - 6 P1
        cx = dx21;              // C = 3 (P1 - P0)
        dx = x1;                // D = P0
        dax = 3.0 * ax;
        dbx = 2.0 * bx;

        final double dy32 = 3.0 * (y3 - y2);
        final double dy21 = 3.0 * (y2 - y1);
        ay = (y4 - y1) - dy32;
        by = (dy32 - dy21);
        cy = dy21;
        dy = y1;
        day = 3.0 * ay;
        dby = 2.0 * by;
    }

    void set(final double x1, final double y1,
             final double x2, final double y2,
             final double x3, final double y3)
    {
        final double dx21 = (x2 - x1);
        ax = 0.0;              // A = 0
        bx = (x3 - x2) - dx21;  // B = P3 - P0 - 2 P2
        cx = 2.0 * dx21;       // C = 2 (P2 - P1)
        dx = x1;                // D = P1
        dax = 0.0;
        dbx = 2.0 * bx;

        final double dy21 = (y2 - y1);
        ay = 0.0;
        by = (y3 - y2) - dy21;
        cy = 2.0 * dy21;
        dy = y1;
        day = 0.0;
        dby = 2.0 * by;
    }

-------------

PR: https://git.openjdk.java.net/jdk/pull/6227