-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[V2] Bugfix/2431 wayland max size #4047
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request enhances Linux desktop window management under Wayland. It introduces a new function to check for Wayland sessions by inspecting the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant SMS as SetMinMaxSize
participant OW as onWayland
participant Env as Environment
App->>SMS: Call SetMinMaxSize
SMS->>OW: Invoke onWayland()
OW->>Env: Read XDG_SESSION_TYPE
Env-->>OW: Return session type (Wayland/X11)
OW-->>SMS: Return true/false based on Wayland
SMS->>SMS: Adjust size constraints if Wayland true
SMS-->>App: Apply adjusted window dimensions
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v2/internal/frontend/desktop/linux/window.c (1)
273-292
: Validate decorator measurements & fix minor typo.
- The approach of computing decorator offsets once may cause issues if decorations change at runtime (e.g., toggling between decorated/undecorated mode). Consider recalculating
decoratorWidth
anddecoratorHeight
if the window’s decoration state changes in the future.- There's a small typo in the comment. Here's a suggested fix:
- // On Wayland window manager get the decoratgors and calculate the differences from the windows' size. + // On Wayland window manager, get the decorators and calculate the differences from the window size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v2/internal/frontend/desktop/linux/window.c
(3 hunks)
🔇 Additional comments (3)
v2/internal/frontend/desktop/linux/window.c (3)
17-19
: Clarify sentinel usage for uninitialized states.
Using-1
as a sentinel is fine, but consider adding a brief comment indicating that-1
denotes "not yet detected" or "unknown." This helps future readers quickly understand the initialization logic ofwmIsWayland
,decoratorWidth
, anddecoratorHeight
.
74-93
: Document or handle fallback logic for unusual sessions.
TheonWayland()
function looks correct for typical Wayland/X11 usage. However, you might want to document any fallbacks ifXDG_SESSION_TYPE
is set to something unexpected.
266-267
: Proper usage of geometry hints.
CombiningGDK_HINT_MAX_SIZE
andGDK_HINT_MIN_SIZE
intoflags
is valid and aligns well with GTK’s geometry hint practices. No concerns here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/src/pages/changelog.mdx (1)
38-38
: Clarify the fix description for Wayland window issues.
The new bullet clearly states that window size issues on Wayland have been fixed (as per PR #4047 by @lyimmi). To further aid users, consider adding a brief note clarifying that the fix addresses discrepancies due to window decoration differences (e.g., header and title offsets) when exiting fullscreen. This additional context would directly link the changelog entry to the PR objectives without requiring users to click through, while maintaining consistency with the rest of the changelog.
Thanks for opening this @lyimmi 🙏 I've made a couple of changes to make the methods thread safe. They shouldn't be called that often so it was probably never going to hit a race condition but just in case... I'm also not sure what would happen if you plugged in a new monitor and called it there... I'm ok with recalculating 👍 LMK if this works for you and I'll merge 👍 |
I'll test it and get back to you. |
Description
This PR Fixes #2431 the fullscreen and maximise issue on wayland.
To fix the issue I compared the window's allocated and reported size from GTK. The allocated size contains the header, title, shadows, etc. while the reported windows size does not.
I stored the decorators width & height, because when the window is unfullscreened GTK reports the window as undecorated and from that point on the calculation is wrong and the wrong size is set for GTK. From that point on all the WM's and wails' functions behave badly. And this way it has check the decorators only once.
It could have been solved another way, but I think this is the most backward compatible. (Without refactoring maximise, minimise, fullscreen, unfullscreen)
My original solution used an IFDEF, but changed it to an
onWayland
function, if we found other wayland specific issues it can be used to decide if the program runs on wayland or not.Please test this fork / PR on existing applications before we merge it!
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor
.Checked on Ubuntu 24.04, wayland, X11, with multiple window configurations.
Test Configuration
Checklist:
website/src/pages/changelog.mdx
with details of this PRSummary by CodeRabbit
New Features
Bug Fixes