Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Ajit Ghaisas
On Mon, 8 Feb 2021 14:22:27 GMT, Kevin Rushforth  wrote:

>> I think, a generic name is OK as the path of shader file already has both 
>> awt (libawt_lwawt) and java2d in it.
>
> In the source tree, yes, but not in the jdk image where it ends up in 
> `$JAVA_HOME/lib/shaders.metallib`. I don't have a problem with this, as long 
> as it is a deliberate decision.

OK. I get your point. Keeping the name unchanged for now as there won't be 
another .metallib in JDK. The name can be changed in the future if need arises.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Gerard Ziemski
On Mon, 8 Feb 2021 12:28:07 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)
>
> Ajit Ghaisas has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Lanai PR#175 - 8261304 - aghaisas

Changes requested by gziemski (Committer).

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

> 59: 
> 60: public abstract class MTLSurfaceData extends SurfaceData
> 61: implements AccelSurface {

`MTLSurfaceData` and `OGLSurfaceData` seem to share lots of the same code, 
isn't there a way to refactor the common code out?

There are other files that structually look identical, except for the names of 
classes they use, ex `MTLMaskBlit.java` and `OGLMaskBlit.java`.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Gerard Ziemski
On Mon, 8 Feb 2021 12:28:07 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)
>
> Ajit Ghaisas has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Lanai PR#175 - 8261304 - aghaisas

> > General comment - I am not sure I like the `MTL` prefix acronym in names 
> > (ex. `sun.java2d.metal.MTLVolatileSurfaceManager`).
> > I think you tried to match the `CGL`, but that is a real acronym that 
> > stands for "Core Graphics Layer" (I think).
> > `MTL` on the other hand is no acronym. I can see `ML` for "Metal Layer" I 
> > suppose, but also just `Metal` would work just fine.
> 
> `MTL` is the abbreviation that Apple uses for Metal in all of their APIs. The 
> only potential issue I might see with this prefix is in the native code where 
> there could be name collisions between Java2D's names and Apple's names. 
> Since we haven't run into such a collision, I don't think this needs to 
> change, and wouldn't necessarily affect the Java class names anyway. If we 
> were to consider it, `METAL` seems better than `ML` (which is too short to be 
> descriptive and might suggest machine learning).

If Apple itself uses `MTL` then we are good.

src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java line 35:

> 33: 
> 34: import sun.java2d.SurfaceData;
> 35: import sun.java2d.opengl.CGLLayer;

Not needed import anymore?

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

> 111: // This indicates fallback to other rendering pipeline also 
> failed.
> 112: // Should never reach here
> 113: throw new InternalError("Error - unable to initialize any 
> rendering pipeline.");

There is no software based renderer to fall back here?

src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java line 66:

> 64:propString.equals("f") ||
> 65:propString.equals("False") ||
> 66:propString.equals("F"))

Shouldn't `1` and `0` be also allowed here?

src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java line 100:

> 98: oglState = PropertyState.ENABLED; // Enable 
> default pipeline
> 99: metalState = PropertyState.DISABLED; // Disable 
> non-default pipeline
> 100: }

This matches JEP 382 specification, but even when both GL is `false` and Metal 
is `false` we still get GL? There is no software based pipeline anymore?

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

> 129: @Native
> 130: static final int CAPS_EXT_GRAD_SHADER  = (FIRST_PRIVATE_CAP << 
> 3);
> 131: /** Indicates the presence of the GL_ARB_texture_rectangle 
> extension. */

Reference to `GL_ARB_texture_rectangle` extension in Metal pipeline?

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

> 132: @Native
> 133: static final int CAPS_EXT_TEXRECT  = (FIRST_PRIVATE_CAP << 
> 4);
> 134: /** Indicates the presence of the GL_NV_texture_barrier 
> extension. */

Reference to `GL_NV_texture_barrier` extension in Metal pipeline?

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:


Re: [OpenJDK 2D-Dev] RFR: 6211198: ICC_Profile.getInstance(byte[]): IAE is not specified [v2]

2021-02-08 Thread Sergey Bylokhov
On Sun, 7 Feb 2021 18:35:31 GMT, Phil Race  wrote:

>> Sergey Bylokhov 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 three additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' into JDK-6211198
>>  - Merge branch 'master' into JDK-6211198
>>  - Initial fix
>
> src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 767:
> 
>> 765:  * @return an {@code ICC_Profile} object corresponding to the data 
>> in the
>> 766:  * specified {@code data} array
>> 767:  * @throws IllegalArgumentException If the byte array does not 
>> contain valid
> 
> The spec does already say "Throws an {@code IllegalArgumentException} if the 
> data does 
> * not correspond to a valid ICC Profile"
> So really you are just adding the @throws clause. 
> You might want to consider removing the now redundant text.

Agree. that text is removed.

-

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


Re: [OpenJDK 2D-Dev] RFR: 6211198: ICC_Profile.getInstance(byte[]): IAE is not specified [v3]

2021-02-08 Thread Sergey Bylokhov
> The specification of the java.awt.color.ICC_Profile.getInstance(byte[]) is 
> updated.
> Its implementation changed over time, and different exceptions were thrown, 
> but since JDK-8013430 always throws an IllegalArgumentException on null and 
> invalid data.

Sergey Bylokhov 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 five additional 
commits since the last revision:

 - Update ICC_Profile.java
 - Merge branch 'master' into JDK-6211198
 - Merge branch 'master' into JDK-6211198
 - Merge branch 'master' into JDK-6211198
 - Initial fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2328/files
  - new: https://git.openjdk.java.net/jdk/pull/2328/files/12f673c1..e96ac91b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2328=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2328=01-02

  Stats: 9387 lines in 200 files changed: 5091 ins; 3213 del; 1083 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2328.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2328/head:pull/2328

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Kevin Rushforth
On Mon, 8 Feb 2021 17:15:25 GMT, Gerard Ziemski  wrote:

> General comment - I am not sure I like the `MTL` prefix acronym in names (ex. 
> `sun.java2d.metal.MTLVolatileSurfaceManager`).
> 
> I think you tried to match the `CGL`, but that is a real acronym that stands 
> for "Core Graphics Layer" (I think).
> 
> `MTL` on the other hand is no acronym. I can see `ML` for "Metal Layer" I 
> suppose, but also just `Metal` would work just fine.

`MTL` is the abbreviation that Apple uses for Metal in all of their APIs. The 
only potential issue I might see with this prefix is in the native code where 
there could be name collisions between Java2D's names and Apple's names. Since 
we haven't run into such a collision, I don't think this needs to change, and 
wouldn't necessarily affect the Java class names anyway. If we were to consider 
it, `METAL` seems better than `ML` (which is too short to be descriptive and 
might suggest machine learning).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Gerard Ziemski
On Mon, 8 Feb 2021 17:15:25 GMT, Gerard Ziemski  wrote:

>> The file in `RenderPerfTest` should have a GPLv2 license header (no 
>> Classpath). I filed 
>> [JDK-8261273](https://bugs.openjdk.java.net/browse/JDK-8261273) and also 
>> highlighted a couple examples below.
>
> General comment - I am not sure I like the `MTL` prefix acronym in names (ex. 
> `sun.java2d.metal.MTLVolatileSurfaceManager`).
> 
> I think you tried to match the `CGL`, but that is a real acronym that stands 
> for "Core Graphics Layer" (I think).
> 
> `MTL` on the other hand is no acronym. I can see `ML` for "Metal Layer" I 
> suppose, but also just `Metal` would work just fine.

I'm in the process of reviewing this feature, but there is lots of code to go 
through - please wait for my review.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Gerard Ziemski
On Sat, 6 Feb 2021 00:53:08 GMT, Kevin Rushforth  wrote:

>> Ajit Ghaisas has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Lanai PR#175 - 8261304 - aghaisas
>
> The file in `RenderPerfTest` should have a GPLv2 license header (no 
> Classpath). I filed 
> [JDK-8261273](https://bugs.openjdk.java.net/browse/JDK-8261273) and also 
> highlighted a couple examples below.

General comment - I am not sure I like the `MTL` prefix acronym in names (ex. 
`sun.java2d.metal.MTLVolatileSurfaceManager`).

I think you tried to match the `CGL`, but that is a real acronym that stands 
for "Core Graphics Layer" (I think).

`MTL` on the other hand is no acronym. I can see `ML` for "Metal Layer" I 
suppose, but also just `Metal` would work just fine.

-

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


[OpenJDK 2D-Dev] Integrated: 8261231: Windows IME was disabled after DnD operation

2021-02-08 Thread Dmitry Markov
On Sun, 7 Feb 2021 08:29:57 GMT, Dmitry Markov  wrote:

> The function InvokeInputMethodFunction() is responsible for invocation of IME 
> API. Typically it uses PostMessage() to execute corresponding IME function on 
> the toolkit thread but if DnD operation takes place SendMessage() is used. 
> The state of m_inputMethodWaitEvent event object remains signalled after 
> SendMessage() execution. That causes failure of subsequent IME functions 
> calls via PostMessage().
> 
> Fix:
> SendMessage() and PostMessage() calls inside InvokeInputMethodFunction() 
> should be synchronised. The state of m_inputMethodWaitEvent event object must 
> be reseted right after SendMessage() execution.

This pull request has now been integrated.

Changeset: d6d5d9bf
Author:Dmitry Markov 
URL:   https://git.openjdk.java.net/jdk/commit/d6d5d9bf
Stats: 5 lines in 1 file changed: 3 ins; 1 del; 1 mod

8261231: Windows IME was disabled after DnD operation

Reviewed-by: kizune, serb

-

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


Re: [OpenJDK 2D-Dev] RFR: 8261231: Windows IME was disabled after DnD operation

2021-02-08 Thread Dmitry Markov
On Mon, 8 Feb 2021 16:51:21 GMT, Alexander Zuev  wrote:

> Change looks good and i haven't found any side-effects during testing. Could 
> you please add the label to the bug noting reason for absence of the 
> regression test, like noreg-hard or something?

Thank you for the review. I have added noreg-hard to the bug

-

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


Re: [OpenJDK 2D-Dev] RFR: 8261231: Windows IME was disabled after DnD operation

2021-02-08 Thread Sergey Bylokhov
On Sun, 7 Feb 2021 08:29:57 GMT, Dmitry Markov  wrote:

> The function InvokeInputMethodFunction() is responsible for invocation of IME 
> API. Typically it uses PostMessage() to execute corresponding IME function on 
> the toolkit thread but if DnD operation takes place SendMessage() is used. 
> The state of m_inputMethodWaitEvent event object remains signalled after 
> SendMessage() execution. That causes failure of subsequent IME functions 
> calls via PostMessage().
> 
> Fix:
> SendMessage() and PostMessage() calls inside InvokeInputMethodFunction() 
> should be synchronised. The state of m_inputMethodWaitEvent event object must 
> be reseted right after SendMessage() execution.

Marked as reviewed by serb (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8261231: Windows IME was disabled after DnD operation

2021-02-08 Thread Ichiroh Takiguchi
On Mon, 8 Feb 2021 16:51:21 GMT, Alexander Zuev  wrote:

>> The function InvokeInputMethodFunction() is responsible for invocation of 
>> IME API. Typically it uses PostMessage() to execute corresponding IME 
>> function on the toolkit thread but if DnD operation takes place 
>> SendMessage() is used. The state of m_inputMethodWaitEvent event object 
>> remains signalled after SendMessage() execution. That causes failure of 
>> subsequent IME functions calls via PostMessage().
>> 
>> Fix:
>> SendMessage() and PostMessage() calls inside InvokeInputMethodFunction() 
>> should be synchronised. The state of m_inputMethodWaitEvent event object 
>> must be reseted right after SendMessage() execution.
>
> Change looks good and i haven't found any side-effects during testing. Could 
> you please add the label to the bug noting reason for absence of the 
> regression test, like noreg-hard or something?

I also tested this fix. It worked fine. Thanks.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8261231: Windows IME was disabled after DnD operation

2021-02-08 Thread Alexander Zuev
On Sun, 7 Feb 2021 08:29:57 GMT, Dmitry Markov  wrote:

> The function InvokeInputMethodFunction() is responsible for invocation of IME 
> API. Typically it uses PostMessage() to execute corresponding IME function on 
> the toolkit thread but if DnD operation takes place SendMessage() is used. 
> The state of m_inputMethodWaitEvent event object remains signalled after 
> SendMessage() execution. That causes failure of subsequent IME functions 
> calls via PostMessage().
> 
> Fix:
> SendMessage() and PostMessage() calls inside InvokeInputMethodFunction() 
> should be synchronised. The state of m_inputMethodWaitEvent event object must 
> be reseted right after SendMessage() execution.

Change looks good and i haven't found any side-effects during testing. Could 
you please add the label to the bug noting reason for absence of the regression 
test, like noreg-hard or something?

-

Marked as reviewed by kizune (Reviewer).

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


Re: [OpenJDK 2D-Dev] RFR: 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified

2021-02-08 Thread Sergey Bylokhov
On Mon, 8 Feb 2021 09:30:02 GMT, Prasanta Sadhukhan  
wrote:

> Constructor AreaAveragingScaleFilter(int, int) for 
> java.awt.image.AreaAveragingScaleFilter class throws IllegalArgumentException 
> when 0 is passed for width,height but it's not specified in the spec.
> Updated spec to illustrate this.

Marked as reviewed by serb (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v4]

2021-02-08 Thread Sergey Bylokhov
On Mon, 8 Feb 2021 11:55:09 GMT, Prasanta Sadhukhan  
wrote:

>> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws 
>> NullPointerException when passed a null object reference for a input 
>> parameter but it's not specified in the spec.
>> Updated spec to illustrate this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove redundant return statement

Marked as reviewed by serb (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8261282: Lazy initialization of built-in ICC_Profile/ColorSpace classes is too lazy

2021-02-08 Thread Alexander Zvegintsev
On Sun, 7 Feb 2021 05:36:52 GMT, Sergey Bylokhov  wrote:

> Short fix description:
> 
> The fix changes the initialization of the built-in color profiles/spaces from 
> the lazy initialization per requested profile via static synchronization to 
> the lazy initialization of all profiles/spaces on the first request via class 
> initialization(holder idiom).
> 
> Long fix description:
> 
> 1. A long time ago the factory method ICC_Profile#getInstance() was a 
> heavyweight method. For each requested profile it used the data of the icc 
> file(ex: sRGB: 6kb and pycc: 200kb).
> As a result, in our code, we have tried to skip this method as much as 
> possible or delay its usage. But we found that we have to use it anyway 
> during startup. So we implemented the deferral mechanic for the sRGB profile. 
> When this profile is requested we did not read the data but return the stub 
> that contained some known in advance properties, such as num of color 
> components, etc. The icc data is used only if the user request some of it 
> later -> in this case we "activate" the profile and drop the stub data.
> 
> 2. Later we found that we may need some other profiles at startup, and the 
> deferral mechanics was implemented for all profiles by the JDK-6793818. But 
> profile activation was implemented as one step for all profiles at once, so 
> if one profile such as sRGB was activated then the next profiles returned 
> from the "ICC_Profile#getInstance" if not requested before was activated as 
> well(used the icc file data).
> 
> 3. The deferral mechanics were updated in the JDK-6986863. Now activation of 
> one profile does not affect other profiles and as a result, the 
> "ICC_Profile#getInstance" always returns the stubs of the requested profiles.
> 
> 4. Each profile stub contains just a few lightweight objects, but still, use 
> the heavyweight static synchronization to access/create it, see:
>
> https://github.com/openjdk/jdk/blob/9d59dec200490f11bfc1c661b30f10c7edee3a6d/src/java.desktop/share/classes/java/awt/color/ICC_Profile.java#L821
>Note that we have separate blocks for each profile("stub"). That looks an 
> overkill.
>
> 5. Note that in theory, it is not necessary to create these stubs lazily, 
> each stub is a ICC_Profile object and ProfileDeferralInfo object, so we can 
> use them as static fields in the ICC_Profile class. But here that classes 
> initialization order come to play -> it is a bad idea to refer to the 
> ICC_ProfileRGB(a subclass of the ICC_Profile) in the static block of 
> ICC_Profile class because this could cause a deadlock.
> 
> 6. So this change merged initialization of all stubs to the one-step, still 
> initialize stubs lazily, and maintains the singleton property.

Marked as reviewed by azvegint (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Kevin Rushforth
On Mon, 8 Feb 2021 13:40:22 GMT, Ajit Ghaisas  wrote:

>> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 894:
>> 
>>> 892:   SHADERS_SUPPORT_DIR := 
>>> $(SUPPORT_OUTPUTDIR)/native/java.desktop/libosxui
>>> 893:   SHADERS_AIR := $(SHADERS_SUPPORT_DIR)/shaders.air
>>> 894:   SHADERS_LIB := $(INSTALL_LIBRARIES_HERE)/shaders.metallib
>> 
>> Q: Should 2d (or awt) be in the name of this file, or is a generic name OK?
>
> I think, a generic name is OK as the path of shader file already has both awt 
> (libawt_lwawt) and java2d in it.

In the source tree, yes, but not in the jdk image where it ends up in 
`$JAVA_HOME/lib/shaders.metallib`. I don't have a problem with this, as long as 
it is a deliberate decision.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Ajit Ghaisas
On Fri, 5 Feb 2021 18:42:02 GMT, Kevin Rushforth  wrote:

>> Ajit Ghaisas has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Lanai PR#175 - 8261304 - aghaisas
>
> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 894:
> 
>> 892:   SHADERS_SUPPORT_DIR := 
>> $(SUPPORT_OUTPUTDIR)/native/java.desktop/libosxui
>> 893:   SHADERS_AIR := $(SHADERS_SUPPORT_DIR)/shaders.air
>> 894:   SHADERS_LIB := $(INSTALL_LIBRARIES_HERE)/shaders.metallib
> 
> Q: Should 2d (or awt) be in the name of this file, or is a generic name OK?

I think, a generic name is OK as the path of shader file already has both awt 
(libawt_lwawt) and java2d in it.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

2021-02-08 Thread Ajit Ghaisas
> **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)

Ajit Ghaisas has updated the pull request incrementally with one additional 
commit since the last revision:

  Lanai PR#175 - 8261304 - aghaisas

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2403/files
  - new: https://git.openjdk.java.net/jdk/pull/2403/files/8ed7b5f5..6044adc0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=01-02

  Stats: 32 lines in 10 files changed: 0 ins; 12 del; 20 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2403.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2403/head:pull/2403

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


Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v4]

2021-02-08 Thread Prasanta Sadhukhan
> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws 
> NullPointerException when passed a null object reference for a input 
> parameter but it's not specified in the spec.
> Updated spec to illustrate this.

Prasanta Sadhukhan has updated the pull request incrementally with one 
additional commit since the last revision:

  Remove redundant return statement

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2377/files
  - new: https://git.openjdk.java.net/jdk/pull/2377/files/d65b8e0a..bdc97f6a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2377=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2377=02-03

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

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


Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v3]

2021-02-08 Thread Alexey Ivanov
On Sun, 7 Feb 2021 09:18:04 GMT, Prasanta Sadhukhan  
wrote:

>> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws 
>> NullPointerException when passed a null object reference for a input 
>> parameter but it's not specified in the spec.
>> Updated spec to illustrate this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - Address review comments
>  - Revert "Address review comments"
>
>This reverts commit 3fff74d7563a6141d67cb18fd7c3dda731a4c752.
>  - Address review comments

Marked as reviewed by aivanov (Reviewer).

test/jdk/java/awt/BasicStroke/TestNullShape.java line 42:

> 40: } catch (NullPointerException ne) {
> 41: System.out.println("result (npe): true");
> 42: return;

Return is from catch is redundant now.
Suggestion:

-

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


Re: [OpenJDK 2D-Dev] RFR: 6211198: ICC_Profile.getInstance(byte[]): IAE is not specified [v2]

2021-02-08 Thread Pankaj Bansal
On Sat, 6 Feb 2021 07:05:05 GMT, Sergey Bylokhov  wrote:

>> The specification of the java.awt.color.ICC_Profile.getInstance(byte[]) is 
>> updated.
>> Its implementation changed over time, and different exceptions were thrown, 
>> but since JDK-8013430 always throws an IllegalArgumentException on null and 
>> invalid data.
>
> Sergey Bylokhov 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 three additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into JDK-6211198
>  - Merge branch 'master' into JDK-6211198
>  - Initial fix

Marked as reviewed by pbansal (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified

2021-02-08 Thread Tejpal Rebari
On Mon, 8 Feb 2021 09:30:02 GMT, Prasanta Sadhukhan  
wrote:

> Constructor AreaAveragingScaleFilter(int, int) for 
> java.awt.image.AreaAveragingScaleFilter class throws IllegalArgumentException 
> when 0 is passed for width,height but it's not specified in the spec.
> Updated spec to illustrate this.

Looks good to me.

-

Marked as reviewed by trebari (Committer).

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


Re: [OpenJDK 2D-Dev] RFR: 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified

2021-02-08 Thread Alexander Zvegintsev
On Mon, 8 Feb 2021 09:30:02 GMT, Prasanta Sadhukhan  
wrote:

> Constructor AreaAveragingScaleFilter(int, int) for 
> java.awt.image.AreaAveragingScaleFilter class throws IllegalArgumentException 
> when 0 is passed for width,height but it's not specified in the spec.
> Updated spec to illustrate this.

Marked as reviewed by azvegint (Reviewer).

-

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


[OpenJDK 2D-Dev] RFR: 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified

2021-02-08 Thread Prasanta Sadhukhan
Constructor AreaAveragingScaleFilter(int, int) for 
java.awt.image.AreaAveragingScaleFilter class throws IllegalArgumentException 
when 0 is passed for width,height but it's not specified in the spec.
Updated spec to illustrate this.

-

Commit messages:
 - Fix
 - Fix
 - 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified

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

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