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

Phil Race prr at openjdk.java.net
Wed Apr 27 18:22:56 UTC 2022


On Fri, 4 Mar 2022 22:38:33 GMT, Jeremy <duke at 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



More information about the client-libs-dev mailing list