RFR: 8271024: Implement macOS Metal Rendering Pipeline

[Martin Fox]

On Wed, 18 Jun 2025 10:16:41 GMT, Jayathirth D V <jdv at openjdk.org> wrote:

>> 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 NSWindowSt...
>
> @beldenfox Thanks for your inputs.
> 
> 1.  Regarding adding comments in GlassView/Layer classes about how they are not subclasses but subViews/subLayers : Sure i will go ahead and add comments in the class. Also as you mentioned we can name them better:
> 
> - GlassView3D ->  GlassViewEventHandler(Since it is common class which handles events for both OpenGL and Metal pipeline) Or GlassSubView(Since it is added as subView in GlassHostView in GlassView.m). Please suggest if you have better names that we can use here.
> - GlassViewCGL3D -> GlassViewCGL(With comment mentioning how it is not subclass but a subView of GlassView3D)
> - GlassViewMTL3D -> GlassViewMTL(With comment mentioning how it is not subclass but a subView of GlassView3D)
> - At other places also i will remove 3D in names and appropriate comment in CGL/MTL layer class.
> 
> 2. Regarding NSOpenGLView : Since both CGL and MTL views are not subclass of same NSView we have current structure with Event handling in common class GlassView3D and CGL & MTL Views as subViews of GlassView3D. I also tried removing NSOpenGLView reference and then running Ensemble8 in ES2 pipeline and things work fine. But we want to minimize changes related to already present default ES2 pipeline in this PR as part of refactoring effort. We can work on it in as a follow-up issue.
> 
> 3. Regarding change in MacOSXWindowSystemInterface.m : Same here we want to minimize changes related to ES2 in this PR. We can work on it in as a follow-up issue.
> 
> 4. Regarding setOpaque() in GlassWindow.m : While refactoring Glass for Metal pipeline under [JDK-8325379](https://bugs.openjdk.org/browse/JDK-8325379) this was identified as follow up issue : [JDK-8356758](https://bugs.openjdk.org/browse/JDK-8356758). We were aware of how this code related to [JDK-8095004](https://bugs.openjdk.org/browse/JDK-8095004) but thanks for pointing to [JDK-8095040](https://bugs.openjdk.org/browse/JDK-8095040) which was not verified. There are no regression seen with this refactoring, PickTest3D works properly in both ES2 and MTL pipeline of this PR and matches output seen with mainline code. I verified that test mentioned in [JDK-8095040](https://bugs.openjdk.org/browse/JDK-8095040) on latest mainline code and i still see that black filling is drawn. Same behaviour is seen with ES2 and MTL pipeline with this PR. But it works properly in 8u431, looks like the issue has resurfaced so i have created new bug : [JDK-8359904](https://bugs.openjdk.org/browse/JDK
 -8359904)
> 
> 5. Regard...

@jayathirthrao Thanks for the thorough response. And, again, it's great to see Metal happening.

> * GlassView3D ->  GlassViewEventHandler(Since it is common class which handles events for both OpenGL and Metal pipeline) Or GlassSubView(Since it is added as subView in GlassHostView in GlassView.m). Please suggest if you have better names that we can use here.

I want to keep the GlassView prefix since these classes implement the platform side of the View class in the toolkit. If we were to rename anything it would be GlassViewCGL and GlassViewMTL since these are more closely related to Prism than the toolkit View class. But I'm overthinking things. For now I think we should keep GlassView and GlassView3D along with GlassViewCGL and GlassViewMTL and just clarify what's going on in the comments. At some point I want to combine GlassView and GlassView3D into one class but that can wait for a follow-up issue.

> 4. Regarding setOpaque() in GlassWindow.m

In my testing the setOpaque call isn't happening in this PR since the layer hasn't been created yet. But it doesn't seem to matter (?)

I'm having a hard time reproducing the original PickTest3D bug in the master branch. The screenshots in the bug report show translucency but the original bug report doesn't mention it and the PickTest3D code doesn't call for it. I tweaked PickTest3D to try a few combinations of setOpacity and DECORATED/TRANSPARENT and that setOpaque call didn't seem to make any difference even in the master branch. I must be missing something.

As for the UNIFIED issue I don't think there's a bug. The implementation relied on the OS to draw a brushed metal background but Apple removed that years ago and now just draws black. I will take a look but only because I want to understand how this works; we might need similar logic if we want to support desktop translucency effects. UNIFIED is deprecated and doesn't work on Windows either so we should probably close the new bug as Never Fix.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1824#issuecomment-2985583069