On Wed, 30 Nov 2022 03:19:54 GMT, Nir Lisker <nlis...@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
