Remove uses of GrabServer #1283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yshui
force-pushed
the
no-more-server-grabs
branch
3 times, most recently
from
eca64b2
to
e4087f9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #1283 +/- ##
==========================================
- Coverage 52.31% 52.21% -0.11%
==========================================
Files 64 64
Lines 13775 13798 +23
==========================================
- Hits 7207 7205 -2
- Misses 6568 6593 +25
|
The way we set and use it, it's completely equivalent to checking if w->win_image == NULL. So it's redundant. Signed-off-by: Yuxuan Shui <[email protected]>
… one Might break on NVIDIA, needs testing. Signed-off-by: Yuxuan Shui <[email protected]>
Signed-off-by: Yuxuan Shui <[email protected]>
Signed-off-by: Yuxuan Shui <[email protected]>
Previously `win->tree_ref` could reference a freed tree node up until all tree changes are handled in `handle_new_windows`. The result is that certain operations can be only performed after all tree changes are handled, because they might use `win->tree_ref`. Such operations include updating the focused window, for example. This commit makes sure `win->tree_ref` is valid at all times. Also split up wm_tree_enqueue_change, because enqueueing a TOPLEVEL_KILLED now needs to return whether the event cancels out a previous TOPLEVEL_NEW, in which case the zombie is freed and there's no need for tree_ref update. Signed-off-by: Yuxuan Shui <[email protected]>
This way focus changes are processed in order with all other events, so we are less likely to run into the issue where the focused window isn't yet in our window tree. Known issue: it is still possible for the window to not be in our window tree. And when that happens, we won't update focus again after the focused window is imported. Signed-off-by: Yuxuan Shui <[email protected]>
The old name is inaccurate, this flag is not limited to ewmh. Signed-off-by: Yuxuan Shui <[email protected]>
Moving it out of the X critical section. Signed-off-by: Yuxuan Shui <[email protected]>
This is an alternative fix for #357. Previous fix is moving configure_root into the X critical section, and now we are removing the critical section so we can't do that anymore. Signed-off-by: Yuxuan Shui <[email protected]>
It doesn't really make any X requests besides querying the root window geometry. Signed-off-by: Yuxuan Shui <[email protected]>
This is to solve a race condition that was previously impossible, and only became possible because the X critical section was removed. Currently, in `handle_new_windows`, after we created a managed window object, we check if its map state is already mapped, if so, we call `win_map_start`. But there is a problem if the window is mapped before we reached `handle_new_windows`. There will be an event generated for that map, because managed windows are toplevels, and we have event mask set up for the root window at the very beginning, so we won't miss any map events from toplevels. (Except when picom has just started, and is trying to manage existing windows). There are two cases, based on whether this map event is handled before we reach `handle_new_windows`. 1) If the map event is handled after. It itself will trigger another `win_map_start` call, which is an error, and we have an assertion for that. But on the other hand, if we don't call `win_map_start` in `handle_new_windows`: 2) If the map event is handled before. It will do nothing since we haven't created a managed window object for that tree node yet. And later, since we don't call `win_map_start` in `handle_new_windows`, this window will just never be mapped. (This would be the same scenario for pre-existing windows when picom just started). All these problem came from the map event being handled out-of-order w.r.t. the reply to the GetWindowAttributes request sent by `win_maybe_allocate`. So we use async requests to make sure they are handled in order. Signed-off-by: Yuxuan Shui <[email protected]>
The property being fetched might change between x_get_prop_info and actually fetching the property. So instead we do this all in one request by setting the length to UINT_MAX. Signed-off-by: Yuxuan Shui <[email protected]>
Signed-off-by: Yuxuan Shui <[email protected]>
If PresentNotifyMsc event is stuck in the out buffer, we will stop rendering frames. Signed-off-by: Yuxuan Shui <[email protected]>
I found sometimes, when we receive an invalid PresentCompleteNotify, keep requesting new present notifys using `last_msc + 1` doesn't really help us recover. Instead X just keeps sending us bad PresentCompleteNotifys, and we stuck in a loop. (This is more likely to happen during screen resolution change. Also this did not seem to be possible previously, when we were still handling resolution changes inside the X critical section.) This commit tries to test out a different approach. The hypothesis is, maybe doing something to the screen (e.g. render and present a new frame) helps change the internal state of the X server, thus kicks us out of the loop. So whenever we get an invalid notify, we fake a vblank event so rendering can happen. And we keeps doing this until we are (hopefully) fine. Signed-off-by: Yuxuan Shui <[email protected]>
There are some cases where a window might be missing its pixmap. Mainly when a window is unmapped/destroyed during a backend reset (there are many reasons for backend resets, e.g. resolution change, unredir-if-possible, etc.). We skip windows like this. Signed-off-by: Yuxuan Shui <[email protected]>
I checked, functions called by refresh_windows should mostly be robust against window disappearing, unless I missed something. (We will find out). Signed-off-by: Yuxuan Shui <[email protected]>
Signed-off-by: Yuxuan Shui <[email protected]>
yshui
force-pushed
the
no-more-server-grabs
branch
from
e4087f9
to
ca3ea85
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the changelog for why this is done. The Xorg bug referenced is https://gitlab.freedesktop.org/xorg/xserver/-/issues/1691
... this might break everything.