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

[Andy Goryachev]

On Tue, 22 Nov 2022 21:09:50 GMT, Kevin Rushforth <kcr 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
>
> 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.

100.0 is a double, so the result is also a double

> 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.

Eclipse knows it is unnecessary.  Personally, I am in favor of removing such unnecessary code.

> 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).

rotationStartTime is a double.  the type cast is really unnecessary

> 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.

good catch - I think the change is not equivalent.  It should be either

4L * physicalWidth * physicalHeight;


or


((long)physicalWidth) * physicalHeight * 4L;


the proposed change might result in overflow bc the first intermediate result is likely to be int.

> 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

4L * w * h;

> 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.

index_y is a float, so the typecast is unnecessary

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

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