Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Fri, 12 Mar 2021 00:36:34 GMT, Sergey Bylokhov wrote: >> I fixed some conversion logic within JDK-8256331. > > OK, just to confirm. I wrote that text for OGL because it was the fastest way > to transfer+convert the data from the video card to the memory. And as far as > I understand the metal has the same limitation? It is not possible to convert > it to some non-ARGB/Pre format on the fly while transferring the pixles? If we're talking about metal blit operation, the answer is No. Also, it looks like we still need a blit operation from video memory even if we use compute shader. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
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/74f2d574...369c3d21 Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 10:11:27 GMT, Prasanta Sadhukhan 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/f5d78efb...369c3d21 > > src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java line 82: > >> 80: * Creates a new SurfaceData that will be associated with the given >> 81: * CGLLayer. >> 82: */ > > I guess we need to specify CGLLayer/MTLLayer as now we are implementing this > method in both pipeline. I have updated JDK-8263363 to include this. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
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/d4addb43...369c3d21 Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
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/b163fb99...369c3d21 src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java line 82: > 80: * Creates a new SurfaceData that will be associated with the given > 81: * CGLLayer. > 82: */ I guess we need to specify CGLLayer/MTLLayer as now we are implementing this method in both pipeline. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
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]
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/392a1086...369c3d21 Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 01:01:09 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/774f3477...369c3d21
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
> 148:
>
>> 146: rq.lock();
>> 147: try {
>> 148: // getMTLConfigInfo() creates and destroys temporary
>
> Does it really create and destroy temporary surfaces/contexts?
No, it doesn't. I have updated JDK-8263363 to include this correction.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> 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]
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? - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> 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]
On Fri, 12 Mar 2021 01:04:22 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/23f9642d...369c3d21 > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.m line 813: > >> 811: initTemplatePipelineDescriptors(); >> 812: // This block is not reached in current implementation. >> 813: // Texture paint XOR mode rendering uses a tile based rendering >> using a SW pipe (similar to OGL) > > Do we have a bugid to implement this later? This is not exactly a TODO which we intend to implement later. Hence, there is no bugid. Metal does not support XOR mode rendering. I have kept this code as to know the code path to take in case we need to implement it in future. > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h > line 59: > >> 57: * The offset in pixels of the Metal viewport origin from the lower-left >> 58: * corner of the heavyweight drawable. For example, a top-level frame on >> 59: * Windows XP has lower-left insets of (4,4). The Metal viewport origin > > Do we really use these fields? "Windows XP"?? This will be fixed as a follow-on issue - JDK-8263486. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:48:58 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/aca5b193...369c3d21
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTSurfaceLayers.m line 73:
>
>> 71: // Updates back buffer size of the layer if it's an OpenGL/Metal layer
>> 72: // including all OpenGL/Metal sublayers
>> 73: + (void) repaintLayersRecursively:(CALayer*)aLayer {
>
> The operation of this code is still being investigated. @prsadhuk please add
> a bugid here.
JDK-8263485
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> 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.)
Scott
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
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 [v13]
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/5e61bf57...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?
src/java.desktop/macosx/native/libawt_lwawt/awt/AWTSurfaceLayers.m line 73:
> 71: // Updates back buffer size of the layer if it's an OpenGL/Metal layer
> 72: // including all OpenGL/Metal sublayers
> 73: + (void) repaintLayersRecursively:(CALayer*)aLayer {
The operation of this code is still being investigated. @prsadhuk please add a
bugid here.
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
148:
> 146: rq.lock();
> 147: try {
> 148: // getMTLConfigInfo() creates and destroys temporary
Does it really create and destroy temporary surfaces/contexts?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.m line 813:
> 811: initTemplatePipelineDescriptors();
> 812: // This block is not reached in current implementation.
> 813: // Texture paint XOR mode rendering uses a tile based rendering
> using a SW pipe (similar to OGL)
Do we have a bugid to implement this later?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h
line 59:
> 57: * The offset in pixels of the Metal viewport origin from the lower-left
> 58: * corner of the heavyweight drawable. For example, a top-level frame on
> 59: * Windows XP has lower-left insets of (4,4). The Metal viewport origin
Do we really use these fields? "Windows XP"??
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 20:16:25 GMT, Alexey Ushakov wrote: >> 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? > > I fixed some conversion logic within JDK-8256331. OK, just to confirm. I wrote that text for OGL because it was the fastest way to transfer+convert the data from the video card to the memory. And as far as I understand the metal has the same limitation? It is not possible to convert it to some non-ARGB/Pre format on the fly while transferring the pixles? >> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line >> 146: >> >>> 144: return MTLSurfaceRTT; >>> 145: default: >>> 146: return MTLSurface; >> >> Do we really support 3 different types of surface? I guess only two of them >> are different: textures used for manageable images, and surfaces used by the >> volatile image. > > It's true but I don't see any problem returning a more generic type as a > default here tbd - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
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/204b2bcc...369c3d21 Latest changes look good. I confirmed that this PR has all of the content from the lanai repo. - Marked as reviewed by kcr (Author). PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
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/ada1a022...369c3d21 Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Thu, 11 Mar 2021 07:40:47 GMT, Alexey Ushakov 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 45 additional >> commits since the last revision: >> >> - 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 >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#206 - 8262729 - aghaisas >> - ... and 35 more: >> https://git.openjdk.java.net/jdk/compare/d1e5ff8f...3980ecbd > > Marked as reviewed by avu (no project role). We have addressed the review comments (except for a few minor follow-on issues) This version of the PR is the one which we are planning to integrate into the JDK. Please review and approve. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> **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/1c8fee73...369c3d21 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/3980ecbd..369c3d21 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=11-12 Stats: 2420 lines in 128 files changed: 1246 ins; 368 del; 806 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Mon, 8 Feb 2021 18:47:09 GMT, Sergey Bylokhov wrote:
>> 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.
Fixed with JDK-8259825.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 23:15:57 GMT, Sergey Bylokhov wrote: >> Also, MTLSD_WINDOW is supported > > How it is used? Can we really draw to the window directly? It looks like it > copied from the OGL where it is unused since CLayer integration. Something is > inconsistent here. Here is a follow-up issue JDK-8263463 - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Wed, 10 Mar 2021 10:51:08 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 45 additional > commits since the last revision: > > - 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 > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#206 - 8262729 - aghaisas > - ... and 35 more: > https://git.openjdk.java.net/jdk/compare/59142f88...3980ecbd Marked as reviewed by avu (no project role). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Wed, 10 Mar 2021 10:51:08 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 45 additional > commits since the last revision: > > - 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 > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#206 - 8262729 - aghaisas > - ... and 35 more: > https://git.openjdk.java.net/jdk/compare/42b2f14b...3980ecbd Marked as reviewed by kcr (Author). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:40:53 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 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/33534417...5cb1fd91
>
> src/java.desktop/macosx/classes/sun/lwawt/macosx/CWarningWindow.java line 304:
>
>> 302: };
>> 303: }
>> 304: public MTLLayer createMTLLayer() {
>
> TODO to test that this functionality works
Testing was done under JDK-8261141.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v11]
On Tue, 9 Mar 2021 20:18:08 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 41 additional >> commits since the last revision: >> >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - 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 >> - ... and 31 more: >> https://git.openjdk.java.net/jdk/compare/d65116dd...de456939 > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m > line 185: > >> 183: >> 184: NSRect contentRect = NSMakeRect(0, 0, 64, 64); >> 185: NSWindow *window = > > Some remnant from the scratch surface initialization? This will be removed under - JDK-8263363 - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
> **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 45 additional commits since the last revision: - 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 - Merge branch 'master' into 8260931_lanai_JEP_branch - Lanai PR#206 - 8262729 - aghaisas - ... and 35 more: https://git.openjdk.java.net/jdk/compare/dfe037de...3980ecbd - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/de456939..3980ecbd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=10-11 Stats: 4881 lines in 177 files changed: 1767 ins; 2327 del; 787 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 22:11:20 GMT, Alexey Ushakov wrote: >> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h >> line 109: >> >>> 107: #define MTLSD_TEXTURE sun_java2d_pipe_hw_AccelSurface_TEXTURE >>> 108: #define MTLSD_FLIP_BACKBUFFER >>> sun_java2d_pipe_hw_AccelSurface_FLIP_BACKBUFFER >>> 109: #define MTLSD_RT_TEXTURE >>> sun_java2d_pipe_hw_AccelSurface_RT_TEXTURE >> >> Only two of them are supported? MTLSD_TEXTURE and MTLSD_RT_TEXTURE? > > Also, MTLSD_WINDOW is supported How it is used? Can we really draw to the window directly? It looks like it copied from the OGL where it is unused since CLayer integration. Something is inconsistent here. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 20:04:17 GMT, Alexey Ushakov wrote: >> Probably it's enough to set the context in SET_SURFACES, I'll double-check >> it. > > Looks like it works even without SET_SCRATCH_SURFACE (I did some limited > testing) but I don't think we should do such a huge change at this moment. JDK-8263309 - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 20:13:03 GMT, Sergey Bylokhov wrote: >> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line >> 676: >> >>> 674: height = srcInfo.bounds.y2 - srcInfo.bounds.y1; >>> 675: >>> 676: pDst = PtrAddBytes(pDst, dstx * dstInfo.pixelStride); >> >> Here and in other places use the PtrPixelsRow instead, do not use >> multiplication, see the history of the OGLBlitLoops_SurfaceToSwBlit method. > > I think this at least should be investigated before integration. JDK-8263324 - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v11]
On Tue, 9 Mar 2021 19:52:35 GMT, Phil Race 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 41 additional >> commits since the last revision: >> >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - 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 >> - ... and 31 more: >> https://git.openjdk.java.net/jdk/compare/1bd8aea0...de456939 > > test/jdk/performance/client/RenderPerfTest/build.xml line 72: > >> 70: >> 71: >> 72: > > This was presumably borrowed from J2DBench so this comment should be fixed ! JDK-8263325 - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 17:59:26 GMT, Alexey Ushakov wrote: >> In fact, we don't have any scratch surfaces. SET_SCRATCH_SURFACE effectively >> sets the new context. For better readability, we should add the new op >> SET_CONTEXT in BufferedOpCodes > > Probably it's enough to set the context in SET_SURFACES, I'll double-check it. Looks like it works even without SET_SCRATCH_SURFACE (I did some limited testing) but I don't think we should do such a huge change at this moment. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:31:16 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 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/2f09d382...5cb1fd91 > > 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? We have different coordinate systems for OGL and Metal. Metal (0,0) for textures is the top-left corner whereas OGL (0,0) is the bottom-left one, so we need to have insets to perform blitting to drawable correctly. > 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? I fixed some conversion logic within JDK-8256331. > 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? JDK-8263306 > src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 146: > >> 144: return MTLSurfaceRTT; >> 145: default: >> 146: return MTLSurface; > > Do we really support 3 different types of surface? I guess only two of them > are different: textures used for manageable images, and surfaces used by the > volatile image. It's true but I don't see any problem returning a more generic type as a default here > src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 221: > >> 219: protected native boolean initRTexture(long pData, boolean isOpaque, >> int width, int height); >> 220: >> 221: protected native boolean initFlipBackbuffer(long pData); > > flip back buffer is supported? No, it should be removed (JDK-8263312) > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceData.m > line 284: > >> 282: */ >> 283: jboolean >> 284: MTLSD_InitMTLWindow(JNIEnv *env, BMTLSDOps *bmtlsdo) > > When the MTLWindow is used? MTLContext setSurfacesEnv > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h > line 109: > >> 107: #define MTLSD_TEXTURE sun_java2d_pipe_hw_AccelSurface_TEXTURE >> 108: #define MTLSD_FLIP_BACKBUFFER >> sun_java2d_pipe_hw_AccelSurface_FLIP_BACKBUFFER >> 109: #define MTLSD_RT_TEXTURE >> sun_java2d_pipe_hw_AccelSurface_RT_TEXTURE > > Only two of them are supported? MTLSD_TEXTURE and MTLSD_RT_TEXTURE? Also, MTLSD_WINDOW is supported - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 17:26:24 GMT, Alexey Ushakov wrote:
>> 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"?
>
> In fact, we don't have any scratch surfaces. SET_SCRATCH_SURFACE effectively
> sets the new context. For better readability, we should add the new op
> SET_CONTEXT in BufferedOpCodes
Probably it's enough to set the context in SET_SURFACES, I'll double-check it.
>> 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.
>
> We can remove all the unused constants
JDK-8263306
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
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/8f9013c3...5cb1fd91 Looks good, but a couple of things should be fixed (JDK-8263325, JDK-8263324) - Changes requested by avu (no project role). PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:48:47 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 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/af4701dd...5cb1fd91 > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line > 676: > >> 674: height = srcInfo.bounds.y2 - srcInfo.bounds.y1; >> 675: >> 676: pDst = PtrAddBytes(pDst, dstx * dstInfo.pixelStride); > > Here and in other places use the PtrPixelsRow instead, do not use > multiplication, see the history of the OGLBlitLoops_SurfaceToSwBlit method. I think this at least should be investigated before integration. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v11]
On Mon, 8 Mar 2021 08:06:03 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 41 additional > commits since the last revision: > > - Lanai PR#210 - 8263159 - jdv > - Lanai PR#209 - 8262936 - jdv > - Lanai PR#208 - 8262928 - jdv > - Lanai PR#207 - 8262750 - jdv > - Merge branch 'master' into 8260931_lanai_JEP_branch > - 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 > - ... and 31 more: > https://git.openjdk.java.net/jdk/compare/32b236e4...de456939 test/jdk/performance/client/RenderPerfTest/build.xml line 72: > 70: > 71: > 72: This was presumably borrowed from J2DBench so this comment should be fixed ! - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v8]
On Mon, 15 Feb 2021 20:55:13 GMT, Phil Race wrote: >> Marked as reviewed by gziemski (Committer). > >> > > Thanks @gerard-ziemski for taking a detailed look at this. >> > > We do have an open bug to address this. Please refer >> > > [JDK-8259825](https://bugs.openjdk.java.net/browse/JDK-8259825). >> > >> > >> > Hi Gerard, expecting a process and parsing its output is definitely not >> > ideal and that's why there's this open bug. >> > One thing that is under consideration is to equate >= 10.14 with Metal is >> > available since as of 10.14 macOS won't install if a system does not >> > support metal. We have no compelling reason to support metal on earlier >> > releases anyway .. those are either already unsupported or barely >> > supported and OGL will always be available there. >> >> I did not know that there already was a bug covering this issue. >> >> Your idea seems reasonable. >> >> More than just focusing on this immediate issue, however, I was hoping to >> raise the point of us starting adopting profiling tools to analyze our code >> (memory utilization, leaks, cpu/gpu profiling etc). A new feature that >> brings 10% benefit, but uses 50% more resources for example would probably >> not be a good investment. And to figure that we need to relay on some good >> tools and Xcode provides some. > > There were actually tasks to do profiling of the memory footprint and look > for leaks. We did not think it possible to be able to assert "Metal must use > less memory than OpenGL" or dig into tiny differences but it was intended to > find the big issues. See https://bugs.openjdk.java.net/browse/JDK-8237856 > @prsadhuk maybe able to say how much of it was doing using Xcode. Regarding this comment from @mrserb > Probably place it near J2Dbench? I can't find the context but if it is suggesting moving RenderPerfTest to be co-located with J2Bench let's NOT do that. It is much more likely that J2DBench will be moved out of demo into test ... - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:24:54 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 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/ffb3d48c...5cb1fd91
>
> 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"?
In fact, we don't have any scratch surfaces. SET_SCRATCH_SURFACE effectively
sets the new context. For better readability, we should add the new op
SET_CONTEXT in BufferedOpCodes
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v11]
> **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 41 additional commits since the last revision: - Lanai PR#210 - 8263159 - jdv - Lanai PR#209 - 8262936 - jdv - Lanai PR#208 - 8262928 - jdv - Lanai PR#207 - 8262750 - jdv - Merge branch 'master' into 8260931_lanai_JEP_branch - 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 - ... and 31 more: https://git.openjdk.java.net/jdk/compare/582d72a8...de456939 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/5cb1fd91..de456939 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=09-10 Stats: 8128 lines in 291 files changed: 5147 ins; 1823 del; 1158 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
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]
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
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
> **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/dd205a58...5cb1fd91 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/614be056..5cb1fd91 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=08-09 Stats: 14064 lines in 640 files changed: 7890 ins; 3094 del; 3080 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v9]
> **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 29 additional commits since the last revision: - Lanai PR#199 - 8262091 - aghaisas - Lanai PR#198 - 8261646 - avu - Lanai PR#197 - 8261960 - jdv - Lanai PR#196 - 8260715 - avu - Lanai PR#195 - 8261908 - jdv - Lanai PR#194 - 8261703 - jdv - Lanai PR#193 - 8261734 - avu - Lanai PR#192 - 8261789 - aghaisas - Merge branch 'master' into 8260931_lanai_JEP_branch - Lanai PR#191 - 8261705 - jdv - ... and 19 more: https://git.openjdk.java.net/jdk/compare/b17c91cb...614be056 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/7b0b0dc4..614be056 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=07-08 Stats: 11833 lines in 292 files changed: 8413 ins; 2170 del; 1250 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v8]
On Mon, 15 Feb 2021 20:52:09 GMT, Gerard Ziemski 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 20 additional >> commits since the last revision: >> >> - Lanai PR#191 - 8261705 - jdv >> - Lanai PR#190 - 8261706 - jdv >> - Lanai PR#189 - 8261712 - avu >> - Lanai PR#187 - 8261704 - jdv >> - Lanai PR#186 - 8261638 - avu >> - Lanai PR#185 - 8261632 - jdv >> - Lanai PR#184 - 8261620 - aghaisas >> - Lanai PR#182 - 8261547 - psadhukhan >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#181 - 8261143 - aghaisas >> - ... and 10 more: >> https://git.openjdk.java.net/jdk/compare/c8554bef...7b0b0dc4 > > Marked as reviewed by gziemski (Committer). > > > Thanks @gerard-ziemski for taking a detailed look at this. > > > We do have an open bug to address this. Please refer > > > [JDK-8259825](https://bugs.openjdk.java.net/browse/JDK-8259825). > > > > > > Hi Gerard, expecting a process and parsing its output is definitely not > > ideal and that's why there's this open bug. > > One thing that is under consideration is to equate >= 10.14 with Metal is > > available since as of 10.14 macOS won't install if a system does not > > support metal. We have no compelling reason to support metal on earlier > > releases anyway .. those are either already unsupported or barely supported > > and OGL will always be available there. > > I did not know that there already was a bug covering this issue. > > Your idea seems reasonable. > > More than just focusing on this immediate issue, however, I was hoping to > raise the point of us starting adopting profiling tools to analyze our code > (memory utilization, leaks, cpu/gpu profiling etc). A new feature that brings > 10% benefit, but uses 50% more resources for example would probably not be a > good investment. And to figure that we need to relay on some good tools and > Xcode provides some. There were actually tasks to do profiling of the memory footprint and look for leaks. We did not think it possible to be able to assert "Metal must use less memory than OpenGL" or dig into tiny differences but it was intended to find the big issues. See https://bugs.openjdk.java.net/browse/JDK-8237856 @prsadhuk maybe able to say how much of it was doing using Xcode. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v8]
On Mon, 15 Feb 2021 06:24:13 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 20 additional > commits since the last revision: > > - Lanai PR#191 - 8261705 - jdv > - Lanai PR#190 - 8261706 - jdv > - Lanai PR#189 - 8261712 - avu > - Lanai PR#187 - 8261704 - jdv > - Lanai PR#186 - 8261638 - avu > - Lanai PR#185 - 8261632 - jdv > - Lanai PR#184 - 8261620 - aghaisas > - Lanai PR#182 - 8261547 - psadhukhan > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#181 - 8261143 - aghaisas > - ... and 10 more: > https://git.openjdk.java.net/jdk/compare/827ab648...7b0b0dc4 Marked as reviewed by gziemski (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Mon, 15 Feb 2021 18:30:51 GMT, Gerard Ziemski wrote: >> Changes requested by gziemski (Committer). > > I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed > a startup time regression with the Metal rendering pipeline, so I dug a bit > and here is what I found using Xcode startup profiler: > > Here is the OpenGL pipeline: > src="https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> > > > Here is the Metal pipeline: > src="https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> > > If I read the results correctly, there is a weird 330ms time gap in the case > of the Metal pipeline where nothing is done and it looks like the culprit is > `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` > > The OpenGL pipeline gets blocked only for 83ms in > `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` > > I would advice that we take a closer look at why > 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so > long, and optimize it, or cache the results, so the next VM instance can skip > it if needed (assuming the call doesn't end up initializing the native Metal > or something like that), still worth taking a look. > > I would also recommend that you adopt Xcode and its Instruments profiler > tools for future work. Please let me know if you need help in this area. > > I only scratched the surface here and I think that there is plenty of > profiling that can be done to investigate the startup time regression further. > > Thanks @gerard-ziemski for taking a detailed look at this. > > We do have an open bug to address this. Please refer > > [JDK-8259825](https://bugs.openjdk.java.net/browse/JDK-8259825). > > Hi Gerard, expecting a process and parsing its output is definitely not ideal > and that's why there's this open bug. > One thing that is under consideration is to equate >= 10.14 with Metal is > available since as of 10.14 macOS won't install if a system does not support > metal. We have no compelling reason to support metal on earlier releases > anyway .. those are either already unsupported or barely supported and OGL > will always be available there. I did not know that there already was a bug covering this issue. Your idea seems reasonable. More than just focusing on this immediate issue, however, I was hoping to raise the point of us starting adopting profiling tools to analyze our code (memory utilization, leaks, cpu/gpu profiling etc). A new feature that brings 10% benefit, but uses 50% more resources for example would probably not be a good investment. And to figure that we need to relay on some good tools and Xcode provides some. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Mon, 15 Feb 2021 18:30:51 GMT, Gerard Ziemski wrote: >> Changes requested by gziemski (Committer). > > I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed > a startup time regression with the Metal rendering pipeline, so I dug a bit > and here is what I found using Xcode startup profiler: > > Here is the OpenGL pipeline: > src="https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> > > > Here is the Metal pipeline: > src="https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> > > If I read the results correctly, there is a weird 330ms time gap in the case > of the Metal pipeline where nothing is done and it looks like the culprit is > `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` > > The OpenGL pipeline gets blocked only for 83ms in > `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` > > I would advice that we take a closer look at why > 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so > long, and optimize it, or cache the results, so the next VM instance can skip > it if needed (assuming the call doesn't end up initializing the native Metal > or something like that), still worth taking a look. > > I would also recommend that you adopt Xcode and its Instruments profiler > tools for future work. Please let me know if you need help in this area. > > I only scratched the surface here and I think that there is plenty of > profiling that can be done to investigate the startup time regression further. > Thanks @gerard-ziemski for taking a detailed look at this. > We do have an open bug to address this. Please refer > [JDK-8259825](https://bugs.openjdk.java.net/browse/JDK-8259825). Hi Gerard, expecting a process and parsing its output is definitely not ideal and that's why there's this open bug. One thing that is under consideration is to equate >= 10.14 with Metal is available since as of 10.14 macOS won't install if a system does not support metal. We have no compelling reason to support metal on earlier releases anyway .. those are either already unsupported or barely supported and OGL will always be available there. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Mon, 15 Feb 2021 18:30:51 GMT, Gerard Ziemski wrote: >> Changes requested by gziemski (Committer). > > I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed > a startup time regression with the Metal rendering pipeline, so I dug a bit > and here is what I found using Xcode startup profiler: > > Here is the OpenGL pipeline: > src="https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> > > > Here is the Metal pipeline: > src="https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> > > If I read the results correctly, there is a weird 330ms time gap in the case > of the Metal pipeline where nothing is done and it looks like the culprit is > `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` > > The OpenGL pipeline gets blocked only for 83ms in > `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` > > I would advice that we take a closer look at why > 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so > long, and optimize it, or cache the results, so the next VM instance can skip > it if needed (assuming the call doesn't end up initializing the native Metal > or something like that), still worth taking a look. > > I would also recommend that you adopt Xcode and its Instruments profiler > tools for future work. Please let me know if you need help in this area. > > I only scratched the surface here and I think that there is plenty of > profiling that can be done to investigate the startup time regression further. Thanks @gerard-ziemski for taking a detailed look at this. We do have an open bug to address this. Please refer [JDK-8259825](https://bugs.openjdk.java.net/browse/JDK-8259825). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 20:55:47 GMT, Gerard Ziemski wrote: >> Ajit Ghaisas has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Lanai PR#181 - 8261143 - aghaisas >> - Lanai PR#180 - 8261546 - jdv > > Changes requested by gziemski (Committer). I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed a startup time regression with the Metal rendering pipeline, so I dug a bit and here is what I found using Xcode startup profiler: Here is the OpenGL pipeline: https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> Here is the Metal pipeline: https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> If I read the results correctly, there is a weird 330ms time gap in the case of the Metal pipeline where nothing is done and it looks like the culprit is `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` The OpenGL pipeline gets blocked only for 83ms in `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` I would advice that we take a closer look at why 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so long, and optimize it, or cache the results, so the next VM instance can skip it if needed (assuming the call doesn't end up initializing the native Metal or something like that), still worth taking a look. I would also recommend that you adopt Xcode and its Instruments profiler tools for future work. Please let me know if you need help in this area. I only scratched the surface here and I think that there is plenty of profiling that can be done to investigate the startup time regression further. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v8]
> **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 20 additional commits since the last revision: - Lanai PR#191 - 8261705 - jdv - Lanai PR#190 - 8261706 - jdv - Lanai PR#189 - 8261712 - avu - Lanai PR#187 - 8261704 - jdv - Lanai PR#186 - 8261638 - avu - Lanai PR#185 - 8261632 - jdv - Lanai PR#184 - 8261620 - aghaisas - Lanai PR#182 - 8261547 - psadhukhan - Merge branch 'master' into 8260931_lanai_JEP_branch - Lanai PR#181 - 8261143 - aghaisas - ... and 10 more: https://git.openjdk.java.net/jdk/compare/89c20e04...7b0b0dc4 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/7340d067..7b0b0dc4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=06-07 Stats: 6664 lines in 263 files changed: 3414 ins; 1659 del; 1591 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
I have worked out how to pass this option to at least the jtreg tests for the lanai headful mach5 job, so once this is fixed we can check it out in jtreg and get some level of confidence that we are checking all the important cases. Note that we know some tests will fail just because it spits out a message that they will mis-parse but it is still worth doing. Need to figure out the same for JCK .. -phil. On 2/11/21 3:12 PM, Phil Race wrote: On Thu, 11 Feb 2021 21:04:16 GMT, Gerard Ziemski wrote: I guess you will only see this if `Metal API Validation Enabled` Which actually begs a question of whether we tested with `Metal API Validation Enabled` ? I submitted https://bugs.openjdk.java.net/browse/JDK-8261620 for this bug. Could be that there are more such issues but since this crash occurs on start up I can't say what else there might be. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 21:04:16 GMT, Gerard Ziemski wrote: >> I guess you will only see this if `Metal API Validation Enabled` > > Which actually begs a question of whether we tested with `Metal API > Validation Enabled` ? I submitted https://bugs.openjdk.java.net/browse/JDK-8261620 for this bug. Could be that there are more such issues but since this crash occurs on start up I can't say what else there might be. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 20:58:36 GMT, Gerard Ziemski wrote: >> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.m line 112: >> >>> 110: sourceSize:MTLSizeMake(self.buffer.width, >>> self.buffer.height, 1) >>> 111: toTexture:mtlDrawable.texture destinationSlice:0 >>> destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; >>> 112: [blitEncoder endEncoding]; >> >> There is an issue with this code. Running Java2D.jar in Xcode asserts here >> with this message: >> >> 2021-02-11 14:11:45.710457-0600 java[49971:9486360] Metal API Validation >> Enabled >> 2021-02-11 14:11:46.038720-0600 system_profiler[49975:9486885] Metal API >> Validation Enabled >> -[MTLDebugBlitCommandEncoder >> internalValidateCopyFromTexture:sourceSlice:sourceLevel:sourceOrigin:sourceSize:toTexture:destinationSlice:destinationLevel:destinationOrigin:options:]:474: >> failed assertion `(sourceOrigin.y + sourceSize.height)(444) must be <= >> height(400).' >> (lldb) >> >> and forcing the execution past it results in the java process crashing. >> >> A solution would be to clip the src size like this: >> >> NSUInteger src_x = self.leftInset*self.contentsScale; >> NSUInteger src_y = self.topInset*self.contentsScale; >> NSUInteger src_w = self.buffer.width-src_x; >> NSUInteger src_h = self.buffer.height-src_y; >> >> [blitEncoder >> copyFromTexture:self.buffer sourceSlice:0 sourceLevel:0 >> sourceOrigin:MTLOriginMake(src_x, src_y, 0) >> sourceSize:MTLSizeMake(src_w, src_h, 1) >> toTexture:mtlDrawable.texture destinationSlice:0 >> destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; >> [blitEncoder endEncoding]; > > I guess you will only see this if `Metal API Validation Enabled` Which actually begs a question of whether we tested with `Metal API Validation Enabled` ? - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 20:55:35 GMT, Gerard Ziemski wrote: >> Ajit Ghaisas has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Lanai PR#181 - 8261143 - aghaisas >> - Lanai PR#180 - 8261546 - jdv > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.m line 112: > >> 110: sourceSize:MTLSizeMake(self.buffer.width, >> self.buffer.height, 1) >> 111: toTexture:mtlDrawable.texture destinationSlice:0 >> destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; >> 112: [blitEncoder endEncoding]; > > There is an issue with this code. Running Java2D.jar in Xcode asserts here > with this message: > > 2021-02-11 14:11:45.710457-0600 java[49971:9486360] Metal API Validation > Enabled > 2021-02-11 14:11:46.038720-0600 system_profiler[49975:9486885] Metal API > Validation Enabled > -[MTLDebugBlitCommandEncoder > internalValidateCopyFromTexture:sourceSlice:sourceLevel:sourceOrigin:sourceSize:toTexture:destinationSlice:destinationLevel:destinationOrigin:options:]:474: > failed assertion `(sourceOrigin.y + sourceSize.height)(444) must be <= > height(400).' > (lldb) > > and forcing the execution past it results in the java process crashing. > > A solution would be to clip the src size like this: > > NSUInteger src_x = self.leftInset*self.contentsScale; > NSUInteger src_y = self.topInset*self.contentsScale; > NSUInteger src_w = self.buffer.width-src_x; > NSUInteger src_h = self.buffer.height-src_y; > > [blitEncoder > copyFromTexture:self.buffer sourceSlice:0 sourceLevel:0 > sourceOrigin:MTLOriginMake((jint)src_x, (jint)src_y, 0) > sourceSize:MTLSizeMake(src_w, src_h, 1) > toTexture:mtlDrawable.texture destinationSlice:0 > destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; > [blitEncoder endEncoding]; I guess you will only see this if `Metal API Validation Enabled` - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 11:51:57 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 incrementally with two additional > commits since the last revision: > > - Lanai PR#181 - 8261143 - aghaisas > - Lanai PR#180 - 8261546 - jdv Changes requested by gziemski (Committer). src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.m line 112: > 110: sourceSize:MTLSizeMake(self.buffer.width, > self.buffer.height, 1) > 111: toTexture:mtlDrawable.texture destinationSlice:0 > destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; > 112: [blitEncoder endEncoding]; There is an issue with this code. Running Java2D.jar in Xcode asserts here with this message: 2021-02-11 14:11:45.710457-0600 java[49971:9486360] Metal API Validation Enabled 2021-02-11 14:11:46.038720-0600 system_profiler[49975:9486885] Metal API Validation Enabled -[MTLDebugBlitCommandEncoder internalValidateCopyFromTexture:sourceSlice:sourceLevel:sourceOrigin:sourceSize:toTexture:destinationSlice:destinationLevel:destinationOrigin:options:]:474: failed assertion `(sourceOrigin.y + sourceSize.height)(444) must be <= height(400).' (lldb) and forcing the execution past it results in the java process crashing. A solution would be to clip the src size like this: NSUInteger src_x = self.leftInset*self.contentsScale; NSUInteger src_y = self.topInset*self.contentsScale; NSUInteger src_w = self.buffer.width-src_x; NSUInteger src_h = self.buffer.height-src_y; [blitEncoder copyFromTexture:self.buffer sourceSlice:0 sourceLevel:0 sourceOrigin:MTLOriginMake((jint)src_x, (jint)src_y, 0) sourceSize:MTLSizeMake(src_w, src_h, 1) toTexture:mtlDrawable.texture destinationSlice:0 destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; [blitEncoder endEncoding]; - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
> **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 incrementally with two additional commits since the last revision: - Lanai PR#181 - 8261143 - aghaisas - Lanai PR#180 - 8261546 - jdv - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/6a3f96ef..7340d067 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=05-06 Stats: 82 lines in 2 files changed: 0 ins; 54 del; 28 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v6]
On Fri, 5 Feb 2021 22:59:43 GMT, Kevin Rushforth wrote: >> Ajit Ghaisas has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Lanai PR#179 - 8261402 - avu >> - Lanai PR#178 - 8261273 - avu > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLTexurePool.m line > 29: > >> 27: #import "Trace.h" >> 28: >> 29: #define SCREEN_MEMORY_SIZE_4K (4096*2160*4) //~33,7 mb > > This means that a 4k display with a narrower aspect ratio wouldn't fit > (assuming there ever were to be such a thing). What would happen if you > encountered a screen that was, say, 4k * 2.5K? This parameter manages our caching strategy for the temporary texture pool. Huge texture allocations 4K/2 size will cause texture pool drain. We need some more profiling to just these parameters. And I think we need to use Metal API (https://developer.apple.com/documentation/metal/mtldevice/2369280-recommendedmaxworkingsetsize?language=objc) in the future to adjust the amount of memory that we should use. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v4]
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 [v4]
On Wed, 10 Feb 2021 22:31:12 GMT, Gerard Ziemski wrote: >> Marked as reviewed by gziemski (Committer). > > 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! - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v5]
> **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 seven additional commits since the last revision: - Merge branch 'master' into 8260931_lanai_JEP_branch Merge - Lanai PR#177 - 8261430 - aghaisas - Lanai PR#176 - 8261399 - jdv - Lanai PR#175 - 8261304 - aghaisas - Lanai PR#174 - 8261234 - kcr - Merge branch 'master' into 8260931_lanai_JEP_branch Merge from openjdk/jdk - Project Lanai Patch - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/64173289..9a72538a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=03-04 Stats: 6660 lines in 320 files changed: 3899 ins; 1099 del; 1662 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v6]
> **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 incrementally with two additional commits since the last revision: - Lanai PR#179 - 8261402 - avu - Lanai PR#178 - 8261273 - avu - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/9a72538a..6a3f96ef Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=04-05 Stats: 73 lines in 6 files changed: 9 ins; 39 del; 25 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v4]
On Wed, 10 Feb 2021 21:45:37 GMT, Gerard Ziemski wrote: >> Ajit Ghaisas has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Lanai PR#177 - 8261430 - aghaisas >> - Lanai PR#176 - 8261399 - jdv > > Marked as reviewed by gziemski (Committer). 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. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v4]
On Tue, 9 Feb 2021 12:29:52 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 incrementally with two additional > commits since the last revision: > > - Lanai PR#177 - 8261430 - aghaisas > - Lanai PR#176 - 8261399 - jdv Marked as reviewed by gziemski (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
On Wed, 10 Feb 2021 21:40:46 GMT, Gerard Ziemski wrote: >> Changes requested by gziemski (Committer). > > 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! 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: https://user-images.githubusercontent.com/63425797/107575744-dc0ca180-6bb5-11eb-9eb3-5cff415eb8a3.png;> Metal (the green color has a yellowish tint): https://user-images.githubusercontent.com/63425797/107575716-cf884900-6bb5-11eb-8ae8-14212ec94e87.png;> Performance wise I did not see much difference either way. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
On Mon, 8 Feb 2021 23:07:39 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 > > Changes requested by gziemski (Committer). I tried to code review the native implementation file, 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! - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v4]
On Tue, 9 Feb 2021 12:29:52 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 incrementally with two additional > commits since the last revision: > > - Lanai PR#177 - 8261430 - aghaisas > - Lanai PR#176 - 8261399 - jdv src/java.desktop/macosx/classes/sun/java2d/metal/MTLBlitLoops.java line 110: > 108: // MTLSurfaceData.PF_BYTE_GRAY), > 109: // new MTLSwToSurfaceBlit(SurfaceType.UshortGray, > 110: // MTLSurfaceData.PF_USHORT_GRAY), Probably we could remove this commented lines - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
On Mon, 8 Feb 2021 16:53:16 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/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?
OpenGL was the sole rendering pipeline on macOS. Now we are adding Metal
rendering pipeline. There is no software based fall back renderer.
> 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?
No.
0 and 1 are not supported for existing VM options for rendering pipelines. We
won't be adding it for metal pipeline either.
> 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?
OpenGL was the sole rendering pipeline on macOS. Now we are adding Metal
rendering pipeline. There is no software based fall back renderer.
If user requests both - GL and Metal - as false - we shall use 'the default'
pipeline.
Current default render pipeline on macOS is OpenGL.
Changing default render pipeline on macOS is not in the scope of this JEP.
> 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`.
Although it is a good suggestion, I do not recommend this refactoring -
- We do not wish to destabilize proven OpenGL pipeline by refactoring -
additional testing on OpenGL pipeline also would be needed.
- OpenGL pipeline on macOS might be removed once Apple removes OpenGL support
from macOS - this will render this refactoring pointless.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v4]
> **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 two additional commits since the last revision: - Lanai PR#177 - 8261430 - aghaisas - Lanai PR#176 - 8261399 - jdv - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/6044adc0..64173289 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=02-03 Stats: 49 lines in 14 files changed: 0 ins; 43 del; 6 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]
> **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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v2]
> **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 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: - Lanai PR#174 - 8261234 - kcr - Merge branch 'master' into 8260931_lanai_JEP_branch Merge from openjdk/jdk - Project Lanai Patch - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/0b8d96b2..8ed7b5f5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=00-01 Stats: 12194 lines in 245 files changed: 7103 ins; 4454 del; 637 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: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 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) 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. 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). test/jdk/performance/client/RenderPerfTest/src/renderperf/RenderPerfTest.java line 9: > 7: * published by the Free Software Foundation. Oracle designates this > 8: * particular file as subject to the "Classpath" exception as provided > 9: * by Oracle in the LICENSE file that accompanied this code. Since this a test, this should not have the Classpath exception. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 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)
I left a few additional comments. Overall this looks good to me.
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?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m
line 83:
> 81: {
> 82: FILE *f = popen("/usr/sbin/system_profiler SPDisplaysDataType", "r");
> 83: bool metalSupported = JNI_FALSE;
This code is mixing types; it should be `jboolean metalSupported`
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLMaskFill.m line 26:
> 24: */
> 25:
> 26: #ifndef HEADLESS
I see a few occurrences of `#ifndef HEADLESS` in the metal pipeline. Is this
needed? I don't see any of the other native macos files in Java2D (e.g., the
OpenGL pipeline) doing the same. Will this file ever be compiled with HEADLESS
being undefined?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.h line 41:
> 39: /**
> 40: * The MTLPaint class represents paint mode (color, gradient, e.t.c.)
> 41: * */
Minor: `* */` --> `*/`; also typo: `e.t.c.` should be `etc.`
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h
line 37:
> 35:
> 36: /**
> 37: * The MTLSDOps structure describes a native OpenGL surface and contains
> all
Should that be "Metal" and not "OpenGL" ?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLTextRenderer.m line
77:
> 75: * be safe to use this one glyph cache for all screens in a multimon
> 76: * environment, since the glyph cache texture is shared between all
> contexts,
> 77: * and (in theory) OpenGL drivers should be smart enough to manage that
`Metal` drivers?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLTexurePool.m line
29:
> 27: #import "Trace.h"
> 28:
> 29: #define SCREEN_MEMORY_SIZE_4K (4096*2160*4) //~33,7 mb
This means that a 4k display with a narrower aspect ratio wouldn't fit
(assuming there ever were to be such a thing). What would happen if you
encountered a screen that was, say, 4k * 2.5K?
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 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)
Here is my initial set of mostly minor comments and a couple questions.
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.h line 33:
> 31: #import "common.h"
> 32:
> 33: #import
JNF has been removed from the jdk, so this must be removed or it will no longer
compile. It has already been fixed in the lanai repo by
[JDK-8261234](https://bugs.openjdk.java.net/browse/JDK-8261234).
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m
line 38:
> 36: #import
> 37: #import
> 38: #import
JNF has been removed from the jdk, so this must be removed or it will no longer
compile. It has already been fixed in the lanai repo by
[JDK-8261234](https://bugs.openjdk.java.net/browse/JDK-8261234).
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?
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line 2:
> 1: /*
> 2: * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
Should be `2019, 2021,` (I missed this file when I fixed up the copyright
notices and years).
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line 81:
> 79: (PrivilegedAction) () ->
> 80: System.getProperty("java.home", "") + File.separator +
> 81: "lib" + File.separator + "shaders.metallib");
Same question as in the makefile about the name of the shader library file.
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
155:
> 153: if (cfginfo != 0L) {
> 154: textureSize = nativeGetMaxTextureSize();
> 155: // 7160609: GL still fails to create a square texture of
> this
This refers to GL. Is this relevant to metal?
src/java.desktop/macosx/classes/sun/java2d/metal/MTLRenderQueue.java line 78:
> 76: * of the MTL pipeline and related classes.
> 77: */
> 78: public static void sync() {
Should this be synchronized?
src/java.desktop/macosx/classes/sun/java2d/opengl/CGLGraphicsConfig.java line
142:
> 140: // TODO : This clamping code is same as in OpenGL.
> 141: // Whether we need such clamping or not in case of Metal
> 142: // will be pursued under 8260644
This change seems wrong. The comment suggests it belong in a metal class or
method, not here where we are using OpenGL.
src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformView.java line 193:
> 191: responder.handleScrollEvent(x, y, absX, absY,
> event.getModifierFlags(),
> 192: event.getScrollDeltaX(), event.getScrollDeltaY(),
> 193: event.getScrollPhase());
Minor: This part of the file is otherwise unchanged. Maybe revert this
whitespace-only change? Same applies to the last two changes in this file.
src/java.desktop/macosx/classes/sun/lwawt/macosx/CWarningWindow.java line 227:
> 225: CWrapper.NSWindow.orderWindow(ptr,
> 226: CWrapper.NSWindow.NSWindowAbove,
> 227: ownerPtr);
Minor: This part of the file is otherwise unchanged. Maybe revert this
whitespace-only change? Same applies futher down in a couple places.
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 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)
Looks good except for one whitespace problem
(src/java.desktop/share/native/libawt/java2d/SurfaceData.c)
src/java.desktop/share/native/libawt/java2d/SurfaceData.c line 238:
> 236: {
> 237: SurfaceDataOps *ops = malloc(opsSize);
> 238:
The only whitespace added. Should be removed
-
Changes requested by avu (no project role).
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 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) Build changes look good. No opinion on the source changes. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2403
RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
This is the implementation of JEP 382 : New macOS Rendering Pipeline (Metal rendering pipeline) JEP Link - https://bugs.openjdk.java.net/browse/JDK-8238361 - Commit messages: - Project Lanai Patch Changes: https://git.openjdk.java.net/jdk/pull/2403/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2403=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8260931 Stats: 17445 lines in 83 files changed: 17395 ins; 1 del; 49 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
