RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

[Kevin Rushforth]

On Wed, 30 Nov 2022 03:19:54 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/WindowStage.java line 418:
>> 
>>> 416:                     scaleFactor = 1.0 / scaleDivider;
>>> 417:                     adjw = (int)Math.round(iw / scaleDivider);
>>> 418:                     adjh = (int)Math.round(ih / scaleDivider);
>> 
>> Same comment here about the old code being clearer.
>
> `scaleDivider` is defined just 2 lines above as a `double`. I don't see how the cast helps here.

Yes, this one is fine.

>> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/WindowStage.java line 422:
>> 
>>> 420:                 }
>>> 421:                 double similarity = (width - adjw) / (double) width +
>>> 422:                     (height - adjh) / (double) height + //Large padding is bad
>> 
>> This change _must_ be reverted. There are cases where doing integer arithmetic on intermediate results is not equivalent to doing double arithmetic.
>> 
>> Consider this:
>> 
>> 
>> int i = Integer.MAX_VALUE;
>> int j = -1;
>> double d1 = (double) i - (double) j;
>> double d2 = i - j;
>> 
>> 
>> `d1` will be `2147483648` and `d2` will be `-2147483648`.
>> 
>> I can't say whether it is possible in practice, but this change is not acceptable, and is a good example of the general concern I raised.
>
> If the casts in the numerator actually matter, then the cast in the denominator can be removed. The latter are the ones that Eclipse flags for me as unnecessary.

I still think that any change here would be a very low value change. If done incorrectly, as it was in the initial attempt, it can introduce bugs. Even if done correctly, I see no point in it.

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

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