On Fri, 15 Aug 2025 15:24:27 GMT, Kevin Rushforth <k...@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
