On Fri, 4 Mar 2022 22:38:33 GMT, Jeremy <d...@openjdk.java.net> 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 don't suppose any new client-lib members would be willing to set up a CSR
> for this PR?
>
> No worries if not. I just wanted to double-check before this PR gets
> auto-closed.
>
> (I'm under the impression I can't initiate a CSR and there's nothing else I
> can do here; if I'm wrong please let me know.)
This is waiting for @mickleness to type "/integrate", so I can follow it up
with "/sponsor"
I don't think it can get pushed unless the fixer does that /integrate.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6227
