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

[Laurent Bourgès]

On Wed, 5 Jan 2022 22:48:37 GMT, Phil Race <prr at openjdk.org> wrote:

>> 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
>>   
>>   This is a second follow-up response to prrace's code review feedback about method modifiers.
>>   
>>   This commit more carefully preserves the getBounds2D() method modifiers for all 3 classes: the Path2D.Double, the Path2D.Float, and the Path2D itself.
>>   
>>   It is possible (if unlikely) that someone previously extended the Path2D class and overrode getBounds2D(), because the Path2D#getBounds2D() method was *not* final. So with this commit: any such existing code will not break. Meanwhile the subclasses (Double and Float) preserve their previous modifiers (final, synchronized).
>>   
>>   This is in response to prrace's code review:
>>   
>>   >    You are changing the signature of public API
>>   >    src/java.desktop/share/classes/java/awt/geom/Path2D.java
>>   >    public final synchronized Rectangle2D getBounds2D() => public Rectangle2D getBounds2D() {
>>   >
>>   > So no longer final, and no longer synchronized.
>>   > This means a CSR is required and we need to think about it ..
>>   > the intention was that the subclass not over-ride.
>>   > And why remove synchronized ? I am fairly sure it was there to
>>   > make sure no one was mutating the Path whilst
>>   > bounds are being calculated.
>>   > And you are using getPathIterator(AffineTransform) and the docs
>>   > for that say it isn't thread safe.
>>   > So I think this implementation needs to be thought about very carefully.
>
> I still recommend a CSR since
> 1) Although "compatible", this visibly moves the specs for getBounds2D() from copies on the Float and Double nested sub-classes to the enclosing parent class
> 2) Although in the spirit of what getBounds2D() is supposed to do  .. it actually hasn't for over 20 years. We like to document such behavioural changes via a CSR.
> 
> Actually regarding (1) there's also a real change there too since now The Float subclass will return a Rectangle2D.Double .. I am not sure that we should actually do that.

This work is great for jdk19, 25 years after java 1.2 providing the java2d API, to get more precise curve bounding boxes (closer to extrema), let's write the CSR altogether, @prrace

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

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