Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uiWindow unit tests and fixes for unix #261

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

szanni
Copy link
Contributor

@szanni szanni commented Dec 10, 2023

This sadly got a lot bigger than initially anticipated but much of this code is interdependent, hence all of this at once:

  • Add unit tests for uiWindow
  • Document uiWindowMargined default to be FALSE

Fixes for unix:

uiWindowContentSize now uses gtk_window_get_size and gtk_window_resize instead of trying to do some magic calculations on the inner container. According to the docs gtk already does a best effort calculation to report the size sans window decorations. The prior, inner container size code does not work on hidden windows and margined windows anyways and does not add anything on all systems I tested (i3wm and weston) apart from code, confusion, bugs and brittleness, hence the removal.

It is unclear to me if there was ever a good reason for doing these calculations. I could not find any documentation in the code or git log. All code related to window sizing is a best effort on all Unix systems anyways as the gtk3 docs state everywhere. I did not notice any regressions by removing all the window size calculations.

@szanni szanni force-pushed the unit-window-unit-tests-and-fixes branch from 4fc63dc to ff6c9e0 Compare December 19, 2023 17:26
Required for uiWindow tests that may not set a child control.
Fix segfault when calling the following functions before the main
loop is run:
- uiWindowSetContentSize()
- uiWindowSetPosition()

Ensure an iterator() function is always set, by setting one in
uiInit().
Fix the reported uiWindow dimensions for windows that have not
been displayed yet.
Make sure uiWindowSetPosition() does not block even if no
configure event was received. This commonly occurs when setting
the position of a newly created window before being displayed.
Make sure uiWindowSetContentSize() does not block even if no
size-allocate event was received. This commonly occurs when
setting the size of a newly created window before being displayed.
Use gtk_window_get_size() on the GtkWindow instead of querying
the dummy child holder widget for it's allocation size.
This fixes uiWindowContentSize() not reporting the correct
window size after an uiWindowSetContentSize() on a hidden window.
Remove the manual calculations around window sizes and trust that
the gtk calculations for window decorations are sufficient on
most systems.

Fixes wrong size reporting on windows that are margined.
Copy link
Contributor

@cody271 cody271 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been tested on Wayland yet?


// Run the event loop manually by default to ensure we can run uiMainStep()
// internally to make asynchronous GTK calls appear synchronous.
uiMainSteps();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the event loop default to "steps mode" on Darwin now as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the event loop default to "steps mode" on Darwin now as well?

I guess that would make sense for consistencies sake? On the other hand Windows has been doing it's own thing all along, which has been calling uiMainStep() in uiMain().

Happy to change that, just needs to be tested very thoroughly to make sure we don't break anything. There seems to be some interaction around that and uiMenu.

@cody271
Copy link
Contributor

cody271 commented Dec 20, 2023

It is unclear to me if there was ever a good reason for doing these calculations. I could not find any documentation in the code or git log. All code related to window sizing is a best effort on all Unix systems anyways as the gtk3 docs state everywhere. I did not notice any regressions by removing all the window size calculations.

👍

on all systems I tested (i3wm and weston)

Has this been tested on Wayland yet?

I will try some other DEs besides Weston.

@szanni
Copy link
Contributor Author

szanni commented Dec 21, 2023

on all systems I tested (i3wm and weston)

Has this been tested on Wayland yet?

I will try some other DEs besides Weston.

To add to that, this means the unit test work. By no means do Wayland systems honor uiWindowSetPosition in any shape or form as Wayland has no protocol for doing so. And SetContentSize only really works when increasing window size form the original on Wayland systems consistently.
Tested with weston and sway+floating windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants