RFR: 8262981: Create implementation for NSAccessibilitySlider protocol

2021-03-09 Thread Pankaj Bansal
Create implementation of NSAccessibilitySlider protocol 
https://developer.apple.com/documentation/appkit/nsaccessibilityslider

The implementation has the function performIncrement and performDecrement to 
increase/decrease the value of slider using the VoiceOver. To implement this 
functionality, I could think of  following two ways. I have chosen the first 
one here though it is more intrusive.
1. Make the AccessibleJSlider class implement the AccessibleAction interface. 
This makes AccessibleJSlider consistent with the AccessibleJSpinner class. It 
is more clear and the logic to increase/decrease Slider value can be changed 
easily. But this changes AccessibleJSlider class and will need a CSR.
2. Get the current Accessible Value from the component and  just 
increment/decrement it in SliderAccessibility.m  class itself and then set the 
current accessible value fro there only. This will not need any changes in 
AccessibleJSlider class, but this does not look correct way and I have not used 
this.

The changes can be easily tested by using a JSlider example, like the following 
example. VO should announce the correct the slider values. To change the slider 
values, use ctrl+opt+shift+down key to start interacting with the slider, then 
use ctrl+opt+up/down arrow keys to increment/decrement slider values.

  import javax.swing.JFrame;
  import javax.swing.JSlider;
  
  public class JSliderDemo{
  public static void main(String[] args) throws Exception {
  JFrame jframe = new JFrame("JSlider Demo");
  jframe.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
  jframe.add(new JSlider());
  
  jframe.setLocationRelativeTo(null);
  jframe.pack();
  jframe.setVisible(true);
  }
  }

-

Commit messages:
 - Remove whitespaces
 - remove SpinboxAccessibility.m changes
 - Code cleanup
 - cleanup
 - 8262981: Create implementation for NSAccessibilitySlider protocol

Changes: https://git.openjdk.java.net/jdk/pull/2874/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2874&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8262981
  Stats: 74 lines in 4 files changed: 68 ins; 1 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2874.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2874/head:pull/2874

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


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

2021-03-09 Thread Alexey Ushakov
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 [v10]

2021-03-09 Thread Alexey Ushakov
On Sun, 7 Mar 2021 22:29:50 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/69ed64f1...5cb1fd91
>
> 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

-

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


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

2021-03-09 Thread Phil Race
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 [v11]

2021-03-09 Thread Phil Race
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 [v10]

2021-03-09 Thread Sergey Bylokhov
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]

2021-03-09 Thread Sergey Bylokhov
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/851d3def...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?

-

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


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

2021-03-09 Thread Alexey Ivanov
On Sun, 7 Mar 2021 23:16:18 GMT, Sergey Bylokhov  wrote:

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

Marked as reviewed by aivanov (Reviewer).

-

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


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

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

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

This pull request has now been integrated.

Changeset: 86fac952
Author:Sergey Bylokhov 
URL:   https://git.openjdk.java.net/jdk/commit/86fac952
Stats: 220 lines in 3 files changed: 0 ins; 217 del; 3 mod

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

Reviewed-by: kizune, aivanov

-

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


Re: RFR: 8262981: Create implementation for NSAccessibilitySlider protocol

2021-03-09 Thread Alexander Zuev
On Mon, 8 Mar 2021 12:51:56 GMT, Pankaj Bansal  wrote:

> Create implementation of NSAccessibilitySlider protocol 
> https://developer.apple.com/documentation/appkit/nsaccessibilityslider
> 
> The implementation has the function performIncrement and performDecrement to 
> increase/decrease the value of slider using the VoiceOver. To implement this 
> functionality, I could think of  following two ways. I have chosen the first 
> one here though it is more intrusive.
> 1. Make the AccessibleJSlider class implement the AccessibleAction interface. 
> This makes AccessibleJSlider consistent with the AccessibleJSpinner class. It 
> is more clear and the logic to increase/decrease Slider value can be changed 
> easily. But this changes AccessibleJSlider class and will need a CSR.
> 2. Get the current Accessible Value from the component and  just 
> increment/decrement it in SliderAccessibility.m  class itself and then set 
> the current accessible value fro there only. This will not need any changes 
> in AccessibleJSlider class, but this does not look correct way and I have not 
> used this.
> 
> The changes can be easily tested by using a JSlider example, like the 
> following example. VO should announce the correct the slider values. To 
> change the slider values, use ctrl+opt+shift+down key to start interacting 
> with the slider, then use ctrl+opt+up/down arrow keys to increment/decrement 
> slider values.
> 
>   import javax.swing.JFrame;
>   import javax.swing.JSlider;
>   
>   public class JSliderDemo{
>   public static void main(String[] args) throws Exception {
>   JFrame jframe = new JFrame("JSlider Demo");
>   jframe.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>   jframe.add(new JSlider());
>   
>   jframe.setLocationRelativeTo(null);
>   jframe.pack();
>   jframe.setVisible(true);
>   }
>   }

src/java.desktop/macosx/native/libawt_lwawt/awt/a11y/CommonComponentAccessibility.m
 line 59:

> 57: [rolesMap setObject:@"RadiobuttonAccessibility" 
> forKey:@"radiobutton"];
> 58: [rolesMap setObject:@"CheckboxAccessibility" forKey:@"checkbox"];
> 59: [rolesMap setObject:@"SliderAccessibility" forKey:@"slider"];

Shouldn't you also increase the rolesMap initial capacity?

-

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


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

2021-03-09 Thread Alexey Ushakov
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]

2021-03-09 Thread Alexey Ushakov
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]

2021-03-09 Thread Alexey Ushakov
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]

2021-03-09 Thread Alexey Ushakov
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 [v11]

2021-03-09 Thread Alexey Ushakov
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]

2021-03-09 Thread Alexey Ushakov
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 [v10]

2021-03-09 Thread Alexey Ushakov
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]

2021-03-09 Thread Sergey Bylokhov
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: 8182043: Access to Windows Large Icons

2021-03-09 Thread Sergey Bylokhov
On Mon, 8 Mar 2021 13:23:00 GMT, Alexander Zuev  wrote:

>> Fix updated after first round of review.
>
> Continuation of review at https://github.com/openjdk/jdk/pull/380

Just to continue the thread from the old review, did you have a chance to look 
at the possibility of loading all existed icons embedded in the file? It was 
discussed here:
https://github.com/openjdk/jdk/pull/380#issuecomment-702999573

-

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


Re: RFR: 8262981: Create implementation for NSAccessibilitySlider protocol

2021-03-09 Thread Sergey Bylokhov
On Mon, 8 Mar 2021 12:51:56 GMT, Pankaj Bansal  wrote:

> Create implementation of NSAccessibilitySlider protocol 
> https://developer.apple.com/documentation/appkit/nsaccessibilityslider
> 
> The implementation has the function performIncrement and performDecrement to 
> increase/decrease the value of slider using the VoiceOver. To implement this 
> functionality, I could think of  following two ways. I have chosen the first 
> one here though it is more intrusive.
> 1. Make the AccessibleJSlider class implement the AccessibleAction interface. 
> This makes AccessibleJSlider consistent with the AccessibleJSpinner class. It 
> is more clear and the logic to increase/decrease Slider value can be changed 
> easily. But this changes AccessibleJSlider class and will need a CSR.
> 2. Get the current Accessible Value from the component and  just 
> increment/decrement it in SliderAccessibility.m  class itself and then set 
> the current accessible value fro there only. This will not need any changes 
> in AccessibleJSlider class, but this does not look correct way and I have not 
> used this.
> 
> The changes can be easily tested by using a JSlider example, like the 
> following example. VO should announce the correct the slider values. To 
> change the slider values, use ctrl+opt+shift+down key to start interacting 
> with the slider, then use ctrl+opt+up/down arrow keys to increment/decrement 
> slider values.
> 
>   import javax.swing.JFrame;
>   import javax.swing.JSlider;
>   
>   public class JSliderDemo{
>   public static void main(String[] args) throws Exception {
>   JFrame jframe = new JFrame("JSlider Demo");
>   jframe.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>   jframe.add(new JSlider());
>   
>   jframe.setLocationRelativeTo(null);
>   jframe.pack();
>   jframe.setVisible(true);
>   }
>   }

If you need a new functionality and CSR I wonder how it currently works on 
Windows and macOS, or it does not work?

-

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


Re: RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows

2021-03-09 Thread Sergey Bylokhov
On Wed, 10 Mar 2021 01:48:28 GMT, Sergey Bylokhov  wrote:

>> Printing text using GlyphVector outline has bad quality on printers with low 
>> DPI on Windows.
>> The GDI system used for text printing on Windows accepts only integer path 
>> coordinates.
>> Rounding GlyphVector outline coordinates leads to distorted printed text.
>> 
>> The issue had been reported as JDK-8256264 but was reverted because of the 
>> regression JDK-8259007 "This test printed a blank page".
>> 
>> The fix JDK-8256264 scaled coordinates in wPrinterJob.moveTo()/lineTo()  
>> methods up and scaled transforms in wPrinterJob.beginPath()/endPath() down.
>> 
>> The regression was in the WPathGraphics.deviceDrawLine() method which uses 
>> wPrinterJob.moveTo()/lineTo() methods without surrounding them with 
>> wPrinterJob.beginPath()/endPath() so the line coordinates were only scaled 
>> up.
>> 
>> I tried to put wPrinterJob.beginPath()/endPath()  methods around 
>> wPrinterJob.moveTo()/lineTo()   in the method WPathGraphics.deviceDrawLine() 
>>  but the line was not drawn at all even without scaling coordinates up and 
>> transform down (without JDK-8256264 fix). It looks like GDI treats this case 
>> as an empty shape.
>> 
>> The proposed fix applies path coordinates and transform scaling only in 
>> WPathGraphics.convertToWPath() method.
>> The one more PathPrecisionScaleFactorShapeTest.java manual test is added 
>> which checks that all methods that draw paths in WPathGraphics are used: 
>> line in WPathGraphics.deviceDrawLine() and SEG_LINETO/SEG_QUADTO/SEG_CUBICTO 
>> in WPathGraphics.convertToWPath() .
>> 
>> The `java/awt/print` and `java/awt/PrintJob` automatic and manual tests were 
>> run on Windows 10 Pro with the fix.
>> 
>> There are two failed automated tests which fail without the fix as well:
>> java/awt/print/PrinterJob/GlyphPositions.java 
>> java/awt/print/PrinterJob/PSQuestionMark.java
>> 
>> The following manual tests have issues on my system:
>> - `java/awt/print/Dialog/PrintDlgPageable.java` 
>> java.lang.IllegalAccessException: class 
>> com.sun.javatest.regtest.agent.MainWrapper$MainThread cannot access a member 
>> of class PrintDlgPageable with modifiers "public static"
>> 
>> - `java/awt/print/PrinterJob/PrintAttributeUpdateTest.java` I select pages 
>> radio button, press the print button but the test does not finish and I do 
>> not see any other dialogs with pass/fail buttons.
>> 
>> - `java/awt/PrintJob/PrintCheckboxTest/PrintCheckboxManualTest.java` Tests 
>> that there is no ClassCastException thrown in printing checkbox and 
>> scrollbar with XAWT. Error. Can't find HTML file: 
>> test\jdk\java\awt\PrintJob\PrintCheckboxTest\PrintCheckboxManualTest.html
>> 
>> 
>> - `java/awt/print/PrinterJob/SecurityDialogTest.java` A windows with 
>> instructions is shown but it does not contain  print/pass/fail buttons and 
>> it is not possible to close the window.
>> 
>> - The tests below fail with "Error. Parse Exception: Arguments to `manual' 
>> option not supported: yesno" message:
>> java/awt/print/Dialog/DialogOrient.java
>> java/awt/print/Dialog/DialogType.java
>> java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java
>> java/awt/print/PrinterJob/ImagePrinting/ImageTypes.java
>> java/awt/print/PrinterJob/ImagePrinting/PrintARGBImage.java
>> java/awt/print/PrinterJob/PageDialogTest.java
>> java/awt/print/PrinterJob/PageRanges.java
>> java/awt/print/PrinterJob/PageRangesDlgTest.java
>> java/awt/print/PrinterJob/PrintGlyphVectorTest.java
>> java/awt/print/PrinterJob/PrintLatinCJKTest.java
>> java/awt/print/PrinterJob/PrintTextTest.java
>> java/awt/print/PrinterJob/SwingUIText.java
>> java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java
>> java/awt/PrintJob/PageSetupDlgBlockingTest/PageSetupDlgBlockingTest.java
>> java/awt/PrintJob/SaveDialogTitleTest.java
>
> Marked as reviewed by serb (Reviewer).

@prsadhuk please take a look

-

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


Re: RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows

2021-03-09 Thread Sergey Bylokhov
On Fri, 26 Feb 2021 19:40:22 GMT, Alexander Scherbatiy  
wrote:

> Printing text using GlyphVector outline has bad quality on printers with low 
> DPI on Windows.
> The GDI system used for text printing on Windows accepts only integer path 
> coordinates.
> Rounding GlyphVector outline coordinates leads to distorted printed text.
> 
> The issue had been reported as JDK-8256264 but was reverted because of the 
> regression JDK-8259007 "This test printed a blank page".
> 
> The fix JDK-8256264 scaled coordinates in wPrinterJob.moveTo()/lineTo()  
> methods up and scaled transforms in wPrinterJob.beginPath()/endPath() down.
> 
> The regression was in the WPathGraphics.deviceDrawLine() method which uses 
> wPrinterJob.moveTo()/lineTo() methods without surrounding them with 
> wPrinterJob.beginPath()/endPath() so the line coordinates were only scaled up.
> 
> I tried to put wPrinterJob.beginPath()/endPath()  methods around 
> wPrinterJob.moveTo()/lineTo()   in the method WPathGraphics.deviceDrawLine()  
> but the line was not drawn at all even without scaling coordinates up and 
> transform down (without JDK-8256264 fix). It looks like GDI treats this case 
> as an empty shape.
> 
> The proposed fix applies path coordinates and transform scaling only in 
> WPathGraphics.convertToWPath() method.
> The one more PathPrecisionScaleFactorShapeTest.java manual test is added 
> which checks that all methods that draw paths in WPathGraphics are used: line 
> in WPathGraphics.deviceDrawLine() and SEG_LINETO/SEG_QUADTO/SEG_CUBICTO in 
> WPathGraphics.convertToWPath() .
> 
> The `java/awt/print` and `java/awt/PrintJob` automatic and manual tests were 
> run on Windows 10 Pro with the fix.
> 
> There are two failed automated tests which fail without the fix as well:
> java/awt/print/PrinterJob/GlyphPositions.java 
> java/awt/print/PrinterJob/PSQuestionMark.java
> 
> The following manual tests have issues on my system:
> - `java/awt/print/Dialog/PrintDlgPageable.java` 
> java.lang.IllegalAccessException: class 
> com.sun.javatest.regtest.agent.MainWrapper$MainThread cannot access a member 
> of class PrintDlgPageable with modifiers "public static"
> 
> - `java/awt/print/PrinterJob/PrintAttributeUpdateTest.java` I select pages 
> radio button, press the print button but the test does not finish and I do 
> not see any other dialogs with pass/fail buttons.
> 
> - `java/awt/PrintJob/PrintCheckboxTest/PrintCheckboxManualTest.java` Tests 
> that there is no ClassCastException thrown in printing checkbox and scrollbar 
> with XAWT. Error. Can't find HTML file: 
> test\jdk\java\awt\PrintJob\PrintCheckboxTest\PrintCheckboxManualTest.html
> 
> 
> - `java/awt/print/PrinterJob/SecurityDialogTest.java` A windows with 
> instructions is shown but it does not contain  print/pass/fail buttons and it 
> is not possible to close the window.
> 
> - The tests below fail with "Error. Parse Exception: Arguments to `manual' 
> option not supported: yesno" message:
> java/awt/print/Dialog/DialogOrient.java
> java/awt/print/Dialog/DialogType.java
> java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java
> java/awt/print/PrinterJob/ImagePrinting/ImageTypes.java
> java/awt/print/PrinterJob/ImagePrinting/PrintARGBImage.java
> java/awt/print/PrinterJob/PageDialogTest.java
> java/awt/print/PrinterJob/PageRanges.java
> java/awt/print/PrinterJob/PageRangesDlgTest.java
> java/awt/print/PrinterJob/PrintGlyphVectorTest.java
> java/awt/print/PrinterJob/PrintLatinCJKTest.java
> java/awt/print/PrinterJob/PrintTextTest.java
> java/awt/print/PrinterJob/SwingUIText.java
> java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java
> java/awt/PrintJob/PageSetupDlgBlockingTest/PageSetupDlgBlockingTest.java
> java/awt/PrintJob/SaveDialogTitleTest.java

Marked as reviewed by serb (Reviewer).

-

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


Re: RFR: 8182043: Access to Windows Large Icons

2021-03-09 Thread Alexander Zuev
On Wed, 10 Mar 2021 00:55:00 GMT, Sergey Bylokhov  wrote:

>> Continuation of review at https://github.com/openjdk/jdk/pull/380
>
> Just to continue the thread from the old review, did you have a chance to 
> look at the possibility of loading all existed icons embedded in the file? It 
> was discussed here:
> https://github.com/openjdk/jdk/pull/380#issuecomment-702999573

> Just to continue the thread from the old review, did you have a chance to 
> look at the possibility of loading all existed icons embedded in the file? It 
> was discussed here:
> [#380 
> (comment)](https://github.com/openjdk/jdk/pull/380#issuecomment-702999573)

Yes, i did. There were 3 shortcomings: 1. It works in about 10% of the cases in 
all other cases it reports only one icon available (32x32) even if there are 
other resolution icons. 2. It is quite  slow, especially on network drives. 3. 
Does not work at all in files that are within AppData\Roaming folder - 
returning list is just empty.

-

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