On Thu, 12 Jun 2025 05:41:23 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 would be a good idea to test both OpenGL and Metal pipelines
> - Known issues and tasks ...
Metal is here! Yes!
It took me a while to sort out how the GlassView and GlassLayer classes work in
this PR. It’s not clear based on the naming that the classes which handle
drawing (GlassViewCGL3D and GlassViewMTL3D ) are not subclasses of the class
that handles events (GlassView3D). The naming conventions could be clearer or
at the very least there should be some comments in the code.
I think some of this structure is being dictated by the way GlassViewCGL3D is
derived from NSOpenGLView since that prevents us from making it a subclass of,
say, GlassView3D. But I’m almost certain the NSOpenGLView reference is just
cruft. All of the OpenGL processing happens in a CAOpenGLLayer which has
nothing to do with the (unused) NSOpenGLView machinery. In the current master
and this PR I can derive from NSView instead of NSOpenGLView and everything
works fine.
There is some code in Prism (MacOSXWindowSystemInterface.m) that can associate
an NSOpenGLContext with an NSView but I don’t think a view is ever passed in.
It would be nice to clean that code up since the view-related calls are
deprecated and generating compiler warnings.
I think the NSOpenGLView reference should be removed. Beyond that you could
keep the current class structure and view layout since there’s something to be
said for separating drawing and event processing into different NSViews.
I would rather not see the 3D terminology carried over into the new classes.
It's there to make a distinction that's no longer relevant (according to the
comments there used to be a GlassView2D). But that is a personal pet peeve so
feel free to ignore it.
Look like it’s not possible to add comments to unchanged code in GitHub (?!) so
I guess I have to write the following issues up here.
In GlassWindow.m line 807 there’s this bit of mysterious code:
CALayer *layer = [window->view layer];
if (([layer isKindOfClass:[CAOpenGLLayer class]] == YES) &&
(([window->nsWindow styleMask] & NSWindowStyleMaskTexturedBackground) ==
NO))
{
[((CAOpenGLLayer*)layer) setOpaque:[window->nsWindow isOpaque]];
}
It was put there to resolve
[JDK-8095004](https://bugs.openjdk.org/browse/JDK-8095004) and
[JDK-8095040](https://bugs.openjdk.org/browse/JDK-8095040). In this PR the
layer is nil at this point so this code isn’t doing anything. I haven’t
verified whether this is causing a regression. In the master branch that
setOpaque: call will always happen since the
NSWindowStyleMaskTexturedBackground bit is never set (it’s associated with the
obsolete UNIFIED stage style).
Over in GlassView.m there’s a JNI method called _getNativeLayer that retrieves
a CALayer. There’s no need to update this logic since the Java side is never
called (it’s in MacView.java). This should all just be removed.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1824#pullrequestreview-2932698156
