On Thu, 8 Dec 2022 19:20:30 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org> 
wrote:

>> This cleans size and positioning code, reducing special cases, code 
>> complexity and size.
>> 
>> Changes:
>> 
>> - cached extents: 28, 1, 1, 1 are old defaults - modern gnome uses different 
>> sizes. It does not assume any size because it varies - it does cache because 
>> it's unlikely to vary on the same system - but if it does occur, it will 
>> only waste a resize event.
>> - window geometry, min/max size are centralized in 
>> `update_window_constraints`;
>> - Frame extents (the window decoration size used for "total window size"):
>>     - frame extents are received in `process_property_notify`;
>>     - removed quirks in java code;
>>     - When received, call `set_bounds` again to adjust the size (to account 
>> decorations later received);
>> - Removed `activate_window` because it's the same as focusing the window. 
>> `gtk_window_present` will deiconify and focus it.
>> - `ensure_window_size` was a quirk - removed;
>> - `requested_bounds` removed - not used anymore;
>> - `window_configure` incorporated in `set_bounds` with `gtk_window_move` and 
>> `gtk_window_resize`;
>> - `process_net_wm_property` is a work-around for Unity only (added a check 
>> if Unity - but it can probably be removed at some point)
>> - `restack` split in `to_front()` and `to_back()` to conform to managed code;
>> - Set `gtk_window_set_focus_on_map` to FALSE because if TRUE the Window 
>> Manager changes the window ordering in the "focus stealing" mechanism - this 
>> makes possible to remove the quirk on `request_focus()`;
>> - Note:  `geometry_get_*` and `geometry_set_*` moved location but unchanged.
>
> Thiago Milczarek Sayao has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Unity uses Compiz

I don't see any new problems on Oracle Linux 7.7. Two additional observations:

1. While doing some additional testing, I did discover a regression introduced 
by this patch (unrelated to your most recent commit). A stage that set to 
iconified before it is shown, is not iconified with your patch. This behavior 
happens on all distros I've tested it on. It turns out that we don't have a 
test that checked this case -- or rather we didn't until 
`SceneChangeShouldNotFocusStageTest` was added by PR #940 as part of the fix 
for JDK-8296621. That test now fails consistently with this patch. A simple 
"HelloWorld" program with `stage.setIconified(true);` called before showing the 
stage reproduces this bug. This needs to be fixed before this PR can be 
integrated. <br><br>
2. I figured out why `IconifiedTest` is so fragile on Linux. It looks like 
there is an intermittent (or, on some distros, not-so-intermittent) bug when 
showing two stages that are both always-on-top. The expectation is that if 
there is more than one Stage that is always-on-top, they will be stacked in the 
order they were created. It seems like this isn't always the case. The 
IconifyTest program creates a "bottom" and a "top" stages with both set to 
always-on-top. If I modify IconifyTest to call `getStage(false)` for the bottom 
stage, meaning it won't be always-on-top, the test passes for me consistently 
on Ubuntu 20.04.  This is unrelated to your patch, but fixing the test will 
make it easier to verify that there is no regression introduced by your patch.

Follow-on issues:

* related to point 1 above, I think we need new test cases to check showing 
stages that are initially {iconified,maximized,fullscreen}, so I'll file a 
follow-on test bug.
* related to point 2 above, I'll file a pair of additional follow-on bugs: a 
test bug to fix `IconifyTest` to make it more robust, and a product bug to 
track the problem of stacking order for multiple always-on-top windows.

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

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

Reply via email to