On Tue, 6 Dec 2022 09:00:02 GMT, John Hendrikx wrote:
>>> This is one giant pull request, somewhat difficult on the reviewers,
>>> especially since we have to go through it several times.
>>>
>>> It would be _much_ easier for the reviewers to deal with one fix per PR,
>>> especially in the cas
On Tue, 6 Dec 2022 08:54:26 GMT, John Hendrikx wrote:
>> I don't see it as a useful warning. The code isn't necessarily better
>> without, and can be less clear. I think this might argue for asking Eclipse
>> users to disable this warning in the IDE.
>
> @kevinrushforth Is this okay as it is?
On Tue, 6 Dec 2022 06:39:32 GMT, Nir Lisker wrote:
>> Since this is an internal class (not public API), I won't object to this
>> change. I do think it's a rather pointless change, but I'll leave it up to
>> you.
>
> Personally, I find that re-declaring type hierarchy is clutter. I can see why
On Tue, 6 Dec 2022 06:33:20 GMT, Nir Lisker wrote:
>> I agree with you - we shouldn't.
>
> This is an example of what I was talking about in [this
> comment](https://github.com/openjdk/jfx/pull/958#pullrequestreview-1198517697).
I agree that no follow-on bug is needed.
-
PR: https
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx 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 auto
On Sat, 3 Dec 2022 20:46:22 GMT, John Hendrikx wrote:
>> modules/javafx.graphics/src/main/java/com/sun/glass/ui/monocle/LinuxStatefulMultiTouchProcessor.java
>> line 42:
>>
>>> 40:
>>> 41: private final Map slotToIDMap =
>>> 42: new HashMap<>();
>>
>> Same line. There are more
On Wed, 30 Nov 2022 12:48:10 GMT, Kevin Rushforth wrote:
>> 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 ch
On Mon, 5 Dec 2022 22:20:59 GMT, John Hendrikx wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentations and merge short lines
>
>> This is one giant pull request, somewhat difficult on the reviewers,
>> e
On Tue, 22 Nov 2022 22:30:16 GMT, Kevin Rushforth wrote:
>> I agree. The problem is that we will not be able to enable the warning in
>> IDE, or it has to be @suppressed.
>>
>> So the choice is either fix the code and enable warning, or keep the code as
>> is and disable the warning.
>
> I do
On Wed, 30 Nov 2022 12:55:08 GMT, Kevin Rushforth wrote:
>> 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 UploadingPaint
On Wed, 30 Nov 2022 12:48:10 GMT, Kevin Rushforth wrote:
>> 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 ch
On Mon, 5 Dec 2022 22:20:59 GMT, John Hendrikx wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentations and merge short lines
>
>> This is one giant pull request, somewhat difficult on the reviewers,
>> e
On Tue, 6 Dec 2022 03:03:13 GMT, Andy Goryachev wrote:
>> The code was already broken (since 2014 apparently when the new Base64
>> decoder was used). Had the author turned on these handy warnings, they
>> would have noticed that something was amiss. Now they unwittingly changed
>> the behav
On Mon, 5 Dec 2022 22:00:01 GMT, John Hendrikx wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java
>> line 601:
>>
>>> 599: }
>>> 600: args.add("--" + k + "=" + (v != null ? v : ""));
>>> 601: idx++;
>>
>> I suspect t
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx 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 auto
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx 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 auto
On Mon, 5 Dec 2022 19:59:45 GMT, Andy Goryachev wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentations and merge short lines
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherI
On Mon, 5 Dec 2022 20:17:04 GMT, Andy Goryachev wrote:
>> I do agree with @kevinrushforth here.
>>
>> It might be borderline useful to enable the warning once just to see if
>> there are any bugs or perhaps to clean up the most obvious cases, but it
>> probably makes little sense to enable the
On Mon, 5 Dec 2022 21:40:40 GMT, John Hendrikx wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java
>> line 586:
>>
>>> 584: ascent = -(float)hhea.getShort(4);
>>> 585: descent = -(float)hhea.getShort(6);
>>> 586:
On Mon, 5 Dec 2022 21:46:12 GMT, John Hendrikx wrote:
>> modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 2579:
>>
>>> 2577: term = nextLayer(lastTerm);
>>> 2578: }
>>> 2579: return new ParsedValueImpl<>(layers,
>>> CornerRadiiConverter.getInstanc
On Mon, 5 Dec 2022 20:37:09 GMT, Andy Goryachev wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentations and merge short lines
>
> modules/javafx.graphics/src/main/java/com/sun/prism/d3d/D3DContext.java li
On Mon, 5 Dec 2022 20:11:53 GMT, Andy Goryachev wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentations and merge short lines
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.ja
On Tue, 22 Nov 2022 22:34:42 GMT, Andy Goryachev wrote:
>> I don't see it as a useful warning. The code isn't necessarily better
>> without, and can be less clear. I think this might argue for asking Eclipse
>> users to disable this warning in the IDE.
>
> I do agree with @kevinrushforth here.
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx 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 auto
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx 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 auto
> - 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 d
On Tue, 29 Nov 2022 16:01:58 GMT, Kevin Rushforth wrote:
>> `4L * physicalWidth * physicalHeight;` does not require an explicit cast, so
>> I think it's the clearest.
>
> That variant is fine with me.
Updated with this variant.
-
PR: https://git.openjdk.org/jfx/pull/960
On Tue, 29 Nov 2022 16:50:00 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentation
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/RotateGestureRecognizer.java
>
On Sat, 26 Nov 2022 18:19:00 GMT, Nir Lisker wrote:
>> John Hendrikx has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Revert changes to gesture recognizers to be addressed in follow up bug
>> - Fix incorrect cast removals
>
> modules/j
On Tue, 29 Nov 2022 16:49:30 GMT, Kevin Rushforth wrote:
>> +1 for reverting the changes and disabling the warning
>
>> Specifically about this code, `rotationStartTime` probably should not have
>> been a `double` depending on what is stored in it (checking: its source is
>> `System.nanoTime()`
> - 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 d
On Tue, 22 Nov 2022 21:28:44 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentation
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/UploadingPainter.java
> line 4
On Mon, 28 Nov 2022 13:39:05 GMT, John Hendrikx 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 aut
On Wed, 30 Nov 2022 03:19:54 GMT, Nir Lisker 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:
On Tue, 22 Nov 2022 21:30:09 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentation
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/WindowStage.java
> line 418:
>
On Tue, 29 Nov 2022 17:11:51 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentation
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/WindowStage.java
> line 422:
>
On Wed, 23 Nov 2022 16:49:50 GMT, Andy Goryachev wrote:
>> You are correct that with these kind of changes you can't see if it is
>> correct just from looking at the diff. Variables would need to be named more
>> explicitly, or explicit casts would need to be added to repeat the type
>> inform
On Tue, 22 Nov 2022 21:34:44 GMT, Kevin Rushforth wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix indentation
>
> modules/javafx.graphics/src/main/java/com/sun/prism/es2/ES2VramPool.java line
> 54:
>
>> 52:
On Mon, 28 Nov 2022 13:39:05 GMT, John Hendrikx 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 aut
On Sat, 26 Nov 2022 19:24:21 GMT, Nir Lisker wrote:
>> The cast has precedence and Java will not downcast mid calculation and lose
>> precision, so all examples here are correct.
>
> `4L * physicalWidth * physicalHeight;` does not require an explicit cast, so
> I think it's the clearest.
That
> - 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 d
On Tue, 22 Nov 2022 22:33:49 GMT, Kevin Rushforth wrote:
>> (long) width * height * 4;
>> is dangerous if one does not remember the precedence rules - is typecast
>> applied to width or the end result?
>> 4L * w * h; is unambiguous.
>
> I'm not sure I like either of the two choices you listed. B
On Wed, 23 Nov 2022 07:26:31 GMT, John Hendrikx wrote:
>> Actually, I think the new code correct, though less clear, since `(long)` is
>> applied to `physicalWidth` before the multiplication. The fact that we had
>> to scratch our head and discuss it highlights the point I was trying to
>> mak
On Tue, 22 Nov 2022 18:39:43 GMT, John Hendrikx 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 autobox
On Thu, 24 Nov 2022 00:12:04 GMT, Andy Goryachev wrote:
> It makes no sense to enable the warning and not fix all the warnings in the
> code. I would like to see 0 warnings in the problem list.
The warning is for all unnecessary casts, which removes a lot of clutter. If,
in some special cases,
On Thu, 24 Nov 2022 00:01:10 GMT, Nir Lisker wrote:
> so I don't think that the warning should be disabled.
It makes no sense to enable the warning and not fix all the warnings in the
code. I would like to see 0 warnings in the problem list.
-
PR: https://git.openjdk.org/jfx/pul
On Tue, 22 Nov 2022 18:39:43 GMT, John Hendrikx 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 autobox
On Wed, 23 Nov 2022 07:21:02 GMT, John Hendrikx wrote:
>> But you don't know that without looking. Really, though, if I were going to
>> make a readability argument, it's the divisor that should be a double
>> constant, since then there is no question that the division is happen in
>> double.
On Tue, 22 Nov 2022 18:39:43 GMT, John Hendrikx 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 autobox
On Tue, 22 Nov 2022 22:21:11 GMT, Kevin Rushforth wrote:
>> 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 over
On Tue, 22 Nov 2022 22:16:46 GMT, Kevin Rushforth wrote:
>> rotationStartTime is a double. the type cast is really unnecessary
>
> But you don't know that without looking. Really, though, if I were going to
> make a readability argument, it's the divisor that should be a double
> constant, sin
On Tue, 22 Nov 2022 22:25:12 GMT, Kevin Rushforth wrote:
>> 100.0 is a double, so the result is also a double
>
> Right. Which is why I said that in this specific case it (meaning the changed
> code) is clear without the explicit cast. So I don't object to this
> particular change. I just used
On Tue, 22 Nov 2022 20:55:56 GMT, Kevin Rushforth 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 a
On Tue, 22 Nov 2022 22:30:16 GMT, Kevin Rushforth wrote:
>> I agree. The problem is that we will not be able to enable the warning in
>> IDE, or it has to be @suppressed.
>>
>> So the choice is either fix the code and enable warning, or keep the code as
>> is and disable the warning.
>
> I do
On Tue, 22 Nov 2022 22:23:52 GMT, Andy Goryachev wrote:
>> I would just revert it back.
>
> (long) width * height * 4;
> is dangerous if one does not remember the precedence rules - is typecast
> applied to width or the end result?
> 4L * w * h; is unambiguous.
I'm not sure I like either of the
On Tue, 22 Nov 2022 22:18:54 GMT, Andy Goryachev wrote:
>> If it aids readability, I don't see it as unnecessary. In this case, since
>> the types of all of the variables are not known, you could certainly argue
>> it doesn't matter much -- you need to look at the types to reason about what
>>
On Tue, 22 Nov 2022 22:15:12 GMT, Kevin Rushforth wrote:
>> Eclipse knows it is unnecessary. Personally, I am in favor of removing such
>> unnecessary code.
>
> If it aids readability, I don't see it as unnecessary. In this case, since
> the types of all of the variables are not known, you cou
On Tue, 22 Nov 2022 22:01:10 GMT, Andy Goryachev wrote:
>> 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:
On Tue, 22 Nov 2022 21:09:50 GMT, Kevin Rushforth 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 a
On Tue, 22 Nov 2022 18:39:43 GMT, John Hendrikx 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 autobox
- 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
61 matches
Mail list logo
