Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v4]

[Jayathirth D V]

On Thu, 11 Feb 2021 05:41:15 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:

>> According to Xcode Instruments leak profile, there are 2 minor memory leaks 
>> in the Metal rendering pipeline:
>> 
>> `#1 Malloc 80 Bytes  1       0x7fde0d4247b0  80 Bytes        libjava.dylib   
>> getStringUTF8`
>>    0 libsystem_malloc.dylib malloc_zone_malloc
>>    1 libsystem_malloc.dylib malloc
>>    2 libjava.dylib getStringUTF8 
>> /Volumes/Work/review/2403/jdk/src/java.base/share/native/libjava/jni_util.c:888
>>    3 libjava.dylib JNU_GetStringPlatformChars 
>> /Volumes/Work/review/2403/jdk/src/java.base/share/native/libjava/jni_util.c:917
>>    4 libawt_lwawt.dylib 
>> Java_sun_java2d_metal_MTLGraphicsConfig_getMTLConfigInfo 
>> /Volumes/Work/review/2403/jdk/src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m:151
>>    5  0x11ab08d48
>>    6  0x11ab0250d
>> 
>> `#2 Malloc 80 Bytes  1       0x7fde0d4325a0  80 Bytes        libjava.dylib   
>> getStringUTF8`
>>    0 libsystem_malloc.dylib malloc_zone_malloc
>>    1 libsystem_malloc.dylib malloc
>>    2 libjava.dylib getStringUTF8 
>> /Volumes/Work/review/2403/jdk/src/java.base/share/native/libjava/jni_util.c:888
>>    3 libjava.dylib JNU_GetStringPlatformChars 
>> /Volumes/Work/review/2403/jdk/src/java.base/share/native/libjava/jni_util.c:917
>>    4 libawt_lwawt.dylib 
>> Java_sun_java2d_metal_MTLGraphicsConfig_tryLoadMetalLibrary 
>> /Volumes/Work/review/2403/jdk/src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m:120
>>    5  0x11ab08d48
>>    6  0x11ab024c8
>> 
>> Those can be handled as a followup issues though if you like, it's only 160 
>> bytes total.
>
>> I tried to code review the native implementation files, but Metal APIs is 
>> brand new to me and it's been a long while since I worked with graphics API, 
>> so I can't be of much help really.
>> 
>> The code I've looked at looked clean and nothing caught my eye. But it's a 
>> partial code review at best.
>> 
>> Good job!
> 
> Every bit helps. Thanks for your review effort!

> A small thing caught my eye when I was comparing OpenGL and Metal pipelines 
> running J2Demo.jar
> 
> It looks to me like OpenGL rendering pipeline uses brighter colors. I prefer 
> the more subdued colors that Metal is rendering, but not sure if this is 
> something you want to investigate further.
> 
> I tried to capture what I see below:
> 
> OpenGL:
> 
> <img alt="OpenGL" width="1128" 
> src="https://user-images.githubusercontent.com/63425797/107575744-dc0ca180-6bb5-11eb-9eb3-5cff415eb8a3.png";>
> 
> Metal (the green color has a yellowish tint):
> 
> <img alt="Metal" width="1125" 
> src="https://user-images.githubusercontent.com/63425797/107575716-cf884900-6bb5-11eb-8ae8-14212ec94e87.png";>
> 
> Performance wise I did not see much difference either way.

@gerard-ziemski Thanks for verifying the behavior.
Text present in Memory monitor/Performance in J2DDemo is not drawn using Metal 
rendering pipeline(drawGlyphList or any other rendering opcodes, I have 
verified it again in latest code). This is drawn using software renderloops and 
only thing happens from Metal pipeline is this content is used in blit opcode. 
Slight difference in color that we are seeing is related to how Metal 
internally handles blending of colors.

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

PR: https://git.openjdk.java.net/jdk/pull/2403