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

Jeremy duke at openjdk.java.net
Fri Nov 12 05:47:12 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 three additional commits since the last revision:

 - 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box
   
   This adds a new unit test that calculates a high-precision bounding box (using BigDecimals), and then makes sure our double-based logic contains that high-precision bounds.
   
   This restores getBounds2D() to its original contract: it should only ever be *larger* than the actual bounds -- it should never be smaller.
   
   Also we want to only apply this margin (aka "padding") when we deal with polynomial-based extrema. We should never apply it to line-based polygons. For ex: a Path2D that represents an int-based rectangle should return the same bounds as before 8176501 was addressed.
   
   This test currently only addresses very small cubic curves.
   
   I experimented with very large cubic & quadratic curves, but I didn't come up with a unit test that failed before and after this commit. Adding unit tests for large curve segments is a possible area of improvement.
 - 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box
   
   Addressing code review comments: given current code structure we don't need separate data structures for x and y equations.
 - 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box
   
   Removing accidental leftover code. This should have been removed in a recent previous commit. The preceding code already defines these values.

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6227/files
  - new: https://git.openjdk.java.net/jdk/pull/6227/files/4b9d87d6..40bda064

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6227&range=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6227&range=05-06

  Stats: 453 lines in 2 files changed: 112 ins; 257 del; 84 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