RFR: 8345261: Refactor the Dimension2D classes
John Hendrikx
jhendrikx at openjdk.org
Thu Mar 6 08:55:06 UTC 2025
On Thu, 6 Mar 2025 02:50:33 GMT, Nir Lisker <nlisker at openjdk.org> wrote:
> The int-based Dimension is only used by Rectangle even if in another class
I've taken another look. Rectangle is not using this class at all. I think it would be much better to give the GTK screencast logic its own little int/int record (not necessarily called anything like dimension). Just define it package private or as an inner type of `TokenItem`. This code really has no relation at all to the rest of FX (it is even GTK specific).
Alternatively (which may be even better), avoid the creation of a dimension and just call `hasAllScreensOfSameSize(dimensions)` with the actual rectangles and have `TokenItem` figure it out using that information.
As for the `float` version of `Dimension2D`, I think we don't need this. Just use the `double` one; everything in FX is doubles, and to be honest, the conversion we sometimes do to floats is a precision loss risk.
So, if it is up to me:
- Don't create any new dimension types; FX uses doubles, down casting to floats is not without risk. Int variants are not needed in core FX code.
- Have TokenItem accept Rectangles in its `hasAllScreensOfSameSize` function. I think you can change the logic to avoid creating a dimension class at all, or you can make a tiny `record ScreenSize(int w, int h)` defined as an inner class or even method class to do what it is currently doing.
- Delete `com.sun.javafx.geom.Dimension` completely
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1653#issuecomment-2703192504
More information about the openjfx-dev
mailing list