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.

@prrace how to proceed ? Could you manage the CSR in JBS ? I never had to do it.

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

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