On Wed, 23 Nov 2022 16:49:50 GMT, Andy Goryachev <ango...@openjdk.org> 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 >> information. If the goal is to make it clear in the diff, then many places >> would benefit from explicit casts (probably in more places than where they >> were removed). I think the general trend is to avoid explicit casts, but I >> could be wrong. >> >> I can revert these changes and we can disable 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()`) as the conversion from `long` to `double` will lose >> precision in the area where it really counts as it wants to see the >> difference of the current and last value, so in this case the warning may >> have exposed a (precision) bug. > > +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()`) as the conversion from `long` to `double` will lose > precision in the area where it really counts as it wants to see the > difference of the current and last value, so in this case the warning may > have exposed a (precision) bug. You may be right. In this case, I would recommend either reverting this change, or else adding a `.0` to the divisor along with the removal of the explicit cast. Either way, we should consider a follow-up bug to address the question of possible loss of precision (in the existing code, which is unchanged by the refactoring). ------------- PR: https://git.openjdk.org/jfx/pull/960
