On Sun, 13 Sep 2026 12:29:43 GMT, Marius Hanl <[email protected]> wrote:
>> This PR integrates the `Platform` functionality into `PlatformUtil`, so that >> we only have one place where we do OS stuff. And can delete `Platform`. >> >> Something that was first discussed in >> https://github.com/openjdk/jfx/pull/1864 and I also saw when I did the >> cleanup some months ago in PR https://github.com/openjdk/jfx/pull/2190. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Marius Hanl has updated the pull request incrementally with three additional > commits since the last revision: > > - rename to GlassPlatform, improve code a bit > - revert back > - Revert "Move PlatformUtil from javafx.base to javafx.graphics" > > This reverts commit e41e8ed504b9ac54d8be7646108151f5aeff07d3. I left one comment pointing out a bug inline. Here are my overall comments. I also like the name `GlassPlatform` for the renamed glass `Platform` class. I think the following methods should move to the `javafx.graphics` module at some point: * `isHeadless()` -- Used by tests to check whether the glass platform is headless * `useEGL()` -- Used in `javafx.graphics` by Prism and Quantum to check whether we are using EGL * `isEmbedded()` -- Mostly used in `javafx.graphics` with one use in `javafx.controls` in `TitledPaneSkin` (line 75) * `getEmbeddedType()` -- Used in `javafx.graphics` only (Prism/ES2) I confirmed that we do not use any of the above 4 methods in our closed repo. You will need to make `GlassPlatform` public when you move these 4 methods, since they are accessed outside of `com.sun.glass.ui`. One thing to consider regarding `isEmbedded()` is that doing a qualified export of `com.sun.glass.ui` to `javafx.controls` is not an ideal choice, so you might consider a utility class in a different `com.sun` package in `javafx.graphics` that delegates to `GlassPlatform`. Moving the 4 methods can be done either as part of this PR or in a follow-up. I might lean towards doing it now, even though it will take more time, since this PR as it stands doesn't achieve the original goal of consolidating the platform checks or the desired separation of the OS checks from the graphics-related checks. There is another subtle point to be aware of: `PlatformUtil` loads a properties file when it is first initialized and sets the system properties found in that file if they aren't already set (so that a system property set on the command line takes precedence). When moving the above 4 methods to `GlassPlatform`, be sure to initialize the `PlatformUtil` class before reading the system properties. Speaking of which, there is an existing order dependency bug that should be fixed at the same time. The existing glass `Platform` class checks the `glass.platform` property before referencing `PlatformUtil`. Unless the `PlatformUtil` class happens to be initialized before `Platform.determinePlatform()` is called, the setting in the property file will be ignored. modules/javafx.graphics/src/main/java/com/sun/glass/ui/GlassPlatform.java line 52: > 50: case "headless" -> HEADLESS; > 51: default -> userPlatform; > 52: }; You need the restore the `return type;` here or else it will be overwritten below (making the switch useless). ------------- PR Review: https://git.openjdk.org/jfx/pull/2299#pullrequestreview-5198109317 PR Review Comment: https://git.openjdk.org/jfx/pull/2299#discussion_r4005533864
