RFR: 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box [v2]
Jeremy
duke at openjdk.java.net
Fri Nov 5 04:41:34 UTC 2021
> 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
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/6227/files
- new: https://git.openjdk.java.net/jdk/pull/6227/files/0da9594d..5a0e2bd0
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6227&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6227&range=00-01
Stats: 84 lines in 2 files changed: 6 ins; 26 del; 52 mod
Patch: https://git.openjdk.java.net/jdk/pull/6227.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6227/head:pull/6227
PR: https://git.openjdk.java.net/jdk/pull/6227
More information about the client-libs-dev
mailing list