date:20220520

On Tue, 17 May 2022 20:43:00 GMT, Alexey Ivanov  wrote:

>> Perhaps you are trying to get layout to happen ? That should be possible but 
>> it might be tricky.
>> But I think if you are going so far as pack() and can't change that, then 
>> show the frame regardless.
>
> Right, JPanel isn't laid out without being added to a peered component.
> 
> Maybe setting explicit positions to child panels will work, then the test can 
> be headless.

> That looks odd. I mean if you show the frame, how is it disposed ?

The idea for that is that frame is shown only if `-show` parameter is passed. 
In this case, it's assumed the user runs the test from the IDE or on command 
line. So the frame is disposed of when the user closes it.

-

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

Re: JDK-8274735, JPEG compression in a TIFF container, openjdk/jdk/pull/7849

2022-05-20 Thread Michal Karm

I stepped the flow in a debugger and I narrowed
down the reproducer to a more focused, simpler
description and test data:

https://github.com/Karm/dev-null/blob/main/jpegtiff/README.md

TL;DR:

JPEG compressed RGB image in TIFF with transparency makes
the TIFF decoder to push 4 components data to the JPEG reader.

The JPEG reader wrongly guesses that the data is CMYK now (based on the number
of components).

If the image is GRAYSCALE though, it ends up in Unsupported Image Type
because JPEG reader doesn't try to guess it's CMYK.

Solution...I don't know.

Would it make sense if I transform the data to the number of
components the JPEG reader expects?
All the necessary information is there in the TIFF metadata,
e.g. it knows the space is RGB and not CMYK, it knows that there
is a transparency saved with the tile.

THX for hints

Karm

On 5/17/22 21:52, Philip Race wrote:

This could be a bug in the TIFF plugin.
Hard to say without debugging it.

-phil.

On 5/17/22 7:04 AM, Michal Karm wrote:

Hello,

There used to be an Unsupported Image Type exception thrown
when one wanted to decode a TIFF container with JPEG compressed image.

That behavior has changed, there is no exception now, although the
resulting image is incorrect.

I outlined the details in this doc [1], including a small reproducer.

The test image is not anything from a fuzzer, it's just me
clicking Export As in GIMP.

Thanks for feedback

Cheers
Karm

[1]
https://urldefense.com/v3/__https://github.com/Karm/dev-null/blob/main/jpegtiff/README.md__;!!ACWV5N9M2RV99hQ!Kw41ShsuTSfpVPMZFrXmdQ7ZETNdkLTk9Xd1NDWELvCXV0fyIqg4sXi5Z1aYfjMoG-5fS0gki8dnN8bQjSYM0g$

Karm Michal Babacek

Michal Karm Babacek

--
Sent from my Hosaka Ono-Sendai Cyberspace 7

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v15]

On Fri, 20 May 2022 12:02:42 GMT, Alexey Ivanov  wrote:

>> Right, JPanel isn't laid out without being added to a peered component.
>> 
>> Maybe setting explicit positions to child panels will work, then the test 
>> can be headless.
>
>> That looks odd. I mean if you show the frame, how is it disposed ?
> 
> The idea for that is that frame is shown only if `-show` parameter is passed. 
> In this case, it's assumed the user runs the test from the IDE or on command 
> line. So the frame is disposed of when the user closes it.

> > Perhaps you are trying to get layout to happen ? That should be possible 
> > but it might be tricky.
>
> Right, JPanel isn't laid out without being added to a peered component.
> 
> Maybe setting explicit positions to child panels will work, then the test can 
> be headless.

Indeed, it works with explicitly set size and positions. Here's [a working 
sample](https://github.com/alisenchung/jdk/compare/8279614...aivanov-jdk:alisen-8279614-titledBorder).
 (Add `.diff` or `.patch` to the URL to get the changeset to apply.)

-

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v14]

On Tue, 10 May 2022 14:08:50 GMT, Alexey Ivanov  wrote:

> One more thing: the test now uses `EtchedBorder` only, shall it be moved to 
> `test/jdk/java/awt/EtchedBorder`? And adding the word _Scaled_ to the title — 
> `ScaledEtchedBorderTest` — would clarify the purpose of the test.
> 
> What do you think?

Thank you, Alisen, for renaming the test file. I am also for moving the test 
from `test/jdk/java/awt/TitledBorder` to `test/jdk/java/awt/EtchedBorder` where 
it belongs.

-

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v15]

On Tue, 17 May 2022 20:05:45 GMT, Phil Race  wrote:

> Actually I'm not sure why only Win L&F is tested .. it is cross-platform and 
> cross L&F code being changed

It was exactly my point, we already [discussed it 
above](https://github.com/openjdk/jdk/pull/7449#discussion_r836586231).

The updated test uses `EtchedBorder` explicitly and it is not 
Look-and-Feel-specific, it can run on any platform.

-

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

Re: JDK-8274735, JPEG compression in a TIFF container, openjdk/jdk/pull/7849

2022-05-20 Thread Brett Okken

JPEG support for RGB with transparency/alpha channel is pretty spotty.
https://stackoverflow.com/a/57626892/676877

On Fri, May 20, 2022 at 8:12 AM Michal Karm 
wrote:

> I stepped the flow in a debugger and I narrowed
> down the reproducer to a more focused, simpler
> description and test data:
>
> https://github.com/Karm/dev-null/blob/main/jpegtiff/README.md
>
> TL;DR:
>
> JPEG compressed RGB image in TIFF with transparency makes
> the TIFF decoder to push 4 components data to the JPEG reader.
>
> The JPEG reader wrongly guesses that the data is CMYK now (based on the
> number
> of components).
>
> If the image is GRAYSCALE though, it ends up in Unsupported Image Type
> because JPEG reader doesn't try to guess it's CMYK.
>
>
> Solution...I don't know.
>
> Would it make sense if I transform the data to the number of
> components the JPEG reader expects?
> All the necessary information is there in the TIFF metadata,
> e.g. it knows the space is RGB and not CMYK, it knows that there
> is a transparency saved with the tile.
>
> THX for hints
>
> Karm
>
>
> On 5/17/22 21:52, Philip Race wrote:
> > This could be a bug in the TIFF plugin.
> > Hard to say without debugging it.
> >
> > -phil.
> >
> >
> > On 5/17/22 7:04 AM, Michal Karm wrote:
> >> Hello,
> >>
> >> There used to be an Unsupported Image Type exception thrown
> >> when one wanted to decode a TIFF container with JPEG compressed image.
> >>
> >> That behavior has changed, there is no exception now, although the
> >> resulting image is incorrect.
> >>
> >> I outlined the details in this doc [1], including a small reproducer.
> >>
> >> The test image is not anything from a fuzzer, it's just me
> >> clicking Export As in GIMP.
> >>
> >> Thanks for feedback
> >>
> >> Cheers
> >> Karm
> >>
> >>
> >> [1]
> >>
> https://urldefense.com/v3/__https://github.com/Karm/dev-null/blob/main/jpegtiff/README.md__;!!ACWV5N9M2RV99hQ!Kw41ShsuTSfpVPMZFrXmdQ7ZETNdkLTk9Xd1NDWELvCXV0fyIqg4sXi5Z1aYfjMoG-5fS0gki8dnN8bQjSYM0g$
> >>
> >>
> >> Karm Michal Babacek
> >>
> >
>
>
> Michal Karm Babacek
>
> --
> Sent from my Hosaka Ono-Sendai Cyberspace 7
>

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v16]

2022-05-20 Thread Alisen Chung

> Changed the drawing area to be increased by 0.5 on the left side to prevent 
> clipping

Alisen Chung has updated the pull request incrementally with one additional 
commit since the last revision:

  made suggested changes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7449/files
  - new: https://git.openjdk.java.net/jdk/pull/7449/files/e32e6c08..b17cdab0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7449&range=15
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7449&range=14-15

  Stats: 513 lines in 4 files changed: 255 ins; 248 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7449.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7449/head:pull/7449

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v15]

2022-05-20 Thread Alisen Chung

On Tue, 17 May 2022 20:29:16 GMT, Phil Race  wrote:

>> Alisen Chung has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   renamed test, renamed some methods, updated error messages, updated test
>
> src/java.desktop/share/classes/javax/swing/border/EtchedBorder.java line 164:
> 
>> 162: stkWidth = (int) 
>> Math.floor(Math.min(at.getScaleX(),at.getScaleY()));
>> 163: ((Graphics2D) g).setStroke(new BasicStroke((float) 
>> stkWidth));
>> 164: }
> 
> This reminds me .. did you try STROKE_PURE ? If so does it make any 
> difference ?
> https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/java/awt/RenderingHints.html#KEY_STROKE_CONTROL

I don't think setting it to STROKE_PURE changed anything

-

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v17]

2022-05-20 Thread Alisen Chung

> Changed the drawing area to be increased by 0.5 on the left side to prevent 
> clipping

Alisen Chung has updated the pull request incrementally with one additional 
commit since the last revision:

  reverted copyright year

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7449/files
  - new: https://git.openjdk.java.net/jdk/pull/7449/files/b17cdab0..65200a23

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7449&range=16
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7449&range=15-16

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

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v14]

On Fri, 20 May 2022 13:34:43 GMT, Alexey Ivanov  wrote:

> > One more thing: the test now uses `EtchedBorder` only, shall it be moved to 
> > `test/jdk/java/awt/EtchedBorder`? And adding the word _Scaled_ to the title 
> > — `ScaledEtchedBorderTest` — would clarify the purpose of the test.
> > What do you think?
> 
> Thank you, Alisen, for renaming the test file. I am also for moving the test 
> from `test/jdk/java/awt/TitledBorder` to `test/jdk/java/awt/EtchedBorder` 
> where it belongs.

The test should be under test/jdk/javax/swing/border/ not java/awt !

-

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v17]

On Fri, 20 May 2022 16:55:43 GMT, Alisen Chung  wrote:

>> Changed the drawing area to be increased by 0.5 on the left side to prevent 
>> clipping
>
> Alisen Chung has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted copyright year

test/jdk/java/awt/EtchedBorder/ScaledEtchedBorderTest.java line 77:

> 75: }
> 76: 
> 77: frame.dispose();

the dispose needs to be inside invokeAndWait too .. oh there's more than that
you are creating a frame in each call to createGUI() and yet only ever dispose 
the last frame.
so that call to dispose needs to be an invokeAndWait inside the loop

-

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

Re: JDK-8274735, JPEG compression in a TIFF container, openjdk/jdk/pull/7849

2022-05-20 Thread Philip Race

Identifying the intended color space of a JPEG is so much guesswork ..

and we've not had support for ARGB since JDK 10 which was the last release
to contain the Sun/Oracle "closed" support for that (from Kodak!) and 
OpenJDK NEVER had it.

See
https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/javax/imageio/metadata/doc-files/jpeg_metadata.html

Optional ColorSpace support: Handling of PhotoYCC (YCC), PhotoYCCA 
(YCCA), RGBA and YCbCrA color spaces by the standard plugin, as 
described below, is dependent on capabilities of the libraries used to 
interpret the JPEG data. Thus all consequential behaviors are optional. 
If the support is not available when decoding, the color space will be 
treated as unrecognized and the appropriate default color space for the 
specified number of component channels may be used

So if you have ARGB you are in the lap of the Gods ..

I think before the latest change there was no default 4 component space, 
so you got an exception,

now there's something we default to assuming we are seeing ..

-phil

On 5/20/22 7:07 AM, Brett Okken wrote:

JPEG support for RGB with transparency/alpha channel is pretty spotty.
https://stackoverflow.com/a/57626892/676877

On Fri, May 20, 2022 at 8:12 AM Michal Karm  
wrote:

I stepped the flow in a debugger and I narrowed
down the reproducer to a more focused, simpler
description and test data:

https://github.com/Karm/dev-null/blob/main/jpegtiff/README.md

TL;DR:

JPEG compressed RGB image in TIFF with transparency makes
the TIFF decoder to push 4 components data to the JPEG reader.

The JPEG reader wrongly guesses that the data is CMYK now (based
on the number
of components).

If the image is GRAYSCALE though, it ends up in Unsupported Image Type
because JPEG reader doesn't try to guess it's CMYK.

Solution...I don't know.

Would it make sense if I transform the data to the number of
components the JPEG reader expects?
All the necessary information is there in the TIFF metadata,
e.g. it knows the space is RGB and not CMYK, it knows that there
is a transparency saved with the tile.

THX for hints

Karm

On 5/17/22 21:52, Philip Race wrote:
> This could be a bug in the TIFF plugin.
> Hard to say without debugging it.
>
> -phil.
>
>
> On 5/17/22 7:04 AM, Michal Karm wrote:
>> Hello,
>>
>> There used to be an Unsupported Image Type exception thrown
>> when one wanted to decode a TIFF container with JPEG compressed
image.
>>
>> That behavior has changed, there is no exception now, although the
>> resulting image is incorrect.
>>
>> I outlined the details in this doc [1], including a small
reproducer.
>>
>> The test image is not anything from a fuzzer, it's just me
>> clicking Export As in GIMP.
>>
>> Thanks for feedback
>>
>> Cheers
>> Karm
>>
>>
>> [1]
>>

https://urldefense.com/v3/__https://github.com/Karm/dev-null/blob/main/jpegtiff/README.md__;!!ACWV5N9M2RV99hQ!Kw41ShsuTSfpVPMZFrXmdQ7ZETNdkLTk9Xd1NDWELvCXV0fyIqg4sXi5Z1aYfjMoG-5fS0gki8dnN8bQjSYM0g$

>>
>>
>> Karm Michal Babacek
>>
>

Michal Karm Babacek

-- 
Sent from my Hosaka Ono-Sendai Cyberspace 7

Re: Unexpected EOFException in ImageReaderSpi.canDecodeInput

2022-05-20 Thread Philip Race

OK .. we can probably take the proposed change, but there's no guarantee 
it won't

still happen with 3rd party external plugins.

I've filed the bug for you : 
https://bugs.openjdk.java.net/browse/JDK-8287102


-phil.

On 5/18/22 2:47 AM, Martin Desruisseaux wrote:

Hello Philip

Le 17/05/2022 à 20:36, Philip Race a écrit :


Why is it unexpected ?

The purpose of ImageReaderSpi.canDecodeInput(Object) is to tell if the 
source object seems to be supported by the reader. If the file is too 
small, it is not supported by the reader. So a return value of false 
is what I would expect from the method contract, because the method is 
capable to provide a clear answer to the question "does it seems a 
valid file?"


By contrast, I would expect IOException to be thrown only if an 
unexpected error prevents canDecodeInput(Object) from fulfilling its 
method contract, e.g. a lost of internet connection prevents the 
reader to check the magic number. EOFException does not met that 
criterion for the reason stated above.



The method declares that it throws IOException .. which if thrown 
clearly means the stream can't be de-coded.


In my understanding of the method contract, if the stream can not be 
decoded, this is indicated by a return value of false rather than an 
IOException. In the context of ImageReaderSpi.canDecodeInput(Object), 
I interpret IOException as an error that prevent the method from 
answering the question "does it seems a valid file?"



But this fix is still putting it on each plugin to behave itself .. 
and we already handle that in the ImageIO class
although I think it isn't perfect, as it still relies on co-operation 
with regard to mark()


And if you call this directly, you can handle that just as well 
yourself .. and still can't trust extra
plugins to behave ..,. so what is the use case for this fix ? 


In my case I handle the search of ImageReader myself. Contrarily to 
ImageIO I do not catch IOException, because it may be a serious error 
that prevent the plugin from answering its question. The stream may 
now be in a corrupted state (it is not always a local file; it can be 
on the cloud, a database BLOB, etc), and continuing with other readers 
after the initial error can cause a series of confusing error 
messages. Instead I let the first IOException propagates. The meaning 
is very different than "unsupported format".


I could catch the specific case of EOFException, but I don't because 
1) it relies on assumptions about how the plugin behave, 2) the input 
may be an object other than ImageInputStream that we don't know how to 
mark, and 3) EOFException can in some circumstances be a serious error 
(i.e. EOFException occurring not in the stream given to the 
canDecodeInput(Object) method, but is some auxiliary file that the 
plugin needs to read). Only the plugin knows for sure if EOFException 
means "unsupported format" or "serious error".


    Regards,

    Martin

Re: RFR: 8286270: [java.desktop] Replace color search in XColors with a switch statement [v3]

On Fri, 20 May 2022 00:53:27 GMT, SWinxy  wrote:

>> The current implementation creates ~750 new objects on load, and uses a 
>> binary array search with a Comparable implementation. This implementation is 
>> probably bad. Instead, we can use the enhanced switch feature to possibly 
>> save time, resources, and code size. The new implementation means that the 
>> array doesn't have to be sorted alphanumerically necessarily, and it creates 
>> the colors on demand.
>
> SWinxy has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   Accidental mergers.
>   
>   I created a quick check to see if any other colors were different (only 
> these three). For some odd reason "yellowgreen" and "yellow green" are 
> different.

Marked as reviewed by prr (Reviewer).

-

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v17]

On Fri, 20 May 2022 16:55:43 GMT, Alisen Chung  wrote:

>> Changed the drawing area to be increased by 0.5 on the left side to prevent 
>> clipping
>
> Alisen Chung has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted copyright year

test/jdk/java/awt/EtchedBorder/ScaledEtchedBorderTest.java line 73:

> 71: public static void main(String[] args) throws Exception {
> 72: for (UIManager.LookAndFeelInfo laf : 
> UIManager.getInstalledLookAndFeels()) {
> 73: SwingUtilities.invokeAndWait(() -> setLookAndFeel(laf));

I don't think there's a need to iterate over available Look-and-Feels, you use 
`EtchedBorder` directly. The default L&F or system L&F on each platform is 
enough.

test/jdk/java/awt/EtchedBorder/ScaledEtchedBorderTest.java line 201:

> 199: }
> 200: 
> 201: frame = new JFrame("Swing Test");

Don't you want to go frameless? It would make the test headless. In [this 
comment](https://github.com/openjdk/jdk/pull/7449#discussion_r878130575), I 
provided [sample 
code](https://github.com/alisenchung/jdk/compare/8279614...aivanov-jdk:alisen-8279614-titledBorder)
 which works without creating frame: just panels and borders.

-

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

Re: RFR: 8279614: The left line of the TitledBorder is not painted on 150 scale factor [v14]

On Fri, 20 May 2022 17:35:40 GMT, Phil Race  wrote:

> The test should be under test/jdk/javax/swing/border/ not java/awt !

Good catch! I never noticed the wrong package, I just copied the path.

-

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

Re: RFR: 8275170: Some jtreg sound tests should be marked with sound keyword [v2]

> This fix adds "headful sound" keywords to a number of javax/sound jtreg tests
> 
> The jtreg javax.sound tests are written in such a way that if a needed audio 
> device
> or other platform resource is not available, they just exit with a "pass" for 
> the test.
> It is as if headful tests just swallowed HeadlessException and issued a pass.
> If we allowed that we'd be effectively testing almost nothing of real 
> importance.
> 
> Currently almost  all sound tests have no keyword like "headful" to indicate 
> they
> may need access to a hardware resource to do anything useful so a similar
> situation arises there except when someone runs those tests manually on
> a local system which has audio devices.
> 
> Of course "headful" and "audio device" aren't exactly interchangeable terms
> but if tests are allowed to be run - or worse usually or always run - in a 
> situation
> where they potentially are on a system without an audio device (a headless 
> VM) or are running in
> a session which doesn't have full desktop access - which may in some
> cases be how audio device access is granted, then they are more likely
> to be running in this scenario where the testing isn't valid.
> 
> Another point is that headful tests must be run one at a time - no 
> concurrency because
> you can't have multiple tests moving the mouse at the same time
> Whereas for headless tests, a test harness may choose to run concurrently - 
> perhaps even in the same VM
> When tests that really need access to an audio device are run it is probably 
> safer to consider
> the headful attribute as implying one test at a time is best .. granted on a 
> desktop it isn't
> usual for a single app to be able to monopolize access to a speaker, but for 
> input devices
> that is what you'd generally expect.
> 
> By no means am I sure that the tests being updated here are the full set that 
> need tagging.
> There are a lot of tests and they are all known to pass on systems with NO 
> audio
> devices, so the search has been for tests which call APIs which will 
> definitely
> need access to some device in order to do anything useful.
> So likely there are more to be added - either by a reviewer pointing them out 
> or by later discovery.
> Alternatively we could mark ALL sound tests headful .. but given where we are 
> starting from
> that might be erring unnecessarily far in the opposite direction.
> 
> By adding both headful and sound keywords it gives options to someone who
> wants to identify the tests and perhaps run just tests which need "sound" on 
> some
> config they know supports audio but isn't headful.

Phil Race 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 sound_tests
   Merge master
 - 8275170: Some jtreg sound tests should be marked headful

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6086/files
  - new: https://git.openjdk.java.net/jdk/pull/6086/files/5439d6c7..98d02ae2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6086&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6086&range=00-01

  Stats: 1918540 lines in 11028 files changed: 1139190 ins; 686368 del; 92982 
mod
  Patch: https://git.openjdk.java.net/jdk/pull/6086.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6086/head:pull/6086

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

RFR: 8284690: [macos] VoiceOver : Getting java.lang.IllegalArgumentException: Invalid location on Editable JComboBox

Check for the available range on the document model and adjust text range 
request accordingly.

-

Commit messages:
 - 8284690: [macos] VoiceOver : Getting java.lang.IllegalArgumentException: 
Invalid location on Editable JComboBox

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

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

Re: RFR: 8286266: [macos] Voice over moving JTable column to be the first column JVM crashes [v3]

> Moving cache invalidation from the clearCache method to a createRowWithIndex 
> method
> eliminating race condition that causes crash. Now clearCache just notifies 
> that cache
> is invalid and should be regenerated next time it is being accessed.

Alexander Zuev has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix typo.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8636/files
  - new: https://git.openjdk.java.net/jdk/pull/8636/files/11f25a16..ffee6ee6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8636&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8636&range=01-02

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

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

Re: RFR: 8286266: [macos] Voice over moving JTable column to be the first column JVM crashes [v2]

On Thu, 19 May 2022 19:29:42 GMT, Phil Race  wrote:

>> Alexander Zuev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Add a test case.
>
> test/jdk/java/awt/a11y/AccessibleJTableTest.java line 81:
> 
>> 79: public void  createUIDraggable() {
>> 80: INSTRUCTIONS = "INSTRUCTIONS:\n"
>> 81: + "Check that table is properly updated when culumn 
>> order is changed.\n\n"
> 
> culumn -> column

Oops. Thanks, fixed.

-

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

Integrated: 8286266: [macos] Voice over moving JTable column to be the first column JVM crashes

On Tue, 10 May 2022 17:34:34 GMT, Alexander Zuev  wrote:

> Moving cache invalidation from the clearCache method to a createRowWithIndex 
> method
> eliminating race condition that causes crash. Now clearCache just notifies 
> that cache
> is invalid and should be regenerated next time it is being accessed.

This pull request has now been integrated.

Changeset: b33c6e52
Author:Alexander Zuev 
URL:   
https://git.openjdk.java.net/jdk/commit/b33c6e52c1ba675efdae6e48a9ff022b2b24513c
Stats: 47 lines in 3 files changed: 38 ins; 4 del; 5 mod

8286266: [macos] Voice over moving JTable column to be the first column JVM 
crashes

Reviewed-by: prr

-

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

Re: RFR: 8275170: Some jtreg sound tests should be marked with sound keyword [v2]

On Fri, 20 May 2022 19:25:57 GMT, Phil Race  wrote:

>> This fix adds "headful sound" keywords to a number of javax/sound jtreg tests
>> 
>> The jtreg javax.sound tests are written in such a way that if a needed audio 
>> device
>> or other platform resource is not available, they just exit with a "pass" 
>> for the test.
>> It is as if headful tests just swallowed HeadlessException and issued a pass.
>> If we allowed that we'd be effectively testing almost nothing of real 
>> importance.
>> 
>> Currently almost  all sound tests have no keyword like "headful" to indicate 
>> they
>> may need access to a hardware resource to do anything useful so a similar
>> situation arises there except when someone runs those tests manually on
>> a local system which has audio devices.
>> 
>> Of course "headful" and "audio device" aren't exactly interchangeable terms
>> but if tests are allowed to be run - or worse usually or always run - in a 
>> situation
>> where they potentially are on a system without an audio device (a headless 
>> VM) or are running in
>> a session which doesn't have full desktop access - which may in some
>> cases be how audio device access is granted, then they are more likely
>> to be running in this scenario where the testing isn't valid.
>> 
>> Another point is that headful tests must be run one at a time - no 
>> concurrency because
>> you can't have multiple tests moving the mouse at the same time
>> Whereas for headless tests, a test harness may choose to run concurrently - 
>> perhaps even in the same VM
>> When tests that really need access to an audio device are run it is probably 
>> safer to consider
>> the headful attribute as implying one test at a time is best .. granted on a 
>> desktop it isn't
>> usual for a single app to be able to monopolize access to a speaker, but for 
>> input devices
>> that is what you'd generally expect.
>> 
>> By no means am I sure that the tests being updated here are the full set 
>> that need tagging.
>> There are a lot of tests and they are all known to pass on systems with NO 
>> audio
>> devices, so the search has been for tests which call APIs which will 
>> definitely
>> need access to some device in order to do anything useful.
>> So likely there are more to be added - either by a reviewer pointing them 
>> out or by later discovery.
>> Alternatively we could mark ALL sound tests headful .. but given where we 
>> are starting from
>> that might be erring unnecessarily far in the opposite direction.
>> 
>> By adding both headful and sound keywords it gives options to someone who
>> wants to identify the tests and perhaps run just tests which need "sound" on 
>> some
>> config they know supports audio but isn't headful.
>
> Phil Race 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 sound_tests
>Merge master
>  - 8275170: Some jtreg sound tests should be marked headful

Looks right to me.

-

Marked as reviewed by kizune (Reviewer).

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

Re: RFR: 8275170: Some jtreg sound tests should be marked with sound keyword [v3]

> This fix adds "headful sound" keywords to a number of javax/sound jtreg tests
> 
> The jtreg javax.sound tests are written in such a way that if a needed audio 
> device
> or other platform resource is not available, they just exit with a "pass" for 
> the test.
> It is as if headful tests just swallowed HeadlessException and issued a pass.
> If we allowed that we'd be effectively testing almost nothing of real 
> importance.
> 
> Currently almost  all sound tests have no keyword like "headful" to indicate 
> they
> may need access to a hardware resource to do anything useful so a similar
> situation arises there except when someone runs those tests manually on
> a local system which has audio devices.
> 
> Of course "headful" and "audio device" aren't exactly interchangeable terms
> but if tests are allowed to be run - or worse usually or always run - in a 
> situation
> where they potentially are on a system without an audio device (a headless 
> VM) or are running in
> a session which doesn't have full desktop access - which may in some
> cases be how audio device access is granted, then they are more likely
> to be running in this scenario where the testing isn't valid.
> 
> Another point is that headful tests must be run one at a time - no 
> concurrency because
> you can't have multiple tests moving the mouse at the same time
> Whereas for headless tests, a test harness may choose to run concurrently - 
> perhaps even in the same VM
> When tests that really need access to an audio device are run it is probably 
> safer to consider
> the headful attribute as implying one test at a time is best .. granted on a 
> desktop it isn't
> usual for a single app to be able to monopolize access to a speaker, but for 
> input devices
> that is what you'd generally expect.
> 
> By no means am I sure that the tests being updated here are the full set that 
> need tagging.
> There are a lot of tests and they are all known to pass on systems with NO 
> audio
> devices, so the search has been for tests which call APIs which will 
> definitely
> need access to some device in order to do anything useful.
> So likely there are more to be added - either by a reviewer pointing them out 
> or by later discovery.
> Alternatively we could mark ALL sound tests headful .. but given where we are 
> starting from
> that might be erring unnecessarily far in the opposite direction.
> 
> By adding both headful and sound keywords it gives options to someone who
> wants to identify the tests and perhaps run just tests which need "sound" on 
> some
> config they know supports audio but isn't headful.

Phil Race has updated the pull request incrementally with one additional commit 
since the last revision:

  8275170

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6086/files
  - new: https://git.openjdk.java.net/jdk/pull/6086/files/98d02ae2..b7e0443e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6086&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6086&range=01-02

  Stats: 57 lines in 57 files changed: 0 ins; 0 del; 57 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6086.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6086/head:pull/6086

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

Re: RFR: 8275170: Some jtreg sound tests should be marked with sound keyword [v2]

On Fri, 20 May 2022 19:25:57 GMT, Phil Race  wrote:

>> This fix adds "headful sound" keywords to a number of javax/sound jtreg tests
>> 
>> The jtreg javax.sound tests are written in such a way that if a needed audio 
>> device
>> or other platform resource is not available, they just exit with a "pass" 
>> for the test.
>> It is as if headful tests just swallowed HeadlessException and issued a pass.
>> If we allowed that we'd be effectively testing almost nothing of real 
>> importance.
>> 
>> Currently almost  all sound tests have no keyword like "headful" to indicate 
>> they
>> may need access to a hardware resource to do anything useful so a similar
>> situation arises there except when someone runs those tests manually on
>> a local system which has audio devices.
>> 
>> Of course "headful" and "audio device" aren't exactly interchangeable terms
>> but if tests are allowed to be run - or worse usually or always run - in a 
>> situation
>> where they potentially are on a system without an audio device (a headless 
>> VM) or are running in
>> a session which doesn't have full desktop access - which may in some
>> cases be how audio device access is granted, then they are more likely
>> to be running in this scenario where the testing isn't valid.
>> 
>> Another point is that headful tests must be run one at a time - no 
>> concurrency because
>> you can't have multiple tests moving the mouse at the same time
>> Whereas for headless tests, a test harness may choose to run concurrently - 
>> perhaps even in the same VM
>> When tests that really need access to an audio device are run it is probably 
>> safer to consider
>> the headful attribute as implying one test at a time is best .. granted on a 
>> desktop it isn't
>> usual for a single app to be able to monopolize access to a speaker, but for 
>> input devices
>> that is what you'd generally expect.
>> 
>> By no means am I sure that the tests being updated here are the full set 
>> that need tagging.
>> There are a lot of tests and they are all known to pass on systems with NO 
>> audio
>> devices, so the search has been for tests which call APIs which will 
>> definitely
>> need access to some device in order to do anything useful.
>> So likely there are more to be added - either by a reviewer pointing them 
>> out or by later discovery.
>> Alternatively we could mark ALL sound tests headful .. but given where we 
>> are starting from
>> that might be erring unnecessarily far in the opposite direction.
>> 
>> By adding both headful and sound keywords it gives options to someone who
>> wants to identify the tests and perhaps run just tests which need "sound" on 
>> some
>> config they know supports audio but isn't headful.
>
> Phil Race 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 sound_tests
>Merge master
>  - 8275170: Some jtreg sound tests should be marked headful

The current version only adds the 'sound' keyword.
So it doesn't carry any of the implications of headful.

-

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

Re: RFR: 8285399: JNI exception pending in awt_GraphicsEnv.c:1432 [v2]

On Thu, 19 May 2022 18:34:57 GMT, Phil Race  wrote:

>> This is a theoretical/potential case of calling JNI methods with a possible 
>> execption pending.
>> As noted in the eval of the bug report, we no longer follow the practice 
>> that AWT_LOCK is
>> called only at the start of a native method and AWT_UNLOCK at the end, if 
>> indeed we ever did.
>> But it seems to be implied in how they are handling exceptions.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8285399

Marked as reviewed by serb (Reviewer).

-

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

Re: RFR: 8284690: [macos] VoiceOver : Getting java.lang.IllegalArgumentException: Invalid location on Editable JComboBox

On Fri, 20 May 2022 21:13:35 GMT, Alexander Zuev  wrote:

> Check for the available range on the document model and adjust text range 
> request accordingly.

src/java.desktop/macosx/classes/sun/lwawt/macosx/CAccessibleText.java line 293:

> 291: int currentLength = aet.getCharCount();
> 292: return aet.getTextRange(Math.min(currentLength, 
> location),
> 293: Math.min(currentLength, location + length));

It will be good to update the spec of the AccessibleEditableText by this or 
separate PR, right now it does not mention any exceptions.

-

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

Re: RFR: 8286786: [macos] javax/swing/JInternalFrame/8146321/JInternalFrameIconTest.java still fails

On Fri, 20 May 2022 09:07:29 GMT, Prasanta Sadhukhan  
wrote:

> In PR#[8380](https://github.com/openjdk/jdk/pull/8380) we have reduced the 
> error tolerance from 25 to 1 so no need of exact color check which is causing 
> failure in specific M1 iMac system because of minimalistic color difference 
> of "1"
> 
> 
>  x 0 y 0 red1 171 red2 171 green1 174 green2 175 blue1 184 blue2 184
>  x 0 y 1 red1 172 red2 173 green1 177 green2 177 blue1 185 blue2 185
>  x 0 y 2 red1 173 red2 174 green1 177 green2 178 blue1 187 blue2 187
>  x 0 y 6 red1 0 red2 0 green1 1 green2 0 blue1 0 blue2 0
>  x 0 y 15 red1 1 red2 0 green1 0 green2 0 blue1 0 blue2 0 
> 
> 
> `so modified to use the tolerance check for all L&Fs.

Marked as reviewed by serb (Reviewer).

-

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

Re: RFR: 8275303: sun/java2d/pipe/InterpolationQualityTest.java fails with D3D basic render driver