On Wed, 5 Jan 2022 22:48:37 GMT, Phil Race <p...@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
