RFR: 8350479: SW pipeline should use default pipeline in Glass [v2]

Jayathirth D V jdv at openjdk.org
Mon Aug 18 07:30:14 UTC 2025


On Fri, 15 Aug 2025 15:24:27 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Jayathirth D V has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Pick default pipeline from PrismSettings
>
> 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.

Thanks, updated.

> 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.

I also had doubts about how to include "SW" in the name. Because it would be very long name.

I have removed the references to "Glass" in the name when we are already in glass code and updated these flags to use "SW" in the name.

> 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?

Sure updated.

> 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.

We are following https://google.github.io/styleguide/objcguide.html#method-invocations & https://google.github.io/styleguide/objcguide.html#method-declarations-and-definitions. So i have also updated the code where it is inconsistent and we are touching in this PR.

Please correct me if i am wrong.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2281515412
PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2281519271
PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2281515800
PR Review Comment: https://git.openjdk.org/jfx/pull/1868#discussion_r2281522820


More information about the openjfx-dev mailing list