Integrated: 8276905: Use appropriate macosx_version_minimum value while compiling metal shaders

2021-11-16 Thread Jayathirth D V
On Thu, 11 Nov 2021 05:52:18 GMT, Jayathirth D V  wrote:

> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

This pull request has now been integrated.

Changeset: 9a9a157a
Author:Jayathirth D V 
URL:   
https://git.openjdk.java.net/jdk/commit/9a9a157a7d45cbfb016d4427931e1d5345210f7a
Stats: 16 lines in 1 file changed: 15 ins; 0 del; 1 mod

8276905: Use appropriate macosx_version_minimum value while compiling metal 
shaders

Reviewed-by: ihse, kcr, erikj, prr

-

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


Re: RFR: 8276905: Use appropriate macosx_version_minimum value while compiling metal shaders [v4]

2021-11-15 Thread Jayathirth D V
On Mon, 15 Nov 2021 14:33:21 GMT, Magnus Ihse Bursie  wrote:

> Looks good to me also. Please let Phil have a say as well before integrating.

Thanks Magnus for the review. Yes i have already Re-Requested review from Phil 
on latest patch.
Also i am waiting on Vitaly(JBS submitter) to verify the latest patch.

-

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v3]

2021-11-15 Thread Jayathirth D V
On Mon, 15 Nov 2021 13:46:29 GMT, Kevin Rushforth  wrote:

>> Jayathirth D V has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use 10.14 macOS version
>
> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 844:
> 
>> 842: # for aarch64 is 11.00.00 at the time of introducing 
>> MACOSX_METAL_VERSION_MIN.
>> 843: ifeq ($(OPENJDK_TARGET_CPU_ARCH), xaarch64)
>> 844: MACOSX_METAL_VERSION_MIN=11.00.00
> 
> I like Erik's suggestion of using `MACOSX_VERSION_MIN` for `aarch64`, since 
> it's one less place you need to duplicate a constant.

Hi Kevin,

I noticed the comment after pushing the change. I have updated it.

Thanks,
Jay

-

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-15 Thread Jayathirth D V
On Mon, 15 Nov 2021 13:28:30 GMT, Kevin Rushforth  wrote:

> The JDK itself has a minimum version of 11.0 on macOS aarch64 systems 
> (because apple only supports it on macOS >= 11), so it probably doesn't 
> matter much. You might wish to file a low-priority follow-up issue to change 
> the runtime check to be consistent.

Thanks Kevin for the clarification and for also clearing up the follow up 
question i wanted to ask about runtime check update.

-

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v4]

2021-11-15 Thread Jayathirth D V
> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

Jayathirth D V has updated the pull request incrementally with one additional 
commit since the last revision:

  Use MACOSX_VERSION_MIN for aarch64

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6346/files
  - new: https://git.openjdk.java.net/jdk/pull/6346/files/49104160..ecea143b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=02-03

  Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v3]

2021-11-15 Thread Jayathirth D V
> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

Jayathirth D V has updated the pull request incrementally with one additional 
commit since the last revision:

  Use 10.14 macOS version

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6346/files
  - new: https://git.openjdk.java.net/jdk/pull/6346/files/7f999650..49104160

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=01-02

  Stats: 13 lines in 1 file changed: 12 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-14 Thread Jayathirth D V
On Mon, 15 Nov 2021 05:46:30 GMT, Jayathirth D V  wrote:

> " Also note that on aarch64 the global value is 11.00.00, and in that case we 
> should use the global value."
> 
> I have no idea what happens if you specify 10.14 to the metal compiler on 
> aarch64 Something else to check although probably the safest option is to 
> make it 11.0 on that architecture

A question popped up while doing this. My making 11.0 as minimum macosx version 
on aarch64, are we not restricting metal to be used only on >=11.0 on aarch 64?

-

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-14 Thread Jayathirth D V
On Fri, 12 Nov 2021 20:27:32 GMT, Phil Race  wrote:

> " Also note that on aarch64 the global value is 11.00.00, and in that case we 
> should use the global value."
> 
> I have no idea what happens if you specify 10.14 to the metal compiler on 
> aarch64 Something else to check although probably the safest option is to 
> make it 11.0 on that architecture

Sure. I will make it to use 11.0 on aarch64 and 10.14 on x64 and add comment 
mentioning that we have runtime guard to ignore metal pipeline for <10.14 
versions. With current 10.12 minimum version patch there is CI run on aarch64 
it has not thrown any error but i am not sure how that works.

-

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-14 Thread Jayathirth D V
On Fri, 12 Nov 2021 03:59:52 GMT, Phil Race  wrote:

>> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 850:
>> 
>>> 848:   COMMAND := $(METAL) -c -std=osx-metal2.0 \
>>> 849:   -mmacosx-version-min=$(MACOSX_VERSION_MIN) \
>>> 850:   -o $(SHADERS_AIR) $(SHADERS_SRC), \
>> 
>> No .. we decided metal requires macos 10.14 as a minimum. In fact it won't 
>> run (by policy) on earlier releases so settting it to what the broader JDK 
>> defines as a minimum is not appropriate right now.
>> And I've no idea what restrictions we'd be placing on metal saying it can 
>> only use 10.12 features.
>> Nor do I know what the xcode 12.4 used to build JDK 17 defaults to and how 
>> much a change to either 10.12 or 10.14 might require in re-testing.
>> 
>> So - 
>> we should use 10.14 
>> what's the actual impact of that on a current build using xcode 12.4 - does 
>> it change anything we use ?
>
> bear in mind "impact" might mean metal can't use some new faster code, not 
> just runtime errors

Hi Phil,

Thanks for the review. I also had doubts using 10.12. And yes as you mentioned 
we dont use Metal pipeline for versions < macOS10.14.

We dont use any Metal API which is specific to >macOS10.14(We had such usage 
when Lanai was under development but they were removed subsequently). So i dont 
see any performance impact of making xcrun to use 10.14 as minimum version.

Thanks,
Jay

-

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS

2021-11-11 Thread Jayathirth D V
On Thu, 11 Nov 2021 13:53:05 GMT, Magnus Ihse Bursie  wrote:

> Also, if you did not create this patch yourself, please make sure to use 
> `/contributor` to give proper credits. Or maybe you mean that Vitaly 
> submitted the bug report, not the patch?

By Submitter i meant submitter of bug in JBS. I was not able to verify the 
patch in our CI, so i shared the patch with Vitaly so that he can verify the 
reproducibility of the issue.

-

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-11 Thread Jayathirth D V
> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

Jayathirth D V has updated the pull request incrementally with one additional 
commit since the last revision:

  Use Macro for version

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6346/files
  - new: https://git.openjdk.java.net/jdk/pull/6346/files/a9f521a5..7f999650

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=00-01

  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

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


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-11 Thread Jayathirth D V
On Thu, 11 Nov 2021 13:52:13 GMT, Magnus Ihse Bursie  wrote:

> We should not hard-code version numbers like that.
> 
> Fortunately for you, there already exists a variable $(MACOSX_VERSION_MIN) 
> that you can use.

Thanks for the review i have updated code to use the Macro.

-

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


RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS

2021-11-10 Thread Jayathirth D V
When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set any 
deployment target when when we use xcrun to create .air file and this issue 
looks similar to https://developer.apple.com/forums/thread/105719. Also 
https://developer.apple.com/forums/thread/105719 states that even if they set 
app deployment target they need to explicitly set deployment in "xcrun metal".

Made same change to use -mmacosx-version-min=10.12.00 in our make file. Whether 
we should keep 10.12.00 or 10.13/14 is open to discussion for metal pipeline.

SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) has 
confirmed that patch resolves the issue.

-

Commit messages:
 - 8276905: Function frag_col has a deployment target which is incompatible 
with this OS

Changes: https://git.openjdk.java.net/jdk/pull/6346/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8276905
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

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


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

2021-03-12 Thread Jayathirth D V
On Thu, 11 Mar 2021 18:00:15 GMT, Ajit Ghaisas  wrote:

>> **Description :**
>> This is the implementation of [JEP 382 : New macOS Rendering 
>> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361)
>> It implements a Java 2D internal rendering pipeline for macOS using the 
>> Apple Metal API.
>> The entire work on this was done under [OpenJDK Project - 
>> Lanai](http://openjdk.java.net/projects/lanai/)
>> 
>> We iterated through several Early Access (EA) builds and have reached a 
>> stage where it is ready to be integrated to openjdk/jdk. The latest EA build 
>> is available at - https://jdk.java.net/lanai/
>> 
>> A new option -Dsun.java2d.metal=true | True needs to be used to use this 
>> pipeline.
>> 
>> **Testing :**
>> This implementation has been tested with the tests present at - [Test Plan 
>> for JEP 382: New macOS Rendering 
>> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396)
>> 
>> **Note to reviewers :**
>> 1) Default rendering pipeline on macOS has not been changed by this PR. 
>> OpenGL still stays as the default rendering pipeline and Metal rendering 
>> pipeline is optional to choose.
>> 
>> 2) To apply and test this PR - 
>> To enable the metal pipeline you must specify on command line 
>> -Dsun.java2d.metal=true (No message will be printed in this case) or 
>> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is 
>> enabled gets printed)
>> 
>> 3) Review comments (including some preliminary informal review comments) are 
>> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598
>
> Ajit Ghaisas has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 47 additional 
> commits since the last revision:
> 
>  - Lanai PR#214 - 8263324 - avu
>  - Merge branch 'master' into 8260931_lanai_JEP_branch
>  - Lanai PR#213 - 8263325 - avu
>  - Lanai PR#212 - 8259825 - aghaisas
>  - Lanai PR#211 - 8262882 - aghaisas
>  - Merge branch 'master' into 8260931_lanai_JEP_branch
>  - Lanai PR#210 - 8263159 - jdv
>  - Lanai PR#209 - 8262936 - jdv
>  - Lanai PR#208 - 8262928 - jdv
>  - Lanai PR#207 - 8262750 - jdv
>  - ... and 37 more: 
> https://git.openjdk.java.net/jdk/compare/46a8379b...369c3d21

Marked as reviewed by jdv (Reviewer).

-

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


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

2021-03-11 Thread Jayathirth D V



> On 12-Mar-2021, at 10:30 AM, Sergey Bylokhov  wrote:
> 
> On Fri, 12 Mar 2021 00:09:54 GMT, Kevin Rushforth  wrote:
> 
>>> Ajit Ghaisas has updated the pull request with a new target base due to a 
>>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>>> brought in by the merge/rebase. The pull request contains 47 additional 
>>> commits since the last revision:
>>> 
>>> - Lanai PR#214 - 8263324 - avu
>>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>>> - Lanai PR#213 - 8263325 - avu
>>> - Lanai PR#212 - 8259825 - aghaisas
>>> - Lanai PR#211 - 8262882 - aghaisas
>>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>>> - Lanai PR#210 - 8263159 - jdv
>>> - Lanai PR#209 - 8262936 - jdv
>>> - Lanai PR#208 - 8262928 - jdv
>>> - Lanai PR#207 - 8262750 - jdv
>>> - ... and 37 more: 
>>> https://git.openjdk.java.net/jdk/compare/9d82be31...369c3d21
>> 
>> Latest changes look good. I confirmed that this PR has all of the content 
>> from the lanai repo.
> 
>> By default we don?t do LCD anti-aliasing for text, only when we set Text 
>> Rendering Hint to use LCD antialiasing we take sub-pixel rendering path.
> 
> And it actually produces LCD glyphs? not the grayscale?
We get glyphs which take more than a byte to represent each pixel.
Based on which we take LCD sub-pixel rendering path.
> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/2403



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

2021-03-11 Thread Jayathirth D V


> On 12-Mar-2021, at 9:29 AM, Scott Palmer  wrote:
> 
> 
>> On Mar 11, 2021, at 9:53 PM, Sergey Bylokhov  wrote:
>> 
>> On Fri, 12 Mar 2021 02:29:04 GMT, Jayathirth D V  wrote:
>> 
>>>> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 
>>>> 323:
>>>> 
>>>>> 321:  * more code just to support a few uncommon cases.
>>>>> 322:  */
>>>>> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
>>>> 
>>>> Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
>>> 
>>> Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK 
>>> manual tests use LCD text on UI Components and it is recently verified in 
>>> 10.14+ systems for EA10.
>> 
>> Ok, for some reason I thought that the new macOS stopped providing LCD 
>> glyphs.
>> 
> 
> Are the glyphs different or just the way they are rasterized?
> 
> Newer versions of macOS don’t do LCD text and have gone back to plain 
> grey-scale anti-aliasing. 
> Java 2D should default to NOT doing LCD anti-aliasing for text on macOS if it 
> wants to fit in with the look of native applications. 
> (I’m not sure if that applies to non-retina displays.)
> 
By default we don’t do LCD anti-aliasing for text, only when we set Text 
Rendering Hint to use LCD antialiasing we take sub-pixel rendering path.
> Scott



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

2021-03-11 Thread Jayathirth D V
On Fri, 12 Mar 2021 00:42:35 GMT, Sergey Bylokhov  wrote:

>> Ajit Ghaisas has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains 47 additional 
>> commits since the last revision:
>> 
>>  - Lanai PR#214 - 8263324 - avu
>>  - Merge branch 'master' into 8260931_lanai_JEP_branch
>>  - Lanai PR#213 - 8263325 - avu
>>  - Lanai PR#212 - 8259825 - aghaisas
>>  - Lanai PR#211 - 8262882 - aghaisas
>>  - Merge branch 'master' into 8260931_lanai_JEP_branch
>>  - Lanai PR#210 - 8263159 - jdv
>>  - Lanai PR#209 - 8262936 - jdv
>>  - Lanai PR#208 - 8262928 - jdv
>>  - Lanai PR#207 - 8262750 - jdv
>>  - ... and 37 more: 
>> https://git.openjdk.java.net/jdk/compare/53b0bea8...369c3d21
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 323:
> 
>> 321:  * more code just to support a few uncommon cases.
>> 322:  */
>> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
> 
> Just curious, can we render LCD on 10.14+ via metal? Does it work fine?

Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK manual 
tests use LCD text on UI Components and it is recently verified in 10.14+ 
systems for EA10.

-

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


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

2021-02-10 Thread Jayathirth D V
On Thu, 11 Feb 2021 05:41:15 GMT, Ajit Ghaisas  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 Byteslibjava.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 Byteslibjava.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:
> 
>  src="https://user-images.githubusercontent.com/63425797/107575744-dc0ca180-6bb5-11eb-9eb3-5cff415eb8a3.png";>
> 
> Metal (the green color has a yellowish tint):
> 
>  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


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

2021-02-09 Thread Jayathirth D V
On Mon, 8 Feb 2021 18:05:02 GMT, Gerard Ziemski  wrote:

>> Ajit Ghaisas has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Lanai PR#175 - 8261304 - aghaisas
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLRenderQueue.java line 97:
> 
>> 95: public static void disposeGraphicsConfig(long pConfigInfo) {
>> 96: MTLRenderQueue rq = getInstance();
>> 97: rq.lock();
> 
> Is it allowed to have multiple `MTLRenderQueue` instances?
> 
> If not, then I see this pattern everywhere:
> 
> MTLRenderQueue rq = getInstance();
> rq.lock();
> {
>   ...
> }
> rq.unlock();
> why not just do:
> 
> MTLRenderQueue.lock();
> {
>   ...
> }
> MTLRenderQueue.unlock();
> 
> and have `MTLRenderQueue.lock()` and `MTLRenderQueue.unlock()` implement 
> getting the actual lock instance and locking/unlocking it?

Thanks for the recommendation. This is a common behavior among different 
pipelines :  D3D, OpenGL and Metal. Mentioned lock/unlock should be implemented 
in parent RenderQueue class after refactoring and it will hit other pipelines. 
Its better to not touch other pipelines as part of this JEP.

-

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


Re: RFR: 8253206: Enforce whitespace checking for additional source files

2020-09-16 Thread Jayathirth D V
On Tue, 15 Sep 2020 22:18:09 GMT, Kevin Rushforth  wrote:

> This adds the following extensions to the list of source files that git 
> jcheck will check for whitespace errors:
> 
>  .cc, .hh, .m, .mm
> 
> All files with the above extensions are now white-space clean after the fix 
> for
> [JDK-8240487](https://bugs.openjdk.java.net/browse/JDK-8240487). This will 
> help them stay that way.
> I just integrated a similar fix to `jfx` (with a few more source extensions 
> that aren't relevant for the JDK) in
> [JDK-8240499](https://bugs.openjdk.java.net/browse/JDK-8240499).

Marked as reviewed by jdv (Reviewer).

-

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