Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v4]

2021-06-09 Thread Sergey Bylokhov
On Tue, 8 Jun 2021 21:45:43 GMT, Phil Race  wrote:

>> There are two issues here, both macOS specific.
>> First the original reported one that occurs when running on a shared remote 
>> VNC type desktop on macOS.
>> Only a single supported display mode is returned and it is also the current 
>> mode.
>> A program that simply enumerates the reported modes and tries to set them, 
>> if it reaches the native layer,
>> will fail because macOS reports the same display mode it returned as valid 
>> as now being illegal.
>> It does not appear to be as simple as a user's permission level since it 
>> occurs with an admin user as well
>> so it probably is the case that macOS simply does not allow this shared 
>> desktop to be changed although
>> there's no docs I can find
>> Ordinarily we would not have tried this since we test if the requested 
>> display mode is the same as the native
>> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
>> allowed as meaning match
>> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
>> so the code is enhanced to check for that
>> 
>> Second, when creating a test it was discovered that *at least* on the 
>> built-in retina display of a 16" macbook pro,
>> the system default contig is never in the list of available modes and can be 
>> discovered only by querying the *current* mode
>> before any modes are changed. This beheaviour is 100% consistent no matter 
>> if the system was just rebooted, has
>> an external monitor attached as well, or not, activates the discrete card, 
>> or not.
>> 
>> Since we've had code since at least 2013 that added the default mode I 
>> suspect this has been seen before but nowhere
>> can I find an explanation.
>> 
>> It has odd consequences like after you change the display mode, you will no 
>> longer see the default display mode reported,
>> so the list of available modes is reduced by one.
>> But for a typical use case which doesn't re-query or more typically caches 
>> the original display mode in order to restore it, it presents a bigger 
>> problem that trying to restore will always fail. 
>> 
>> So the other thing this fix does is detect when we fail to restore the 
>> initial mode and try again in a different way.
>> Only if that fails do we then throw an exception.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
> Unable to set display mode

Marked as reviewed by serb (Reviewer).

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v3]

2021-06-08 Thread Phil Race
On Tue, 8 Jun 2021 17:08:54 GMT, Victor Dyakov  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
>> Unable to set display mode
>
> please review @mrserb and @azvegint

@victordyakov @mrserb white space is fixed now

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v4]

2021-06-08 Thread Phil Race
> There are two issues here, both macOS specific.
> First the original reported one that occurs when running on a shared remote 
> VNC type desktop on macOS.
> Only a single supported display mode is returned and it is also the current 
> mode.
> A program that simply enumerates the reported modes and tries to set them, if 
> it reaches the native layer,
> will fail because macOS reports the same display mode it returned as valid as 
> now being illegal.
> It does not appear to be as simple as a user's permission level since it 
> occurs with an admin user as well
> so it probably is the case that macOS simply does not allow this shared 
> desktop to be changed although
> there's no docs I can find
> Ordinarily we would not have tried this since we test if the requested 
> display mode is the same as the native
> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
> allowed as meaning match
> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
> so the code is enhanced to check for that
> 
> Second, when creating a test it was discovered that *at least* on the 
> built-in retina display of a 16" macbook pro,
> the system default contig is never in the list of available modes and can be 
> discovered only by querying the *current* mode
> before any modes are changed. This beheaviour is 100% consistent no matter if 
> the system was just rebooted, has
> an external monitor attached as well, or not, activates the discrete card, or 
> not.
> 
> Since we've had code since at least 2013 that added the default mode I 
> suspect this has been seen before but nowhere
> can I find an explanation.
> 
> It has odd consequences like after you change the display mode, you will no 
> longer see the default display mode reported,
> so the list of available modes is reduced by one.
> But for a typical use case which doesn't re-query or more typically caches 
> the original display mode in order to restore it, it presents a bigger 
> problem that trying to restore will always fail. 
> 
> So the other thing this fix does is detect when we fail to restore the 
> initial mode and try again in a different way.
> Only if that fails do we then throw an exception.

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

  8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
Unable to set display mode

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4373/files
  - new: https://git.openjdk.java.net/jdk/pull/4373/files/3a496412..2289c30d

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

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

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode!

2021-06-08 Thread Sergey Bylokhov
On Mon, 7 Jun 2021 15:13:27 GMT, Phil Race  wrote:

>> There are two issues here, both macOS specific.
>> First the original reported one that occurs when running on a shared remote 
>> VNC type desktop on macOS.
>> Only a single supported display mode is returned and it is also the current 
>> mode.
>> A program that simply enumerates the reported modes and tries to set them, 
>> if it reaches the native layer,
>> will fail because macOS reports the same display mode it returned as valid 
>> as now being illegal.
>> It does not appear to be as simple as a user's permission level since it 
>> occurs with an admin user as well
>> so it probably is the case that macOS simply does not allow this shared 
>> desktop to be changed although
>> there's no docs I can find
>> Ordinarily we would not have tried this since we test if the requested 
>> display mode is the same as the native
>> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
>> allowed as meaning match
>> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
>> so the code is enhanced to check for that
>> 
>> Second, when creating a test it was discovered that *at least* on the 
>> built-in retina display of a 16" macbook pro,
>> the system default contig is never in the list of available modes and can be 
>> discovered only by querying the *current* mode
>> before any modes are changed. This beheaviour is 100% consistent no matter 
>> if the system was just rebooted, has
>> an external monitor attached as well, or not, activates the discrete card, 
>> or not.
>> 
>> Since we've had code since at least 2013 that added the default mode I 
>> suspect this has been seen before but nowhere
>> can I find an explanation.
>> 
>> It has odd consequences like after you change the display mode, you will no 
>> longer see the default display mode reported,
>> so the list of available modes is reduced by one.
>> But for a typical use case which doesn't re-query or more typically caches 
>> the original display mode in order to restore it, it presents a bigger 
>> problem that trying to restore will always fail. 
>> 
>> So the other thing this fix does is detect when we fail to restore the 
>> initial mode and try again in a different way.
>> Only if that fails do we then throw an exception.
>
> fixed typo in comment

@prrace @victordyakov looks like "removed the rfr label yesterday" due to 
jcheck failure, so the fix is not under review right now.
" @openjdk openjdk / jcheck
Whitespace error
Column 23: trailing whitespace"

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v2]

2021-06-08 Thread Sergey Bylokhov
On Mon, 7 Jun 2021 19:50:33 GMT, Sergey Bylokhov  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
>> Unable to set display mode
>
> test/jdk/java/awt/GraphicsDevice/DisplayModes/UnknownRefrshRateTest.java line 
> 67:
> 
>> 65:  } finally {
>> 66:  System.out.println("restoring original mode"+odm);
>> 67:  d.setDisplayMode(odm);
> 
> I suggest waiting here as well.

I still suggest waiting here, other tests were updated to wait 10 seconds, 
since on the old systems the last restore may affect the next test.

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v3]

2021-06-08 Thread Victor Dyakov
On Mon, 7 Jun 2021 20:22:44 GMT, Phil Race  wrote:

>> There are two issues here, both macOS specific.
>> First the original reported one that occurs when running on a shared remote 
>> VNC type desktop on macOS.
>> Only a single supported display mode is returned and it is also the current 
>> mode.
>> A program that simply enumerates the reported modes and tries to set them, 
>> if it reaches the native layer,
>> will fail because macOS reports the same display mode it returned as valid 
>> as now being illegal.
>> It does not appear to be as simple as a user's permission level since it 
>> occurs with an admin user as well
>> so it probably is the case that macOS simply does not allow this shared 
>> desktop to be changed although
>> there's no docs I can find
>> Ordinarily we would not have tried this since we test if the requested 
>> display mode is the same as the native
>> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
>> allowed as meaning match
>> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
>> so the code is enhanced to check for that
>> 
>> Second, when creating a test it was discovered that *at least* on the 
>> built-in retina display of a 16" macbook pro,
>> the system default contig is never in the list of available modes and can be 
>> discovered only by querying the *current* mode
>> before any modes are changed. This beheaviour is 100% consistent no matter 
>> if the system was just rebooted, has
>> an external monitor attached as well, or not, activates the discrete card, 
>> or not.
>> 
>> Since we've had code since at least 2013 that added the default mode I 
>> suspect this has been seen before but nowhere
>> can I find an explanation.
>> 
>> It has odd consequences like after you change the display mode, you will no 
>> longer see the default display mode reported,
>> so the list of available modes is reduced by one.
>> But for a typical use case which doesn't re-query or more typically caches 
>> the original display mode in order to restore it, it presents a bigger 
>> problem that trying to restore will always fail. 
>> 
>> So the other thing this fix does is detect when we fail to restore the 
>> initial mode and try again in a different way.
>> Only if that fails do we then throw an exception.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
> Unable to set display mode

please review @mrserb and @azvegint

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v3]

2021-06-07 Thread Phil Race
> There are two issues here, both macOS specific.
> First the original reported one that occurs when running on a shared remote 
> VNC type desktop on macOS.
> Only a single supported display mode is returned and it is also the current 
> mode.
> A program that simply enumerates the reported modes and tries to set them, if 
> it reaches the native layer,
> will fail because macOS reports the same display mode it returned as valid as 
> now being illegal.
> It does not appear to be as simple as a user's permission level since it 
> occurs with an admin user as well
> so it probably is the case that macOS simply does not allow this shared 
> desktop to be changed although
> there's no docs I can find
> Ordinarily we would not have tried this since we test if the requested 
> display mode is the same as the native
> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
> allowed as meaning match
> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
> so the code is enhanced to check for that
> 
> Second, when creating a test it was discovered that *at least* on the 
> built-in retina display of a 16" macbook pro,
> the system default contig is never in the list of available modes and can be 
> discovered only by querying the *current* mode
> before any modes are changed. This beheaviour is 100% consistent no matter if 
> the system was just rebooted, has
> an external monitor attached as well, or not, activates the discrete card, or 
> not.
> 
> Since we've had code since at least 2013 that added the default mode I 
> suspect this has been seen before but nowhere
> can I find an explanation.
> 
> It has odd consequences like after you change the display mode, you will no 
> longer see the default display mode reported,
> so the list of available modes is reduced by one.
> But for a typical use case which doesn't re-query or more typically caches 
> the original display mode in order to restore it, it presents a bigger 
> problem that trying to restore will always fail. 
> 
> So the other thing this fix does is detect when we fail to restore the 
> initial mode and try again in a different way.
> Only if that fails do we then throw an exception.

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

  8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
Unable to set display mode

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4373/files
  - new: https://git.openjdk.java.net/jdk/pull/4373/files/d93be8fa..3a496412

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

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

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v2]

2021-06-07 Thread Phil Race
On Mon, 7 Jun 2021 19:45:49 GMT, Sergey Bylokhov  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
>> Unable to set display mode
>
> src/java.desktop/macosx/classes/sun/awt/CGraphicsDevice.java line 72:
> 
>> 70: public CGraphicsDevice(final int displayID) {
>> 71: this.displayID = displayID;
>> 72: this.initialMode = getDisplayMode();
> 
> This probably should be revalidated when this device is invalidated, 
> otherwise deleted device will restore its own old-initial mode, instead of 
> the new-mode for the new device.

Ok I've added copying it in invalidate()

> src/java.desktop/macosx/classes/sun/awt/CGraphicsDevice.java line 341:
> 
>> 339:  * that mode reported and it restores all devices, but
>> 340:  * this seems a better compromise than failing to 
>> restore
>> 341:  */
> 
> Would like to highlight that this tradeoff will break the spec, since we 
> successfully restore the mode which is not in the list of modes.

So .. it was in the lsit of modes that the app was handed, and it is a bug that 
it isn't still there.
But what I've just done is fixed getDIsplayModes() to  include it

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v2]

2021-06-07 Thread Sergey Bylokhov
On Mon, 7 Jun 2021 15:17:48 GMT, Phil Race  wrote:

>> There are two issues here, both macOS specific.
>> First the original reported one that occurs when running on a shared remote 
>> VNC type desktop on macOS.
>> Only a single supported display mode is returned and it is also the current 
>> mode.
>> A program that simply enumerates the reported modes and tries to set them, 
>> if it reaches the native layer,
>> will fail because macOS reports the same display mode it returned as valid 
>> as now being illegal.
>> It does not appear to be as simple as a user's permission level since it 
>> occurs with an admin user as well
>> so it probably is the case that macOS simply does not allow this shared 
>> desktop to be changed although
>> there's no docs I can find
>> Ordinarily we would not have tried this since we test if the requested 
>> display mode is the same as the native
>> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
>> allowed as meaning match
>> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
>> so the code is enhanced to check for that
>> 
>> Second, when creating a test it was discovered that *at least* on the 
>> built-in retina display of a 16" macbook pro,
>> the system default contig is never in the list of available modes and can be 
>> discovered only by querying the *current* mode
>> before any modes are changed. This beheaviour is 100% consistent no matter 
>> if the system was just rebooted, has
>> an external monitor attached as well, or not, activates the discrete card, 
>> or not.
>> 
>> Since we've had code since at least 2013 that added the default mode I 
>> suspect this has been seen before but nowhere
>> can I find an explanation.
>> 
>> It has odd consequences like after you change the display mode, you will no 
>> longer see the default display mode reported,
>> so the list of available modes is reduced by one.
>> But for a typical use case which doesn't re-query or more typically caches 
>> the original display mode in order to restore it, it presents a bigger 
>> problem that trying to restore will always fail. 
>> 
>> So the other thing this fix does is detect when we fail to restore the 
>> initial mode and try again in a different way.
>> Only if that fails do we then throw an exception.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
> Unable to set display mode

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

> 339:  * that mode reported and it restores all devices, but
> 340:  * this seems a better compromise than failing to restore
> 341:  */

Would like to highlight that this tradeoff will break the spec, since we 
successfully restore the mode which is not in the list of modes.

test/jdk/java/awt/GraphicsDevice/DisplayModes/UnknownRefrshRateTest.java line 
67:

> 65:  } finally {
> 66:  System.out.println("restoring original mode"+odm);
> 67:  d.setDisplayMode(odm);

I suggest waiting here as well.

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v2]

2021-06-07 Thread Sergey Bylokhov
On Mon, 7 Jun 2021 15:17:48 GMT, Phil Race  wrote:

>> There are two issues here, both macOS specific.
>> First the original reported one that occurs when running on a shared remote 
>> VNC type desktop on macOS.
>> Only a single supported display mode is returned and it is also the current 
>> mode.
>> A program that simply enumerates the reported modes and tries to set them, 
>> if it reaches the native layer,
>> will fail because macOS reports the same display mode it returned as valid 
>> as now being illegal.
>> It does not appear to be as simple as a user's permission level since it 
>> occurs with an admin user as well
>> so it probably is the case that macOS simply does not allow this shared 
>> desktop to be changed although
>> there's no docs I can find
>> Ordinarily we would not have tried this since we test if the requested 
>> display mode is the same as the native
>> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
>> allowed as meaning match
>> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
>> so the code is enhanced to check for that
>> 
>> Second, when creating a test it was discovered that *at least* on the 
>> built-in retina display of a 16" macbook pro,
>> the system default contig is never in the list of available modes and can be 
>> discovered only by querying the *current* mode
>> before any modes are changed. This beheaviour is 100% consistent no matter 
>> if the system was just rebooted, has
>> an external monitor attached as well, or not, activates the discrete card, 
>> or not.
>> 
>> Since we've had code since at least 2013 that added the default mode I 
>> suspect this has been seen before but nowhere
>> can I find an explanation.
>> 
>> It has odd consequences like after you change the display mode, you will no 
>> longer see the default display mode reported,
>> so the list of available modes is reduced by one.
>> But for a typical use case which doesn't re-query or more typically caches 
>> the original display mode in order to restore it, it presents a bigger 
>> problem that trying to restore will always fail. 
>> 
>> So the other thing this fix does is detect when we fail to restore the 
>> initial mode and try again in a different way.
>> Only if that fails do we then throw an exception.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
> Unable to set display mode

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

> 70: public CGraphicsDevice(final int displayID) {
> 71: this.displayID = displayID;
> 72: this.initialMode = getDisplayMode();

This probably should be revalidated when this device is invalidated, otherwise 
deleted device will restore its own old-initial mode, instead of the new-mode 
for the new device.

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v2]

2021-06-07 Thread Phil Race
On Mon, 7 Jun 2021 16:07:20 GMT, Alexander Zvegintsev  
wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
>> Unable to set display mode
>
> test/jdk/java/awt/GraphicsDevice/DisplayModes/UnknownRefrshRateTest.java line 
> 44:
> 
>> 42: for (GraphicsDevice d : devices) {
>> 43: 
>> 44: if (!d.isDisplayChangeSupported()) {
> 
> BTW, do we want to run this test on other platforms?
> 
> If so, we need to modify test. Because `isDisplayChangeSupported()` may 
> return true on Windows and Linux only if we have a fullscreen window showing.

I don't see the harm.

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v2]

2021-06-07 Thread Alexander Zvegintsev
On Mon, 7 Jun 2021 15:17:48 GMT, Phil Race  wrote:

>> There are two issues here, both macOS specific.
>> First the original reported one that occurs when running on a shared remote 
>> VNC type desktop on macOS.
>> Only a single supported display mode is returned and it is also the current 
>> mode.
>> A program that simply enumerates the reported modes and tries to set them, 
>> if it reaches the native layer,
>> will fail because macOS reports the same display mode it returned as valid 
>> as now being illegal.
>> It does not appear to be as simple as a user's permission level since it 
>> occurs with an admin user as well
>> so it probably is the case that macOS simply does not allow this shared 
>> desktop to be changed although
>> there's no docs I can find
>> Ordinarily we would not have tried this since we test if the requested 
>> display mode is the same as the native
>> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
>> allowed as meaning match
>> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
>> so the code is enhanced to check for that
>> 
>> Second, when creating a test it was discovered that *at least* on the 
>> built-in retina display of a 16" macbook pro,
>> the system default contig is never in the list of available modes and can be 
>> discovered only by querying the *current* mode
>> before any modes are changed. This beheaviour is 100% consistent no matter 
>> if the system was just rebooted, has
>> an external monitor attached as well, or not, activates the discrete card, 
>> or not.
>> 
>> Since we've had code since at least 2013 that added the default mode I 
>> suspect this has been seen before but nowhere
>> can I find an explanation.
>> 
>> It has odd consequences like after you change the display mode, you will no 
>> longer see the default display mode reported,
>> so the list of available modes is reduced by one.
>> But for a typical use case which doesn't re-query or more typically caches 
>> the original display mode in order to restore it, it presents a bigger 
>> problem that trying to restore will always fail. 
>> 
>> So the other thing this fix does is detect when we fail to restore the 
>> initial mode and try again in a different way.
>> Only if that fails do we then throw an exception.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
> Unable to set display mode

test/jdk/java/awt/GraphicsDevice/DisplayModes/UnknownRefrshRateTest.java line 
44:

> 42: for (GraphicsDevice d : devices) {
> 43: 
> 44: if (!d.isDisplayChangeSupported()) {

BTW, do we want to run this test on other platforms?

If so, we need to modify test. Because `isDisplayChangeSupported()` may return 
true on Windows and Linux only if we have a fullscreen window showing.

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode!

2021-06-07 Thread Phil Race
On Sat, 5 Jun 2021 15:07:27 GMT, Phil Race  wrote:

> There are two issues here, both macOS specific.
> First the original reported one that occurs when running on a shared remote 
> VNC type desktop on macOS.
> Only a single supported display mode is returned and it is also the current 
> mode.
> A program that simply enumerates the reported modes and tries to set them, if 
> it reaches the native layer,
> will fail because macOS reports the same display mode it returned as valid as 
> now being illegal.
> It does not appear to be as simple as a user's permission level since it 
> occurs with an admin user as well
> so it probably is the case that macOS simply does not allow this shared 
> desktop to be changed although
> there's no docs I can find
> Ordinarily we would not have tried this since we test if the requested 
> display mode is the same as the native
> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
> allowed as meaning match
> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
> so the code is enhanced to check for that
> 
> Second, when creating a test it was discovered that *at least* on the 
> built-in retina display of a 16" macbook pro,
> the system default contig is never in the list of available modes and can be 
> discovered only by querying the *current* mode
> before any modes are changed. This beheaviour is 100% consistent no matter if 
> the system was just rebooted, has
> an external monitor attached as well, or not, activates the discrete card, or 
> not.
> 
> Since we've had code since at least 2013 that added the default mode I 
> suspect this has been seen before but nowhere
> can I find an explanation.
> 
> It has odd consequences like after you change the display mode, you will no 
> longer see the default display mode reported,
> so the list of available modes is reduced by one.
> But for a typical use case which doesn't re-query or more typically caches 
> the original display mode in order to restore it, it presents a bigger 
> problem that trying to restore will always fail. 
> 
> So the other thing this fix does is detect when we fail to restore the 
> initial mode and try again in a different way.
> Only if that fails do we then throw an exception.

fixed typo in comment

-

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode! [v2]

2021-06-07 Thread Phil Race
> There are two issues here, both macOS specific.
> First the original reported one that occurs when running on a shared remote 
> VNC type desktop on macOS.
> Only a single supported display mode is returned and it is also the current 
> mode.
> A program that simply enumerates the reported modes and tries to set them, if 
> it reaches the native layer,
> will fail because macOS reports the same display mode it returned as valid as 
> now being illegal.
> It does not appear to be as simple as a user's permission level since it 
> occurs with an admin user as well
> so it probably is the case that macOS simply does not allow this shared 
> desktop to be changed although
> there's no docs I can find
> Ordinarily we would not have tried this since we test if the requested 
> display mode is the same as the native
> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
> allowed as meaning match
> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
> so the code is enhanced to check for that
> 
> Second, when creating a test it was discovered that *at least* on the 
> built-in retina display of a 16" macbook pro,
> the system default contig is never in the list of available modes and can be 
> discovered only by querying the *current* mode
> before any modes are changed. This beheaviour is 100% consistent no matter if 
> the system was just rebooted, has
> an external monitor attached as well, or not, activates the discrete card, or 
> not.
> 
> Since we've had code since at least 2013 that added the default mode I 
> suspect this has been seen before but nowhere
> can I find an explanation.
> 
> It has odd consequences like after you change the display mode, you will no 
> longer see the default display mode reported,
> so the list of available modes is reduced by one.
> But for a typical use case which doesn't re-query or more typically caches 
> the original display mode in order to restore it, it presents a bigger 
> problem that trying to restore will always fail. 
> 
> So the other thing this fix does is detect when we fail to restore the 
> initial mode and try again in a different way.
> Only if that fails do we then throw an exception.

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

  8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
Unable to set display mode

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4373/files
  - new: https://git.openjdk.java.net/jdk/pull/4373/files/d396b068..d93be8fa

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

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

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


Re: RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode!

2021-06-07 Thread Alexander Zvegintsev
On Sat, 5 Jun 2021 15:07:27 GMT, Phil Race  wrote:

> There are two issues here, both macOS specific.
> First the original reported one that occurs when running on a shared remote 
> VNC type desktop on macOS.
> Only a single supported display mode is returned and it is also the current 
> mode.
> A program that simply enumerates the reported modes and tries to set them, if 
> it reaches the native layer,
> will fail because macOS reports the same display mode it returned as valid as 
> now being illegal.
> It does not appear to be as simple as a user's permission level since it 
> occurs with an admin user as well
> so it probably is the case that macOS simply does not allow this shared 
> desktop to be changed although
> there's no docs I can find
> Ordinarily we would not have tried this since we test if the requested 
> display mode is the same as the native
> one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
> allowed as meaning match
> any mode that satisfies W,H,BPP. But this caused it to fail the equals test 
> so the code is enhanced to check for that
> 
> Second, when creating a test it was discovered that *at least* on the 
> built-in retina display of a 16" macbook pro,
> the system default contig is never in the list of available modes and can be 
> discovered only by querying the *current* mode
> before any modes are changed. This beheaviour is 100% consistent no matter if 
> the system was just rebooted, has
> an external monitor attached as well, or not, activates the discrete card, or 
> not.
> 
> Since we've had code since at least 2013 that added the default mode I 
> suspect this has been seen before but nowhere
> can I find an explanation.
> 
> It has odd consequences like after you change the display mode, you will no 
> longer see the default display mode reported,
> so the list of available modes is reduced by one.
> But for a typical use case which doesn't re-query or more typically caches 
> the original display mode in order to restore it, it presents a bigger 
> problem that trying to restore will always fail. 
> 
> So the other thing this fix does is detect when we fail to restore the 
> initial mode and try again in a different way.
> Only if that fails do we then throw an exception.

src/java.desktop/macosx/native/libawt_lwawt/awt/CGraphicsDevice.m line 266:

> 264: /*
> 265:  * Class: sun_awt_CGraphicsDevice
> 266:  * Method:nativeSetDisplayMode

`nativeResetDisplayMode`?

-

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


RFR: 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws AE: Unable to set display mode!

2021-06-05 Thread Phil Race
There are two issues here, both macOS specific.
First the original reported one that occurs when running on a shared remote VNC 
type desktop on macOS.
Only a single supported display mode is returned and it is also the current 
mode.
A program that simply enumerates the reported modes and tries to set them, if 
it reaches the native layer,
will fail because macOS reports the same display mode it returned as valid as 
now being illegal.
It does not appear to be as simple as a user's permission level since it occurs 
with an admin user as well
so it probably is the case that macOS simply does not allow this shared desktop 
to be changed although
there's no docs I can find
Ordinarily we would not have tried this since we test if the requested display 
mode is the same as the native
one and skip it, but the provided test used REFRESH_RATE_UNKNOWN which is 
allowed as meaning match
any mode that satisfies W,H,BPP. But this caused it to fail the equals test so 
the code is enhanced to check for that

Second, when creating a test it was discovered that *at least* on the built-in 
retina display of a 16" macbook pro,
the system default contig is never in the list of available modes and can be 
discovered only by querying the *current* mode
before any modes are changed. This beheaviour is 100% consistent no matter if 
the system was just rebooted, has
an external monitor attached as well, or not, activates the discrete card, or 
not.

Since we've had code since at least 2013 that added the default mode I suspect 
this has been seen before but nowhere
can I find an explanation.

It has odd consequences like after you change the display mode, you will no 
longer see the default display mode reported,
so the list of available modes is reduced by one.
But for a typical use case which doesn't re-query or more typically caches the 
original display mode in order to restore it, it presents a bigger problem that 
trying to restore will always fail. 

So the other thing this fix does is detect when we fail to restore the initial 
mode and try again in a different way.
Only if that fails do we then throw an exception.

-

Commit messages:
 - 8267430: GraphicsDevice.setDisplayMode(REFRESH_RATE_UNKNOWN) throws IAE: 
Unable to set display mode

Changes: https://git.openjdk.java.net/jdk/pull/4373/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4373&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267430
  Stats: 123 lines in 3 files changed: 120 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4373.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4373/head:pull/4373

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