-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
WindowClone: Layout in allocate virtual #2082
Conversation
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.
I can confirm that this fixes the linked issues, but it looks like the textures (window clones, close buttons, app icons) are all rendered half a pixel offset so they're blurry (this is on LoDPI and Wayland)
@danirabbit the allocations are clamped to pixels now could you try again with that? I'm not so good at noticing those differences 😅 |
5798c65
to
0ae64b1
Compare
0ae64b1
to
32ea331
Compare
|
Nvm I found a utility method that's much better to use in this case and properly exported :) |
@leolost2605 Is this ready for review? I really want to include this in 8.0.4 release. One thing I noticed is that clone no longer has magnifying filters? |
@lenemter yes it's ready. I've added the filters although TBH i don't really know what they do 😅 |
Opened https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4156 for your missing includes… |
Thank you! |
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.
Looks great, one small issue in the code, after that should be ready to go
@danirabbit Can you review this branch please? |
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.
Everything looks sharp but it looks like this re-introduces the issue where windows that spawn while multitasking view is open aren't added. To reproduce:
- Open screenshot and set a timed screenshot
- Open multitasking view
- when the timer runs out the new window will be transparent with no icon tooltip etc
From the description (the breaking refers to the version before the workaround was merged) If it's a must I can do it here though 🤷 |
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.
Ah sorry I missed that. In that case 👍 😀
Currently we are doing most of the layout management by hardcoding x, y, width and height of our child actors. However we can make the whole thing simpler and more robust by doing the layout management in the allocate virtual method as recommended by clutter docs.
Since the root cause for the following issues was that we didn't allocate some children on rare occasions which was fixed in this PR this
fixes #2053 and
fixes #1898 and therefore
closes #2072
There are still some issues when windows spawn while multitasking view is open (like for example they reserve space but don't show) that I would like to tackle in follow up PRs but at least it doesn't completely break anymore as on current main :)
fixes #2088 by reverting #2072
Fixes #2128