Re: RFR: 8270116: Expand ButtonGroupLayoutTraversalTest.java to run in all LaFs, including Aqua on macOS
On Tue, 3 Aug 2021 18:30:09 GMT, rajat mahajan
wrote:
>> test/jdk/java/awt/Focus/FocusTraversalPolicy/ButtonGroupLayoutTraversal/ButtonGroupLayoutTraversalTest.java
>> line 54:
>>
>>> 52: public class ButtonGroupLayoutTraversalTest {
>>> 53:
>>> 54: private static final int nx = 3;
>>
>> Normally, as per coding standard, for static final modifier, the variable
>> should be named in CAPS...but is there any need of making it final?
>
> @prsadhuk I did what you asked, do you have any more questions or comments ?,
> if not could you please approve this PR, thanks.
I still think nx, ny should be made CAPS...It seems to be the case for static
final constant variables in java/awt test folder...I don't think it will
increase noise as it will impact only in l57
-
PR: https://git.openjdk.java.net/jdk/pull/4937
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v4]
> This is a request to clean up a desktop module as was done in JDK-8233884 for
> "java.base" module.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> Tested by the desktop headless/headful tests on linux/windows.
Sergey Bylokhov has updated the pull request incrementally with one additional
commit since the last revision:
Update IPPPrintService.java
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4951/files
- new: https://git.openjdk.java.net/jdk/pull/4951/files/e4e82c83..47ed1e81
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4951=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4951=02-03
Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/4951.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4951/head:pull/4951
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v2]
On Tue, 3 Aug 2021 22:42:55 GMT, Alexey Ivanov wrote: >> I am fine to do that, if there are no objections I can change the whole fix. > >> >> I am fine to do that, if there are no objections I can change the whole fix. > > Modifying the entire changeset seems like an overkill. > Using static imports in only one file is _inconsistent_, yet it makes the > places where the encodings are used clearer. Updated - PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v3]
> This is a request to clean up a desktop module as was done in JDK-8233884 for
> "java.base" module.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> Tested by the desktop headless/headful tests on linux/windows.
Sergey Bylokhov has updated the pull request incrementally with one additional
commit since the last revision:
static import StandardCharsets
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4951/files
- new: https://git.openjdk.java.net/jdk/pull/4951/files/886264aa..e4e82c83
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4951=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4951=01-02
Stats: 245 lines in 31 files changed: 93 ins; 63 del; 89 mod
Patch: https://git.openjdk.java.net/jdk/pull/4951.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4951/head:pull/4951
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v2]
On Tue, 3 Aug 2021 22:03:54 GMT, Sergey Bylokhov wrote: > > I am fine to do that, if there are no objections I can change the whole fix. Modifying the entire changeset seems like an overkill. Using static imports in only one file is _inconsistent_, yet it makes the places where the encodings are used clearer. - PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v2]
On Tue, 3 Aug 2021 21:54:08 GMT, Alexey Ivanov wrote: >> it is aligned already, the StandardCharsets.UTF_8 is parameter of "new >> String()", not the getTransferData. > > Ah, right! > But it's confusing: it looks as if `StandardCharsets.UTF_8` is a parameter to > `getTransferData`. Maybe avoid breaking the line and leave `UTF_8` on the > same line? If you import `UTF_8` and `UTF_16LE` statically, line break is > unnecessary. I am fine to do that, if there are no objections I can change the whole fix. - PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v2]
On Tue, 3 Aug 2021 21:39:14 GMT, Sergey Bylokhov wrote: >> src/java.desktop/windows/classes/sun/awt/windows/WDataTransferer.java line >> 270: >> >>> 268: charset = new String((byte[])localeTransferable. >>> 269: getTransferData(javaTextEncodingFlavor), >>> 270: StandardCharsets.UTF_8); >> >> Suggestion: >> >> getTransferData(javaTextEncodingFlavor), >> StandardCharsets.UTF_8); >> >> The parameter on the second line should probably be aligned with the first >> parameter as it's done in the snippet above. > > it is aligned already, the StandardCharsets.UTF_8 is parameter of "new > String()", not the getTransferData. Ah, right! But it's confusing: it looks as if `StandardCharsets.UTF_8` is a parameter to `getTransferData`. Maybe avoid breaking the line and leave `UTF_8` on the same line? If you import `UTF_8` and `UTF_16LE` statically, line break is unnecessary. - PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v2]
On Tue, 3 Aug 2021 21:18:40 GMT, Alexey Ivanov wrote: >> Sergey Bylokhov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains two additional >> commits since the last revision: >> >> - Merge branch 'master' into JDK-8271456 >> - Initial fix for JDK-8271456 > > src/java.desktop/windows/classes/sun/awt/windows/WDataTransferer.java line > 270: > >> 268: charset = new String((byte[])localeTransferable. >> 269: getTransferData(javaTextEncodingFlavor), >> 270: StandardCharsets.UTF_8); > > Suggestion: > > getTransferData(javaTextEncodingFlavor), > StandardCharsets.UTF_8); > > The parameter on the second line should probably be aligned with the first > parameter as it's done in the snippet above. it is aligned already, the StandardCharsets.UTF_8 is parameter of "new String()", not the getTransferData. - PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v2]
On Tue, 3 Aug 2021 19:37:04 GMT, Sergey Bylokhov wrote:
>> This is a request to clean up a desktop module as was done in JDK-8233884
>> for "java.base" module.
>>
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>>
>> This could be done more efficiently(x20 time faster) with use of
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>>
>> The later variant also makes the code cleaner, as it is known not to throw
>> UnsupportedEncodingException in contrary to the former variant.
>>
>> Tested by the desktop headless/headful tests on linux/windows.
>
> Sergey Bylokhov has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains two additional
> commits since the last revision:
>
> - Merge branch 'master' into JDK-8271456
> - Initial fix for JDK-8271456
Marked as reviewed by aivanov (Reviewer).
src/java.desktop/windows/classes/sun/awt/windows/WDataTransferer.java line 270:
> 268: charset = new String((byte[])localeTransferable.
> 269: getTransferData(javaTextEncodingFlavor),
> 270: StandardCharsets.UTF_8);
Suggestion:
getTransferData(javaTextEncodingFlavor),
StandardCharsets.UTF_8);
The parameter on the second line should probably be aligned with the first
parameter as it's done in the snippet above.
-
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL
On Tue, 3 Aug 2021 19:53:22 GMT, Sergey Bylokhov wrote: >> Looks like making MTLLayer opaque by default require much more time that I >> expected. I tried different things to resolve artefacts that I mentioned >> before but nothing seems to work. So, I suggest to stick with my latest >> approach. It passes all the tests and don't break SwingSet2. >> >>> BTW Probably implementation of LWWindowPeer.setTextured() should call the >>> updateOpaque() as well. >> >> I'll add this change > > How to reproduce that artifact? 1. Run SwingSet2 2. Click on all tabs starting from the second one 3. On the tab with swing tree control try to expand some tree nodes -> you'll see black rectangles while performing the clicks It may not be reproduced on a first attempt. On my machine it's reproducible with probability at least 50% - PR: https://git.openjdk.java.net/jdk/pull/4946
Re: RFR: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL
On Tue, 3 Aug 2021 19:51:30 GMT, Alexey Ushakov wrote: >> Yes, sounds reasonable. Moreover, It was my initial attempt but I faced with >> some regressions in SwingSet2 demo. In some cases background of tree nodes >> become black and this artifact was reproducible only if some other tabs were >> clicked before. It was hot time for jdk17 so I decided not to fight with >> this regression. Now we have more time, so I can spend more time on this. > > Looks like making MTLLayer opaque by default require much more time that I > expected. I tried different things to resolve artefacts that I mentioned > before but nothing seems to work. So, I suggest to stick with my latest > approach. It passes all the tests and don't break SwingSet2. > >> BTW Probably implementation of LWWindowPeer.setTextured() should call the >> updateOpaque() as well. > > I'll add this change How to reproduce that artifact? - PR: https://git.openjdk.java.net/jdk/pull/4946
Re: RFR: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL
On Tue, 3 Aug 2021 06:46:07 GMT, Alexey Ushakov wrote: >> I need to look at it closely, I do not understand why we made the native >> layer opaque=NO by default while the java code uses opaque=true by default. >> And then we reset the native code from NO to yes by the code above. Why we >> cannot make both native/java use true/YES and when necessary it will be >> reset to false/NO by the existing code(via LWWindowPeer.setOpaque() and >> LWWindowPeer.updateOpaque?) >> BTW Probably implementation of LWWindowPeer.setTextured() should call the >> updateOpaque() as well. > > Yes, sounds reasonable. Moreover, It was my initial attempt but I faced with > some regressions in SwingSet2 demo. In some cases background of tree nodes > become black and this artifact was reproducible only if some other tabs were > clicked before. It was hot time for jdk17 so I decided not to fight with > this regression. Now we have more time, so I can spend more time on this. Looks like making MTLLayer opaque by default require much more time that I expected. I tried different things to resolve artefacts that I mentioned before but nothing seems to work. So, I suggest to stick with my latest approach. It passes all the tests and don't break SwingSet2. > BTW Probably implementation of LWWindowPeer.setTextured() should call the > updateOpaque() as well. I'll add this change - PR: https://git.openjdk.java.net/jdk/pull/4946
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module [v2]
> This is a request to clean up a desktop module as was done in JDK-8233884 for
> "java.base" module.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> Tested by the desktop headless/headful tests on linux/windows.
Sergey Bylokhov has updated the pull request with a new target base due to a
merge or a rebase. The incremental webrev excludes the unrelated changes
brought in by the merge/rebase. The pull request contains two additional
commits since the last revision:
- Merge branch 'master' into JDK-8271456
- Initial fix for JDK-8271456
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4951/files
- new: https://git.openjdk.java.net/jdk/pull/4951/files/430c9b3a..886264aa
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4951=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4951=00-01
Stats: 1774 lines in 83 files changed: 1230 ins; 337 del; 207 mod
Patch: https://git.openjdk.java.net/jdk/pull/4951.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4951/head:pull/4951
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8270116: Expand ButtonGroupLayoutTraversalTest.java to run in all LaFs, including Aqua on macOS
On Fri, 30 Jul 2021 05:35:58 GMT, Prasanta Sadhukhan
wrote:
>> Summary: Expanded ButtonGroupLayoutTraversalTest.java to run in all LAFs on
>> all OS. Added synchronization for focusCnt.
>
> test/jdk/java/awt/Focus/FocusTraversalPolicy/ButtonGroupLayoutTraversal/ButtonGroupLayoutTraversalTest.java
> line 54:
>
>> 52: public class ButtonGroupLayoutTraversalTest {
>> 53:
>> 54: private static final int nx = 3;
>
> Normally, as per coding standard, for static final modifier, the variable
> should be named in CAPS...but is there any need of making it final?
@prsadhuk I did what you asked, do you have any more questions or comments ?,
if not could you please approve this PR, thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/4937
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module
On Sun, 1 Aug 2021 07:07:21 GMT, Sergey Bylokhov wrote:
> This is a request to clean up a desktop module as was done in JDK-8233884 for
> "java.base" module.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> Tested by the desktop headless/headful tests on linux/windows.
Marked as reviewed by azvegint (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8267385: Create NSAccessibilityElement implementation for JavaComponentAccessibility [v2]
On Tue, 6 Jul 2021 10:43:51 GMT, Pankaj Bansal wrote: >> Artem Semenov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Even on Linux? I meant if the test will be run on the platform where a11y >> is not implemented then this will be > > I tested the NavigableStaticTextAccessibility functionality for TextFields, > TaxtPane and PasswordField. It works fine and I don't see any issue in VO > output with this change. @pankaj-bansal please review the latest revision - PR: https://git.openjdk.java.net/jdk/pull/4412
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module
On Tue, 3 Aug 2021 13:30:28 GMT, Alexander Zvegintsev
wrote:
>> This is a request to clean up a desktop module as was done in JDK-8233884
>> for "java.base" module.
>>
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>>
>> This could be done more efficiently(x20 time faster) with use of
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>>
>> The later variant also makes the code cleaner, as it is known not to throw
>> UnsupportedEncodingException in contrary to the former variant.
>>
>> Tested by the desktop headless/headful tests on linux/windows.
>
> src/java.desktop/share/classes/com/sun/imageio/plugins/wbmp/WBMPMetadata.java
> line 25:
>
>> 23: * questions.
>> 24: */
>> 25:
>
> Since you updating a copyright year in all other files, you probably might
> want to update in this file too.
I did not update it since this is the only file where the imports are updated
only(the UnsupportedEncodingException is removed).
-
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8267385: Create NSAccessibilityElement implementation for JavaComponentAccessibility [v9]
On Mon, 2 Aug 2021 09:00:14 GMT, Artem Semenov wrote: >> 8267385: Create NSAccessibilityElement implementation for >> JavaComponentAccessibility >> This pull request contains solutions for the following tickets: >> * JDK-8267385 Create NSAccessibilityElement implementation for >> JavaComponentAccessibility; >> * JDK-8262031 Create implementation for NSAccessibilityNavigableStaticText >> protocol; >> * JDK-8264287 Create implementation for NSAccessibilityComboBox protocol >> peer; >> * JDK-8264303 Create implementation for NSAccessibilityTabGroup protocol >> peer; >> * JDK-8264292 Create implementation for NSAccessibilityList protocol peer; >> * JDK-8267387 Create implementation for NSAccessibilityOutline protocol; >> * JDK-8267388 Create implementation for NSAccessibilityTable protocol. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > 1. Fixed problems with vertical navigation in table cells; > 2. I suggest doing the opposite, leave the CHECK_EXCEPTION and remove the > chech and the logging. The CHECK_EXCEPTION is better since it can be > configured via properties, and it will chech itself on what thread the > current code is executed. There are still some issues - namely, accessibility cursor got stuck inside combobox popup after popup is closed and voiceover shortcuts such as control option space announced in combobox but do nothing - but they can be fixed in separate bugs. Overall the new API is working and the functional difference can be overcome later in separate changes. - Marked as reviewed by kizune (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4412
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module
On Sun, 1 Aug 2021 07:07:21 GMT, Sergey Bylokhov wrote:
> This is a request to clean up a desktop module as was done in JDK-8233884 for
> "java.base" module.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> Tested by the desktop headless/headful tests on linux/windows.
src/java.desktop/share/classes/com/sun/imageio/plugins/wbmp/WBMPMetadata.java
line 25:
> 23: * questions.
> 24: */
> 25:
Since you updating a copyright year in all other files, you probably might want
to update in this file too.
-
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8270312: Error: Not a test or directory containing tests: java/awt/print/PrinterJob/XparColor.java
On Tue, 13 Jul 2021 14:57:30 GMT, lawrence.andrews wrote: > 1) This testcase was throwing error saying Error: Not a test or directory > containing tests: java/awt/print/PrinterJob/XparColor.java > 2) Added @test tag so that this testcase can be run > 3) Since Applet is deprecated and its not supported any more in the future > release remove applet and make the testcase to run as main > 4) Since this is a manual testcase instruction frame or dailog was not > visible just print dialog was visible so fixed it. > 5) When the testcase was run via main method two print dialog was shown one > was from main method and one more was from constructor. Fixed to show just > one print dialog. > 6) Since JDK 17 introduce metal rendering touching the java2d code thought > this testcase will be useful to checking the rendering. > > @shurymury Since you mentioned about using metal rendering path to verify things. We need to pass -Dsun.java2d.metal=true as of now to run this test case using metal. We can add additional @run with metal enabled for this use case. - PR: https://git.openjdk.java.net/jdk/pull/4769
Re: RFR: 8271456: Avoid looking up standard charsets in "java.desktop" module
On Sun, 1 Aug 2021 07:07:21 GMT, Sergey Bylokhov wrote:
> This is a request to clean up a desktop module as was done in JDK-8233884 for
> "java.base" module.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> Tested by the desktop headless/headful tests on linux/windows.
Marked as reviewed by jdv (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/4951
Re: RFR: 8266079: Lanai: AlphaComposite shows differences on Metal compared to OpenGL
On Mon, 2 Aug 2021 21:53:02 GMT, Sergey Bylokhov wrote: >> Yes, and we need this call here to initialise content view with correct >> default value >> CPlatformWindow.java:931 contentView.setWindowLayerOpaque(isOpaque); > > I need to look at it closely, I do not understand why we made the native > layer opaque=NO by default while the java code uses opaque=true by default. > And then we reset the native code from NO to yes by the code above. Why we > cannot make both native/java use true/YES and when necessary it will be reset > to false/NO by the existing code(via LWWindowPeer.setOpaque() and > LWWindowPeer.updateOpaque?) > BTW Probably implementation of LWWindowPeer.setTextured() should call the > updateOpaque() as well. Yes, sounds reasonable. Moreover, It was my initial attempt but I faced with some regressions in SwingSet2 demo. In some cases background of tree nodes become black and this artifact was reproducible only if some other tabs were clicked before. It was hot time for jdk17 so I decided not to fight with this regression. Now we have more time, so I can spend more time on this. - PR: https://git.openjdk.java.net/jdk/pull/4946
