On Wed, 18 Jun 2025 21:48:22 GMT, Martin Fox <m...@openjdk.org> wrote:
>> @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. > >> I'm having a hard time reproducing the original PickTest3D bug in the master >> branch. > > Spoke too soon. PickTest3D has been tweaked since the original Mac bug was > posted and resolved. @beldenfox Found out the root cause for why we are seeing layer as nil GlassWindow.m. We have additional level of abstraction for views and we set layer for GlassViewCGL/MTL and not for GlassView3D. We already have change in GlassView.m->getGlassView() to get GlassView3D object and then call getLayer() on it. We need to make similar change in GlassWindow.m->getMacView(), with this change we are able to get appropriate layer by calling getLayer(). This change is made and testing is in progress. Also what i am also noticing is setOpaque() in GlassWindow.m is never called on OpenGL pipeline when i run Ensemble8 demos, PickTest3D or test present in [JDK-8095040](https://bugs.openjdk.org/browse/JDK-8095040). I will keep this OpenGL specific setOpaque call since we don't want to make changes related to OpenGL in this PR. But i will add comment about refactoring and moving it to CGL specific class in future. Regarding classname change, i will keep GlassView3D name as it is but remove 3D references in other classes. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1824#issuecomment-2988208764
