On Mon, 28 Nov 2022 13:39:05 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be autoboxed) >> - Remove unnecessary semi-colons (at end of class definitions, or just >> repeated ones) >> - Remove redundant super interfaces (interface that is already inherited) >> - Remove unused type parameters >> - Remove declared checked exceptions that are never thrown >> - Add missing `@Override` annotations > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix indentation There is at least one place where you removed a cast in such a way that the modified code is not equivalent. That change that _must_ be reverted before this PR can be accepted. Additionally, I request a change to the time calculation in the gesture recognizer classes. See inline comments for details. modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/RotateGestureRecognizer.java line 306: > 304: sendRotateEvent(false); > 305: angleReference = newAngle; > 306: double timePassed = (time - rotationStartTime) / > 1000000000; Ditto here and in the other gesture recognizers. 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 `2147483647` and `d2` will be `-2147483647`. 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. ------------- Changes requested by kcr (Lead). PR: https://git.openjdk.org/jfx/pull/960
