On Mon, 12 Dec 2022 16:51:01 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> This includes a fix for the precision problem we've found as part of the 
>> graphics warnings clean ups.
>> 
>> I've included two commits, one with just the minimal fix, and one with the 
>> clean ups. I can drop off the 2nd commit if it is deemed to be of no added 
>> value.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/RotateGestureRecognizer.java
>  line 60:
> 
>> 58:             String s = 
>> System.getProperty("com.sun.javafx.gestures.rotate.threshold");
>> 59:             if (s != null) {
>> 60:                 rotationThresholdDegrees = Double.valueOf(s);
> 
> should we catch a NumberFormatException and re-throw it with a meaningful 
> text message?  Or the current behavior is ok?
> 
> ditto on line 64

I suppose that would be out of scope for this PR.  I agree though that it would 
be good to mention that the double that couldn't be parsed is the one we got 
from `System.getProperty("com.sun.javafx.gestures.rotate.threshold")` as 
otherwise the error would mean nothing to anyone.

> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/ScrollGestureRecognizer.java
>  line 252:
> 
>> 250:                         factorX = deltaX / scrollMagnitude;
>> 251:                         factorY = deltaY / scrollMagnitude;
>> 252:                         initialInertiaScrollVelocity = scrollMagnitude 
>> / nanosPassed * NANOS_TO_SECONDS;
> 
> is this correct?  shouldn't it be `scrollMagnitude / (nanosPassed * 
> NANOS_TO_SECONDS)`?

I think it's correct, unless you have reason to believe why it wouldn't be.  
`scrollMagnitude` is a double, and the division and multiplication that follow 
it will be all done as doubles.

> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/ZoomGestureRecognizer.java
>  line 277:
> 
>> 275: 
>> 276:                         if (nanosPassed > 
>> INITIAL_VELOCITY_THRESHOLD_NANOS) {
>> 277:                             initialInertiaZoomVelocity = 
>> (totalZoomFactor - prevTotalZoomFactor) / nanosPassed * NANOS_TO_SECONDS;
> 
> `(totalZoomFactor - prevTotalZoomFactor) / (nanosPassed * NANOS_TO_SECONDS);` 
> ?

The order is irrelevant, it could as well be `((totalZoomFactor - 
prevTotalZoomFactor) / nanosPassed) * NANOS_TO_SECONDS` -- I'll let the 
compiler decide what is more optimal.

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

PR: https://git.openjdk.org/jfx/pull/966

Reply via email to