On Tue, 5 Aug 2025 14:18:01 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> ### Description >> This is the implementation of new graphics rendering pipeline for JavaFX >> using Metal APIs on MacOS. >> We released two Early Access (EA) builds and have reached a stage where it >> is ready to be integrated. >> Default rendering pipeline on macOS has not been changed by this PR. OpenGL >> still stays as the default rendering pipeline and Metal rendering pipeline >> is optional to choose (by providing `-Dprism.order=mtl`) >> The `-Dprism.verbose=true` option can be used to verify the rendering >> pipeline in use. >> >> ### Details about the changes >> >> **Shader changes** >> - MSLBackend class: This is the primary class that parses (Prism and Decora) >> jsl shaders into Metal shaders(msl) >> - There are a few additional Metal shader files added under directory : >> modules/javafx.graphics/src/main/native-prism-mtl/msl >> >> **Build changes** - There are new tasks added to build.gradle for >> - Generation/ Compilation/ linking of Metal shaders >> - Compilation of Prism Java and Objective C files >> >> **Prism** - Prism is the rendering engine of JavaFX. >> - Added Metal specific Java classes and respective native implementation >> which use Metal APIs >> - Java side changes: >> - New Metal specific classes: Classes prefixed with MTL, are added here : >> modules/javafx.graphics/src/main/java/com/sun/prism/mtl >> - Modification to Prism common classes: A few limited changes were >> required in Prism common classes to support Metal classes. >> - Native side changes: >> - New Metal specific Objective C implementation is added here: >> modules/javafx.graphics/src/main/native-prism-mtl >> >> **Glass** - Glass is the windowing toolkit of JavaFX >> - Existing Glass classes are refactored to support both OpenGL and Metal >> pipelines >> - Added Metal specific Glass implementation. >> >> ### Testing >> - Testing performed on different hardware and macOS versions. >> - HW - macBooks with Intel, Intel with discrete graphics card, Apple >> Silicon (M1, M2 and M4) >> - macOS versions - macOS 13, macOS 14 and macOS 15 >> - Functional Testing: >> - All headful tests pass with Metal pipeline. >> - Verified samples applications: Ensemble and toys >> - Performance Testing: >> - Tested with RenderPerfTest: Almost all tests match/exceed es2 >> performance except a few that fall a little short on different hardwares. >> These will be addressed separately (there are open bugs already filed) >> >> ### Note to reviewers : >> - Providing `-Dprism.order=mtl` option is a must for testing the Metal >> pipeline >> - It woul... > > Ambarish Rapte has updated the pull request incrementally with one additional > commit since the last revision: > > nir: review comments Most of the cleanup changes look good. I left an inline comment about one that I think needs to be reverted. modules/javafx.graphics/src/main/java/com/sun/prism/mtl/MTLTexture.java line 133: > 131: arr = new byte[buf.remaining()]; > 132: buf.get(arr); > 133: } I instrumented the code, and found that `updateTextureByte` is sometimes called with a direct byte buffer, which means that `buf.hasArray()` returns false. In the cases I tested, the format is `BYTE_ALPHA`, so without your change it will correctly call the native method with a null array and the direct byte buffer. This will cause the native method to use the optimized path. With this change, we now create a new byte array, copy from the direct byte buffer to that array, and pass that to the native method. This isn't what we want. I think you should revert this change, and move the array allocation / copy code to the cases that actually need it. The revert needs to happen now. The allocation / copy code could be done either now or via a follow-up bug. ------------- PR Review: https://git.openjdk.org/jfx/pull/1824#pullrequestreview-3088903402 PR Review Comment: https://git.openjdk.org/jfx/pull/1824#discussion_r2254777222
