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

[Kevin Rushforth]

On Tue, 22 Nov 2022 18:39:43 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

I spot checked the changes. Someone else will need to review it thoroughly.

Overall, I see the following changes as non-controversial:

* removing unused imports
* adding missing `@Override`
* removing redundant semicolons
* using the diamond operator

The following might need a little more discussion in some cases:

* removing "redundant" casts -- in addition to not always being 100% equivalent (the case where a value is cast to a subclass and later assigned to a superclass), I left a few comments on places where removing redundant floating point casts makes it harder to quickly reason about whether the code is correct.
* removing instanceof (which I didn't see here, but we had discussed in another PR)

As long as the ones in this second list are checked carefully enough, this should be fine, but it isn't as quick a review as it might otherwise be.

Given the shear number of changes, a second reviewer is in order.

modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkMenuDelegate.java line 52:

> 50: 
> 51:    @Override
> 52: public boolean setPixels(Pixels pixels) {

Indentation was lost here.

modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java line 783:

> 781:                     if (simulateSlowProgress) {
> 782:                         for (int i = 0; i < 100; i++) {
> 783:                             notifyProgress(currentPreloader, i / 100.0);

Does Eclipse really flag this as a warning? It doesn't seem a particularly useful warning, since an explicit cast to double is a reasonable pattern that can improve clarity, even if it isn't needed. In this specific case it is pretty clear without the explicit cast that it will do what you want, but other places are not so clear.

modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java line 1240:

> 1238:                 cadv = advanceWidths[numHMetrics-1];
> 1239:             }
> 1240:             return ((cadv & 0xffff)*ptSize)/upem;

This is an example where I think the explicit cast makes it easier to quickly tell at a glance that it is correct, even though it happens to be unnecessary. In this case, I would need to look at the definition of `ptSize` and `upem` to know that it is unneeded, and thus correct without it.

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/RotateGestureRecognizer.java line 230:

> 228:             }
> 229:             if (ROTATION_INERTIA_ENABLED && (state == RotateRecognitionState.PRE_INERTIA || state == RotateRecognitionState.ACTIVE)) {
> 230:                 double timeFromLastRotation = (time - rotationStartTime) / 1000000;

Another place where I'm not sure removing the explicit cast is best (probably OK here).

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/UploadingPainter.java line 41:

> 39:  * The PresentingPainter is used when we are rendering to the main screen.
> 40:  */
> 41: final class UploadingPainter extends ViewPainter {

I'm not sure I see the value in making this change.

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.

modules/javafx.graphics/src/main/java/com/sun/prism/d3d/D3DTextureData.java line 45:

> 43:                                boolean hasDepth)
> 44:     {
> 45:         return (long) physicalWidth * physicalHeight * 4L;

Another case where the old code might be clearer.

modules/javafx.graphics/src/main/java/com/sun/prism/d3d/D3DVramPool.java line 59:

> 57:     {
> 58:         // REMIND: need to deal with size of depth buffer, etc.
> 59:         return (long) width * height * 4;

Ditto

modules/javafx.graphics/src/main/java/com/sun/prism/es2/ES2VramPool.java line 54:

> 52:     {
> 53:         // REMIND: need to deal with size of depth buffer, etc.
> 54:         return (long) width * height * 4;

DItto

modules/javafx.graphics/src/main/java/com/sun/prism/impl/ps/PaintHelper.java line 310:

> 308:         shader.setConstants("fractions", stopVals, 0, MULTI_MAX_FRACTIONS);
> 309:         float index_y = initGradient(paint);
> 310:         shader.setConstant("offset", index_y / MULTI_CACHE_SIZE + HALF_TEXEL_Y);

Here's another.

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

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