<AWT Dev> RFR: 8258788: incorrect response to change in window insets [lanai]

[Alexey Ushakov-3]

Perform replaceSurfaceData on insets change

-------------

Commit messages:
 - 8258788: incorrect response to change in window insets [lanai]

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

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

[Sergey Bylokhov-2]

On Wed, 7 Apr 2021 23:04:04 GMT, Alexey Ushakov <[hidden email]> wrote:

> Perform replaceSurfaceData on insets change

src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java line 741:

> 739:             if (CGraphicsDevice.usingMetalPipeline() && invalid) {
> 740:                 replaceSurfaceData();
> 741:             }

I think fix can be moved to the 729 line "if (pResized || isNewDevice || invalid)". Looks like it is a bug even in the OGL case, if "pResized == false" and window is not resized but in the insets were changed we should update the surface.

BTW I think the updateMinimumSize() should be called as well since the minimum/maximum size depends on the insets and NSWindow frame/contentRect.

-------------

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

[Alexey Ushakov-3]

> Perform replaceSurfaceData on insets change

Alexey Ushakov has updated the pull request incrementally with one additional commit since the last revision:

  8258788: incorrect response to change in window insets [lanai]
 
  Moved replaceSurfaceData to more appropriate place

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3390/files
  - new: https://git.openjdk.java.net/jdk/pull/3390/files/ed5e6814..e63c3eb9

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

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

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

[Alexey Ushakov-3]

On Thu, 8 Apr 2021 21:31:29 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Alexey Ushakov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8258788: incorrect response to change in window insets [lanai]
>>  
>>   Moved replaceSurfaceData to more appropriate place
>
> src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java line 741:
>
>> 739:             if (CGraphicsDevice.usingMetalPipeline() && invalid) {
>> 740:                 replaceSurfaceData();
>> 741:             }
>
> I think fix can be moved to the 729 line "if (pResized || isNewDevice || invalid)". Looks like it is a bug even in the OGL case, if "pResized == false" and window is not resized but in the insets were changed we should update the surface.
>
> BTW I think the updateMinimumSize() should be called as well since the minimum/maximum size depends on the insets and NSWindow frame/contentRect.

Yes, good idea. Corrected

-------------

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

[Alexey Ushakov-3]

> Perform replaceSurfaceData on insets change

Alexey Ushakov has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:

  8258788: incorrect response to change in window insets [lanai]
 
  Perform replaceSurfaceData on insets change

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3390/files
  - new: https://git.openjdk.java.net/jdk/pull/3390/files/e63c3eb9..0aa7ca97

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

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

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

[Sergey Bylokhov-2]

On Fri, 9 Apr 2021 11:55:46 GMT, Alexey Ushakov <[hidden email]> wrote:

>> Perform replaceSurfaceData on insets change
>
> Alexey Ushakov has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>
>   8258788: incorrect response to change in window insets [lanai]
>  
>   Perform replaceSurfaceData on insets change

test/jdk/java/awt/Window/FullWindowContentTest/FullWindowContentRenderTest.java line 29:

> 27:  * @key headful
> 28:  * @bug 8258788
> 29:  * @summary [macosx] full window content option rendering test

The test passed even before the fix.

-------------

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

[Alexey Ushakov-3]

On Fri, 9 Apr 2021 17:33:46 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Alexey Ushakov has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>>
>>   8258788: incorrect response to change in window insets [lanai]
>>  
>>   Perform replaceSurfaceData on insets change
>
> test/jdk/java/awt/Window/FullWindowContentTest/FullWindowContentRenderTest.java line 29:
>
>> 27:  * @key headful
>> 28:  * @bug 8258788
>> 29:  * @summary [macosx] full window content option rendering test
>
> The test passed even before the fix.

It's strange. Just rerun it with the following command line:

jtreg -Dsun.java2d.metal=True -testjdk:/Users/user/ws/jbr-dev/build/macosx-x86_64-server-release/images/jdk-bundle/jdk-17.jdk/Contents/Home test/jdk/java/awt/Window/FullWindowContentTest/FullWindowContentRenderTest.java

Test results: failed: 1

-------------

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

[Alexey Ushakov-3]

On Fri, 9 Apr 2021 19:38:15 GMT, Alexey Ushakov <[hidden email]> wrote:

>> test/jdk/java/awt/Window/FullWindowContentTest/FullWindowContentRenderTest.java line 29:
>>
>>> 27:  * @key headful
>>> 28:  * @bug 8258788
>>> 29:  * @summary [macosx] full window content option rendering test
>>
>> The test passed even before the fix.
>
> It's strange. Just rerun it with the following command line:
>
> jtreg -Dsun.java2d.metal=True -testjdk:/Users/user/ws/jbr-dev/build/macosx-x86_64-server-release/images/jdk-bundle/jdk-17.jdk/Contents/Home test/jdk/java/awt/Window/FullWindowContentTest/FullWindowContentRenderTest.java
>
> Test results: failed: 1

What OS version do you have?

-------------

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

[Sergey Bylokhov-2]

On Fri, 9 Apr 2021 19:48:15 GMT, Alexey Ushakov <[hidden email]> wrote:

>> It's strange. Just rerun it with the following command line:
>>
>> jtreg -Dsun.java2d.metal=True -testjdk:/Users/user/ws/jbr-dev/build/macosx-x86_64-server-release/images/jdk-bundle/jdk-17.jdk/Contents/Home test/jdk/java/awt/Window/FullWindowContentTest/FullWindowContentRenderTest.java
>>
>> Test results: failed: 1
>
> What OS version do you have?

macOS 11.2(20D64), did you set the sRGB color profile in the settings?

-------------

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

[Alexey Ushakov-3]

On Fri, 9 Apr 2021 19:59:38 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> What OS version do you have?
>
> macOS 11.2(20D64), did you set the sRGB color profile in the settings?

Yes, it's IEC61966-2.1

-------------

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

[Sergey Bylokhov-2]

On Fri, 9 Apr 2021 11:55:46 GMT, Alexey Ushakov <[hidden email]> wrote:

>> Perform replaceSurfaceData on insets change
>
> Alexey Ushakov has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>
>   8258788: incorrect response to change in window insets [lanai]
>  
>   Perform replaceSurfaceData on insets change

Marked as reviewed by serb (Reviewer).

-------------

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

[Sergey Bylokhov-2]

On Fri, 9 Apr 2021 21:26:18 GMT, Alexey Ushakov <[hidden email]> wrote:

>> macOS 11.2(20D64), did you set the sRGB color profile in the settings?
>
> Yes, it's IEC61966-2.1

Ok, found the issue on my side, by default the test uses the OGL, not a metal.

-------------

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

[Jayathirth D V-2]

On Wed, 7 Apr 2021 23:04:04 GMT, Alexey Ushakov <[hidden email]> wrote:

> Perform replaceSurfaceData on insets change

@avu I am running CI tests, please wait until EOD for integration.

-------------

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

[Jayathirth D V-2]

On Fri, 9 Apr 2021 11:55:46 GMT, Alexey Ushakov <[hidden email]> wrote:

>> Perform replaceSurfaceData on insets change
>
> Alexey Ushakov has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>
>   8258788: incorrect response to change in window insets [lanai]
>  
>   Perform replaceSurfaceData on insets change

Marked as reviewed by jdv (Reviewer).

-------------

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

[Jayathirth D V-2]

On Sat, 10 Apr 2021 05:52:16 GMT, Jayathirth D V <[hidden email]> wrote:

>> Alexey Ushakov has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>>
>>   8258788: incorrect response to change in window insets [lanai]
>>  
>>   Perform replaceSurfaceData on insets change
>
> Marked as reviewed by jdv (Reviewer).

CI tests are fine. LGTM.

-------------

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

[Alexey Ushakov-3]

On Fri, 9 Apr 2021 22:41:37 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Yes, it's IEC61966-2.1
>
> Ok, found the issue on my side, by default the test uses the OGL, not a metal.

Good news! @mrserb Could you sponsor this pull request?

-------------

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

[Alexey Ushakov-3]

On Wed, 7 Apr 2021 23:04:04 GMT, Alexey Ushakov <[hidden email]> wrote:

> Perform replaceSurfaceData on insets change

This pull request has now been integrated.

Changeset: 714ae54f
Author:    Alexey Ushakov <[hidden email]>
Committer: Phil Race <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/714ae54f
Stats:     165 lines in 2 files changed: 164 ins; 0 del; 1 mod

8258788: incorrect response to change in window insets [lanai]

Reviewed-by: serb, jdv

-------------

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