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