Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window [v3]

2024-04-02 Thread Lukasz Kostyra
On Sat, 9 Mar 2024 18:41:10 GMT, Marius Hanl  wrote:

>> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` 
>> is broken when `sizeToScene()` was called before or after.
>> 
>> The approach here is to ignore the `sizeToScene()` request when the `Stage` 
>> is maximized or set to fullscreen. 
>> Otherwise the Window Manager of the OS will be confused and you will get 
>> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' 
>> button still shows the 'Restore' Icon, while we already resized the `Stage` 
>> to not be maximized).
>
> Marius Hanl has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   improve tests

LGTM, tests also seem to be fine on my end (checked on Windows and Ubuntu 22.04 
LTS)

-

Marked as reviewed by lkostyra (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1382#pullrequestreview-1973986480


Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window [v3]

2024-03-09 Thread Marius Hanl
> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is 
> broken when `sizeToScene()` was called before or after.
> 
> The approach here is to ignore the `sizeToScene()` request when the `Stage` 
> is maximized or set to fullscreen. 
> Otherwise the Window Manager of the OS will be confused and you will get 
> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' 
> button still shows the 'Restore' Icon, while we already resized the `Stage` 
> to not be maximized).

Marius Hanl has updated the pull request incrementally with one additional 
commit since the last revision:

  improve tests

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1382/files
  - new: https://git.openjdk.org/jfx/pull/1382/files/bbdb4db7..e58a2fda

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1382=02
 - incr: https://webrevs.openjdk.org/?repo=jfx=1382=01-02

  Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jfx/pull/1382.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1382/head:pull/1382

PR: https://git.openjdk.org/jfx/pull/1382


Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window

2024-03-08 Thread Marius Hanl
On Wed, 28 Feb 2024 23:43:40 GMT, Kevin Rushforth  wrote:

> This seems like a reasonable fix and spec change. Have you tested the case of 
> calling sizeToScene before setting full-screen or maximzed? Since the pending 
> flag will still be set in that case, I want to make sure that case is tested 
> as well.

I thought about this as well but could not find any problem at least on Windows.
If we want to be perfectly safe, we may still want to set the flag when 
`sizeToScene()` is called. What do you think?

I used the following code to test this, and it didn't matter when 
`sizeToScene()` was called:


   @Override
public void start(Stage primaryStage) throws Exception {
StackPane root = new StackPane();
Button wss = new Button("Wss");
wss.setPrefSize(50, 50);
root.getChildren().add(wss);

Scene scene = new Scene(root);

Stage stage = new Stage();
stage.setWidth(500);
stage.setMaximized(true);
stage.sizeToScene();
stage.setScene(scene);
stage.show();
}

-

PR Comment: https://git.openjdk.org/jfx/pull/1382#issuecomment-1986030015


Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window [v2]

2024-03-08 Thread Marius Hanl
> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is 
> broken when `sizeToScene()` was called before or after.
> 
> The approach here is to ignore the `sizeToScene()` request when the `Stage` 
> is maximized or set to fullscreen. 
> Otherwise the Window Manager of the OS will be confused and you will get 
> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' 
> button still shows the 'Restore' Icon, while we already resized the `Stage` 
> to not be maximized).

Marius Hanl has updated the pull request incrementally with one additional 
commit since the last revision:

  JDK-8326619: Improve tests

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1382/files
  - new: https://git.openjdk.org/jfx/pull/1382/files/bc607409..bbdb4db7

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1382=01
 - incr: https://webrevs.openjdk.org/?repo=jfx=1382=00-01

  Stats: 22 lines in 2 files changed: 5 ins; 2 del; 15 mod
  Patch: https://git.openjdk.org/jfx/pull/1382.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1382/head:pull/1382

PR: https://git.openjdk.org/jfx/pull/1382


Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window

2024-02-28 Thread Kevin Rushforth
On Mon, 26 Feb 2024 20:51:56 GMT, Marius Hanl  wrote:

> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is 
> broken when `sizeToScene()` was called before or after.
> 
> The approach here is to ignore the `sizeToScene()` request when the `Stage` 
> is maximized or set to fullscreen. 
> Otherwise the Window Manager of the OS will be confused and you will get 
> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' 
> button still shows the 'Restore' Icon, while we already resized the `Stage` 
> to not be maximized).

Btw, I get the following test failures on our headful Linux test systems:


SizeToSceneFullscreenTest > testInitialSizeFullscreen FAILED
java.lang.AssertionError: Stage height expected:<1080.0> but was:<1117.0>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotEquals(Assert.java:835)
at org.junit.Assert.assertEquals(Assert.java:555)
at 
test.javafx.stage.SizeToSceneFullscreenTest.testInitialSizeFullscreen(SizeToSceneFullscreenTest.java:78)


This seems unrelated to your fix. I think the problem might be that in 
full-screen mode the stage can be bigger than the visible screen size or maybe 
the decorations are just larger on that system. You might either need to 
increase the tolerance values or instead check for >= visible width / height 
(possibly with some small tolerance).

-

PR Comment: https://git.openjdk.org/jfx/pull/1382#issuecomment-1970158754


Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window

2024-02-28 Thread Kevin Rushforth
On Mon, 26 Feb 2024 20:51:56 GMT, Marius Hanl  wrote:

> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is 
> broken when `sizeToScene()` was called before or after.
> 
> The approach here is to ignore the `sizeToScene()` request when the `Stage` 
> is maximized or set to fullscreen. 
> Otherwise the Window Manager of the OS will be confused and you will get 
> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' 
> button still shows the 'Restore' Icon, while we already resized the `Stage` 
> to not be maximized).

This seems like a reasonable fix and spec change. Have you tested the case of 
calling sizeToScene before setting full-screen or maximzed? Since the pending 
flag will still be set in that case, I want to make sure that case is tested as 
well.

Also, if this fixed [JDK-8316425](https://bugs.openjdk.org/browse/JDK-8316425), 
then that bug should be closed as a duplicate of this one.

@lukostyra @arapte can you also review this?

-

PR Comment: https://git.openjdk.org/jfx/pull/1382#issuecomment-1970103785


Re: RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window

2024-02-26 Thread Kevin Rushforth
On Mon, 26 Feb 2024 20:51:56 GMT, Marius Hanl  wrote:

> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is 
> broken when `sizeToScene()` was called before or after.
> 
> The approach here is to ignore the `sizeToScene()` request when the `Stage` 
> is maximized or set to fullscreen. 
> Otherwise the Window Manager of the OS will be confused and you will get 
> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' 
> button still shows the 'Restore' Icon, while we already resized the `Stage` 
> to not be maximized).

Since this involved changing the specified behavior it will need a CSR. If we 
agree that this is the right behavior, then the CSR will be trivial.

-

PR Comment: https://git.openjdk.org/jfx/pull/1382#issuecomment-1965592210


RFR: JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the Window

2024-02-26 Thread Marius Hanl
This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` is 
broken when `sizeToScene()` was called before or after.

The approach here is to ignore the `sizeToScene()` request when the `Stage` is 
maximized or set to fullscreen. 
Otherwise the Window Manager of the OS will be confused and you will get weird 
flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' button 
still shows the 'Restore' Icon, while we already resized the `Stage` to not be 
maximized).

-

Commit messages:
 - JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the 
Window

Changes: https://git.openjdk.org/jfx/pull/1382/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1382=00
  Issue: https://bugs.openjdk.org/browse/JDK-8326619
  Stats: 179 lines in 3 files changed: 179 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jfx/pull/1382.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1382/head:pull/1382

PR: https://git.openjdk.org/jfx/pull/1382