Re: RFR: 8263142: Delete unused entry points in libawt/libawt_xawt/libawt_headless

2021-03-07 Thread Alexander Zuev
On Sun, 7 Mar 2021 23:16:18 GMT, Sergey Bylokhov  wrote:

> During the review of:
>8254024: Enhance native libs for AWT and Swing to work with GraalVM Native 
> Image
> 
> I have found that some of the entry points in our libraries are never used, 
> and can be removed, we do not need to update the code to make it work in 
> static_build.

Marked as reviewed by kizune (Reviewer).

src/java.desktop/unix/native/libawt/awt/awt_LoadLibrary.c line 267:

> 265: {
> 266: }
> 267: #endif

I guess we do not support CDE JMF anymore? Then seems logical.

-

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


RFR: 8263142: Delete unused entry points in libawt/libawt_xawt/libawt_headless

2021-03-07 Thread Sergey Bylokhov
During the review of:
   8254024: Enhance native libs for AWT and Swing to work with GraalVM Native 
Image

I have found that some of the entry points in our libraries are never used, and 
can be removed, we do not need to update the code to make it work in 
static_build.

-

Commit messages:
 - Initial fix

Changes: https://git.openjdk.java.net/jdk/pull/2865/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2865=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263142
  Stats: 220 lines in 3 files changed: 0 ins; 217 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2865.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2865/head:pull/2865

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


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

2021-03-07 Thread Sergey Bylokhov
On Mon, 1 Mar 2021 11:17:39 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 36 additional 
> commits since the last revision:
> 
>  - Lanai PR#206 - 8262729 - aghaisas
>  - Lanai PR#205 - 8262496 - avu
>  - Lanai PR#203 - 8262313 - jdv
>  - Lanai PR#202 - 8262293 - avu
>  - Lanai PR#201 - 8261891 - avu
>  - Lanai PR#200 - 8262115 - aghaisas
>  - Merge branch 'master' into 8260931_lanai_JEP_branch
>  - Lanai PR#199 - 8262091 - aghaisas
>  - Lanai PR#198 - 8261646 - avu
>  - Lanai PR#197 - 8261960 - jdv
>  - ... and 26 more: 
> https://git.openjdk.java.net/jdk/compare/49503c0d...5cb1fd91

src/java.desktop/macosx/classes/sun/awt/CGraphicsDevice.java line 81:

> 79: // Try falling back to OpenGL pipeline
> 80: if (MacOSFlags.isMetalVerbose()) {
> 81: System.out.println("Metal rendering pipeline 
> initialization failed. Using OpenGL rendering pipeline.");

Please split the long lines, here and there. Most of these files use 80 chars 
per line.

src/java.desktop/macosx/classes/sun/java2d/metal/MTLBlitLoops.java line 499:

> 497: }
> 498: 
> 499: // We can convert argb_pre data from MTL surface in two places:

Does anybody check that this is true for the metal pipeline? or It is just 
copied from the OGL?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 58:

> 56:  * Makes the given GraphicsConfig's context current to its associated
> 57:  * "scratch surface".  Each GraphicsConfig maintains a native context
> 58:  * (MTLDevice) as well as a native pbuffer

"pbuffer"?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 68:

> 66:  * when disposing a texture-based surface).
> 67:  */
> 68: public static void setScratchSurface(long pConfigInfo) {

How the scratch surface is used in the metal pipeline? Why it is not enough to 
set the "context current"?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line 
168:

> 166: }
> 167: 
> 168: ContextCapabilities caps = new MTLContext.MTLContextCaps(

CAPS_DOUBLEBUFFERED is not included?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 140:

> 138: StringBuilder sb = new StringBuilder(super.toString());
> 139: if ((caps & CAPS_DOUBLEBUFFERED) != 0) {
> 140: sb.append("CAPS_DOUBLEBUFFERED|");

Related to other questions, we do not include CAPS_DOUBLEBUFFERED, but anyway, 
report the surface as double buffered.

src/java.desktop/macosx/classes/sun/java2d/metal/MTLLayer.java line 44:

> 42: 
> 43: // Pass the insets to native code to make adjustments in blitTexture
> 44: private static native void nativeSetInsets(long layerPtr, int top, 
> int left);

Probably I have asked this already, but why we need to use insets? Why insets 
is not necessary for ogl?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLRenderer.java line 25:

> 23:  * questions.
> 24:  */
> 25: package sun.java2d.metal;

Looks like other files use blank line befor "package".

src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 146:

> 144: return MTLSurfaceRTT;
> 145: default:
> 146: return MTLSurface;

Do we 

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

2021-03-07 Thread Sergey Bylokhov
On Fri, 5 Feb 2021 22:00: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 36 additional 
>> commits since the last revision:
>> 
>>  - Lanai PR#206 - 8262729 - aghaisas
>>  - Lanai PR#205 - 8262496 - avu
>>  - Lanai PR#203 - 8262313 - jdv
>>  - Lanai PR#202 - 8262293 - avu
>>  - Lanai PR#201 - 8261891 - avu
>>  - Lanai PR#200 - 8262115 - aghaisas
>>  - Merge branch 'master' into 8260931_lanai_JEP_branch
>>  - Lanai PR#199 - 8262091 - aghaisas
>>  - Lanai PR#198 - 8261646 - avu
>>  - Lanai PR#197 - 8261960 - jdv
>>  - ... and 26 more: 
>> https://git.openjdk.java.net/jdk/compare/f34bb8ed...5cb1fd91
>
> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m 
> line 82:
> 
>> 80: (JNIEnv *env, jclass mtlgc)
>> 81: {
>> 82: FILE *f = popen("/usr/sbin/system_profiler SPDisplaysDataType", "r");
> 
> How robust is this? It seems like the contents of this could be an 
> implementation detail and subject to change. Is it documented by Apple?

I suggest fixing this before the integration.

> test/jdk/performance/client/RenderPerfTest/Makefile line 10:
> 
>> 8: #   - Redistributions of source code must retain the above copyright
>> 9: # notice, this list of conditions and the following disclaimer.
>> 10: #
> 
> As mentioned in the preliminary review, this file and `build.xml` have a BSD 
> copyright. I think that should be GPLv2 (without classpath exception since 
> this is a test).

Probably place it near J2Dbench?

-

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