RFR: 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box [v9]
Laurent Bourgès
lbourges at openjdk.java.net
Fri Nov 19 07:11:43 UTC 2021
On Wed, 17 Nov 2021 08:53:07 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 code review feedback: "I would prefer having accumulate functions called twice (using offset)"
>
> https://github.com/openjdk/jdk/pull/6227#issuecomment-970040098
test/jdk/java/awt/geom/Path2D/GetBounds2DPrecisionTest.java line 254:
> 252: res[roots++] = q.divide(a, RoundingMode.HALF_EVEN);
> 253: if (!q.equals(BigDecimal.ZERO)) {
> 254: res[roots++] = c.divide(q, RoundingMode.HALF_EVEN);
This divide operation suffers missing decimals:
Use BigDecomal.setScale(40) to ascertain precision.
See https://github.com/bourgesl/marlin-math/blob/c9bbfbac4565e81e54ce79fc02c38fcaa2fe0e48/src/main/java/test/FindExtremaAccuracyTest.java#L667
See cubic test fixed:
https://github.com/bourgesl/marlin-math/blob/main/results/findExtrema/jdk17-2021-11-19-dist-cubic.log
-------------
PR: https://git.openjdk.java.net/jdk/pull/6227
More information about the client-libs-dev
mailing list