Make XWaylandWMSurface thread safe #1186
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
…inter that deletes itself
AlanGriffiths
approved these changes
Jan 10, 2020
Comment on lines
285
to
293
| aquire_shell_surface([detail](auto shell_surface) | ||
| { | ||
| if (detail == _NET_WM_MOVERESIZE_MOVE) | ||
| shell_surface->initiate_interactive_move(); | ||
| else if (auto const edge = wm_resize_edge_to_mir_resize_edge(detail)) | ||
| shell_surface->initiate_interactive_resize(edge.value()); | ||
| else | ||
| mir::log_warning("XWaylandWMSurface::move_resize() called with unknown detail %d", detail); | ||
| }); |
There was a problem hiding this comment.
Should only the shell_surface->... calls be in the aquire_shell_surface( functor?
|
✌️ wmww can now approve this pull request. To approve and merge a pull request, simply reply with |
|
bors r+ |
bors bot
added a commit
that referenced
this pull request
Jan 10, 2020
1186: Make XWaylandWMSurface thread safe r=wmww a=wmww Make Wayland calls on the Wayland thread and lock everything else behind a mutext Co-authored-by: William Wold <[email protected]>
Build succeeded
|
AlanGriffiths
pushed a commit
that referenced
this pull request
Jan 13, 2020
1186: Make XWaylandWMSurface thread safe r=wmww a=wmww Make Wayland calls on the Wayland thread and lock everything else behind a mutext Co-authored-by: William Wold <[email protected]>
AlanGriffiths
added a commit
that referenced
this pull request
Jan 16, 2020
* Merge #1113 #1116 1113: Add Surface::set_window_margins() for SSD r=AlanGriffiths a=wmww This allows setting a surface's "window margins" (the padding around a surface that makes room for decorations). There are alternative ways we could have done this, each with its advantages and disadvantages. We settled on this approach several weeks ago because it's easy to implement with minimum code churn, and appears to be similar to what whoever was here last intended to do. In the future (especially if we split up the `scene::Surface` interface) we may want to refactor this, but it has proven to be sufficient for usable SSDs. 1116: Add and remove test files r=AlanGriffiths a=wmww Replace and unused frontend shell stub with a scene shell stub that I will actually need for SSD tests. Also drop an unused mock shell and move `ExplicitExecutor` to its own file as it will be needed in multiple test files. Co-authored-by: William Wold <[email protected]> * Merge #1117 1117: Add decoration::Manager r=AlanGriffiths a=wmww Adds a new `mir::shell::decoration` namespace, and a `decoration::Manager` configuration global. Stacked on #1116. Co-authored-by: William Wold <[email protected]> * Merge #1119 1119: SSD surface creation param r=AlanGriffiths a=wmww This lets the XWayland frontend mark its windows as server-side-decorated and causes the shell to decorate them. Co-authored-by: William Wold <[email protected]> * Merge #1123 1123: BasicSurface: don't allow zero content size r=AlanGriffiths a=wmww Fixes small mistake in implementation of `BasicSurface::content_size()` that may have caused some SSD crashes. EDIT: doesn't look like the old behavior was causing any problem in practice, but this doesn't hurt. Co-authored-by: William Wold <[email protected]> * Merge #1126 1126: Simplifiy mtd::StubSurface r=RAOF a=wmww Remove a redundant surface stub, and move the method implementations of the remaining one into the header. This reduces the number of files that need to be edited for `scene::Surface` changes by two, which speeds up adding surface properties and future refactoring of the class. Co-authored-by: William Wold <[email protected]> * Merge #1121 1121: SSD MVP r=AlanGriffiths a=wmww Not based on other PRs, but won't take effect until combined with #1119. After that decorations should appear on all X11 windows. Adds a working title bar with resize edges and minimize/maximize/close buttons. Co-authored-by: William Wold <[email protected]> * Merge #1129 1129: Correctly constrain window size r=wmww a=wmww Max and min window size were only being respected when first set. This PR constrains the window size whenever the window is resized. Tests included. EDIT: note that this PR alone will not fix SSD size constraints, but it will fix the visible problem of windows moving weirdly when you resize them too small. Co-authored-by: William Wold <[email protected]> * Merge #1134 1134: [wayland-platform] Apply output scale r=wmww a=AlanGriffiths Apply output scale. (Fixes: #1133) Mir uses unscaled coordinates, so we need to mark buffers as pre-scaled and scale some input values. Co-authored-by: Alan Griffiths <[email protected]> * Merge #1132 1132: Revert "Merge #1089" r=RAOF a=AlanGriffiths We're no longer seeing intermittent failures to set up ubuntu-devel. Reinstate in CI. This reverts commit a9a7bd5, reversing changes made to 5fdc447. Co-authored-by: Alan Griffiths <[email protected]> * Merge #1137 1137: x11 tweaks r=wmww a=AlanGriffiths Drop some code used in early testing. Add a configuration option to specify the Xwayland executable. Co-authored-by: Alan Griffiths <[email protected]> * Merge #1143 1143: MinimalWindowManager: drag the requested window r=AlanGriffiths a=wmww ...instead of whatever is under the cursor (fixes #1135) I believe this is correct implementation because it mirrors what's already in `apply_resize_by()` and `handle_touch_event()`. This should fix a variety of edge cases I've been stumbling on for a while (such as dragging a window under a panel). Co-authored-by: William Wold <[email protected]> * Merge #1138 1138: Refactor decoration r=AlanGriffiths a=wmww * Add a few properties to the `StaticGeometry` struct that were previously magic numbers strewn about the code * Make `static_geometry` a shared pointer and give all decoration components that need it a copy (instead of having to request it from a `WindowState` * Give `Renderer` a new `Theme` struct that can be changed when switching between the focused and unfocused look A PR adding title text rendering is to come after this is merged. Co-authored-by: William Wold <[email protected]> * Merge #1145 1145: SSD: Redraw titlebar when buttons move r=AlanGriffiths a=wmww Fixes #1142 Co-authored-by: William Wold <[email protected]> * Merge #1144 1144: Draw window titles on SSDs r=wmww a=wmww Uses FreeType to render window titles on server side decorations. It appears the current XWayland implementation does not update the window title after it is initially set, but this will handle title updates when it happens. Co-authored-by: William Wold <[email protected]> * Merge #1150 1150: Draw SSD titlebar button icons r=AlanGriffiths a=wmww Programmatically draws close, minimize and maximize icons. They should look like so:  Co-authored-by: William Wold <[email protected]> * Merge #1151 1151: XWayland refactor and fix r=AlanGriffiths a=wmww Previously, `XWaylandServer::spawn()` forked, and handled both forks in a large switch statement. This splits the two branches into `XWaylandServer::execl_xwayland()` and `XWaylandServer::connect_to_xwayland()`. Also runs `wl_client_create()` on the Wayland thread to fix #1147. It may be helpful to look over the commits individually. Co-authored-by: William Wold <[email protected]> * Merge #1146 1146: SSD: improve how BasicDecoration states are updated r=AlanGriffiths a=wmww TLDR this cleans up logic and prevents a flicker. You don't need to fully understand the problem unless you really care. A call to `window_state_updated()` would trigger creating multiple `WindowState`s, which could be different if a change to the window was in-progress. The end result would be correct once all the callbacks had fired, but it would render one frame where the `WindowState` the `InputState` was generated from was different from the one being rendered. This would cause a flicker of button positions. Co-authored-by: William Wold <[email protected]> * Merge #1152 1152: Tidy up x11 frontend r=wmww a=AlanGriffiths Co-authored-by: Alan Griffiths <[email protected]> * Merge #1149 1149: Fix process snap version check r=Saviq a=AlanGriffiths We want to update "edge" channels when master has a different version. E.g. >>> "1.6.0+dev73".startswith("1.6.0") True >>> "1.6.0+dev73" =="1.6.0" False Co-authored-by: Alan Griffiths <[email protected]> Co-authored-by: Michał Sawicz <[email protected]> * Merge #1159 1159: Remove unused function r=wmww a=AlanGriffiths Remove unused function (that cannot be used safely). Co-authored-by: Alan Griffiths <[email protected]> * Merge #1154 1154: [X11 frontend] Make Wayland calls on the correct thread r=wmww a=AlanGriffiths Make Wayland calls on the correct thread. (Fixes: #1153) Co-authored-by: Alan Griffiths <[email protected]> * Merge #1162 1162: [server/input] Use a sorted map for outputs r=wmww a=mariogrip This changes the map of outputs from a unordered map to a sorted map, as mir takes the first output as default for touch screens if not otherwise is provided. it makes no sense for this to give a different result depending on whats first in the unordered_map bucket. Co-authored-by: Marius Gripsgard <[email protected]> * Merge #1161 1161: Add frontend::SurfaceStack r=wmww a=wmww Replaces #1155 Co-authored-by: William Wold <[email protected]> * Merge #1165 1165: [packaging] Add libwayland-dev to Build-Depends r=AlanGriffiths a=RAOF Why did we get away with not having this before? Co-authored-by: Christopher James Halse Rogers <[email protected]> * Merge #1160 1160: [X11 frontend] A pass at tidying XWaylandServer r=wmww a=AlanGriffiths A pass at tidying XWaylandServer: o Reducing scope of stuff o Avoiding indirect recursion o Simplify threading o Initialize on creation & make const Co-authored-by: Alan Griffiths <[email protected]> * Merge #1170 1170: Fix clang found clangers r=AlanGriffiths a=RAOF Clang 10 picks up a couple of extra warnings, which have picked up some incorrect Mir code. Co-authored-by: Christopher James Halse Rogers <[email protected]> * Merge #1168 1168: Maximize/restore windows with SSDs on titlebar double click r=AlanGriffiths a=wmww Fixes #1167 Double click ssd maximize Co-authored-by: William Wold <[email protected]> * Merge #1177 1177: Don't restart Xwayland if we killed it deliberately r=wmww a=AlanGriffiths Don't restart Xwayland if we killed it deliberately. (Fixes: #1169) This whole area is a mess, as the reason we kill Xwayland is that it is waiting for clients to exit before closing. And if we ensured they were closed, we wouldn't be deleting windows that no longer exist and crashing after this. I'll have a look at that (fixed, second commit). We still fail to give clients a chance to close nicely, c.f. #1171. Co-authored-by: Alan Griffiths <[email protected]> * Merge #1176 1176: Some initial XWayland cleanup r=AlanGriffiths a=wmww Removes a lot of dead code that we wont need, moves some stuff around, and patches initial window properties closer to where they will be used. This shouldn't have any noticeable effects on behavior ~except for the first commit, which is the same as #1173 and fixes X #1172~ Co-authored-by: William Wold <[email protected]> * Merge #1179 1179: Threadsafe XwaylandServer r=wmww a=AlanGriffiths Threadsafe XwaylandServer. Protects the mutable state with a mutex and simplifies formerly racy logic. Co-authored-by: Alan Griffiths <[email protected]> * Merge #1181 1181: X11: Tidy XWaylandWM r=wmww a=AlanGriffiths More initialize-on-construction, constification and other tidying Co-authored-by: Alan Griffiths <[email protected]> * Merge #1173 #1182 1173: XWayland: create scene surface only when needed r=AlanGriffiths a=wmww XWayland: create scene surface only when needed. (Fixes: #1172) 1182: Wayland: don't assume PixelSource::read() will execute the do_with_pixels functor r=wmww a=AlanGriffiths Wayland: (cursor code) don't assume PixelSource::read() will execute the do_with_pixels functor. (Fixes: #1180) Co-authored-by: William Wold <[email protected]> Co-authored-by: Alan Griffiths <[email protected]> * Merge #1183 1183: XWayland: make names more verbose r=AlanGriffiths a=wmww Also moves some logic from `XWaylandWMSurface` into `XWaylandWM` that removes the need for a few getters. Co-authored-by: William Wold <[email protected]> * Merge #1184 1184: Drop unused headers r=wmww a=AlanGriffiths Drop unused headers Co-authored-by: Alan Griffiths <[email protected]> * Merge #1186 1186: Make XWaylandWMSurface thread safe r=wmww a=wmww Make Wayland calls on the Wayland thread and lock everything else behind a mutext Co-authored-by: William Wold <[email protected]> * Merge #1156 1156: BasicWindowManager requests on unknown windows should noop r=RAOF a=wmww It is impossible for one subsystem in Mir to know if another subsystem on another thread is in the process of destroying a surface. It is also impossible to externally lock the window manager. Therefore, a surface may be destroyed twice in quick succession, or the window manager may get a `modify_surface` after the surface has been destroyed. The current behavior is to `fatal_error()` if a surface is destroyed twice, and throw `std::out_of_range` or do nothing in other cases. This PR implements, documents and tests: Requests on unknown windows warn and noop. Co-authored-by: William Wold <[email protected]> * Merge #1191 1191: X11: Don't stop Xwayland when pausing the Mir server r=wmww a=AlanGriffiths We don't need to stop Xwayland when pausing the Mir server. (Fixes: #1188) Co-authored-by: Alan Griffiths <[email protected]> * Bump mirserver ABI * Changelog * Merge #1194 1194: SurfaceCreationParameters::update_from(): correctly access name r=RAOF a=wmww Previously, C++ was helpfully casting the optional into a bool, then a char, then "\x1". Co-authored-by: William Wold <[email protected]> * Merge #1192 1192: Add top_left to shell::SurfaceSpecification r=RAOF a=wmww It appears this was never added because no mirclient or Wayland protocols needed it. XWayland will. Co-authored-by: William Wold <[email protected]> * Revert "Merge #1129" (#1202) * Revert "miral: Add window resize tests" This reverts commit abab614 * Revert "BasicWindowManager::modify_window(): respect size constraints" This reverts commit e14dd13 (cherry picked from commit 514352b) * Merge #1198 1198: XWayland: improve verbose logging r=AlanGriffiths a=wmww Improvements to the verbose logs generated by the XWayland frontend. Probably not interesting to anyone but me, but merging will make maintaining my branches simpler. Co-authored-by: William Wold <[email protected]> Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com> Co-authored-by: William Wold <[email protected]> Co-authored-by: Michał Sawicz <[email protected]> Co-authored-by: Marius Gripsgard <[email protected]> Co-authored-by: Christopher James Halse Rogers <[email protected]>
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.
Make Wayland calls on the Wayland thread and lock everything else behind a mutext