On Thu, 14 Aug 2025 07:26:04 GMT, Jayathirth D V <j...@openjdk.org> wrote:
>> We have Metal pipeline also now in macOS apart from OpenGL pipeline for
>> accelerated hardware pipelines.
>>
>> Currently when Software pipeline is used we continue to use OpenGL(ES2)
>> pipeline on glass side, which is the default hardware pipeline in macOS. If
>> we want to switch to Metal pipeline as default hardware pipeline in prism,
>> we should be able to use Metal pipeline in glass also when software pipeline
>> is used. For this to happen we need to set some information when SWPipeline
>> is getting initialized and pass that information to glass.
>>
>> This PR adds new flag `useMTLInGlass` in device details when SWPipeline is
>> getting initialized and then passes it to glass code. When SWPipeline is
>> used, based on this flag we will use appropriate hardware pipeline in glass.
>>
>> Patch is tested when useMTLInGlass is set to true/false and then using
>> -Dprism.order=es2/mtl/sw and it picks proper GlassView/Layer.
>
> Jayathirth D V has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Pick default pipeline from PrismSettings
Looks good with a few minor comments. I'll reapprove if you make any changes as
a result.
@arapte can you be the second reviewer?
modules/javafx.graphics/src/main/java/com/sun/prism/sw/SWPipeline.java line 78:
> 76: if (PlatformUtil.isMac()) {
> 77: HashMap devDetails =
> 78: (HashMap)SWPipeline.getInstance().getDeviceDetails();
You're already in an instance method of SWPipeline, so the
`SWPipeline.getInstance().` is not needed, and seems odd.
modules/javafx.graphics/src/main/native-glass/mac/GlassLayer.h line 40:
> 38: withHiDPIAware:(BOOL)HiDPIAware
> 39: withIsSwPipe:(BOOL)isSwPipe
> 40: useMTLForBlit:(BOOL)useMTLInGlass;
Minor question on the naming: Since this is only ever set to true when using
the SW pipeline, do you think having "Sw" somewhere in the name would be
helpful? Given the name, it could be confusing to notice that it is set to
false when using the MTL pipeline.
modules/javafx.graphics/src/main/native-glass/mac/GlassLayer.m line 55:
> 53: {
> 54: if (mtlCommandQueuePtr != 0l ||
> 55: useMTLInGlass) { // MTL
Minor: perhaps this would look better with the "if" test all on the same line?
modules/javafx.graphics/src/main/native-glass/mac/GlassViewCGL.m line 178:
> 176: withHiDPIAware:isHiDPIAware
> 177: withIsSwPipe:isSwPipe
> 178: useMTLForBlit:useMTLInGlass];
Minor: indentation isn't consistent.
-------------
Marked as reviewed by kcr (Lead).
PR Review: https://git.openjdk.org/jfx/pull/1868#pullrequestreview-3124193962
PR Comment: https://git.openjdk.org/jfx/pull/1868#issuecomment-3192031026
PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2279243883
PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2279382183
PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2279339570
PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2279385548
