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

Kevin Rushforth kcr at openjdk.org
Tue Nov 29 17:42:33 UTC 2022


On Mon, 28 Nov 2022 13:39:05 GMT, John Hendrikx <jhendrikx at 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


More information about the openjfx-dev mailing list