Use wlroots scene graph api #6844
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
|
Hmm, the xwayland damage tracking issue goes away on DRM. That's very strange. |
Nefsen402
force-pushed
the
scene-graph
branch
2 times, most recently
from
5a4daf3
to
914982a
Compare
|
Xwayland cropped cursor also goes away on DRM. |
Nefsen402
force-pushed
the
scene-graph
branch
5 times, most recently
from
de21489
to
8d7349c
Compare
|
I'm getting some sort of nonsense SIGSEGV crash on DRM. This is what was in the core dump. @emersion can you make anything out in this? |
Nefsen402
force-pushed
the
scene-graph
branch
2 times, most recently
from
ba6f116
to
b4456b0
Compare
|
I have another code dump here that makes more sense. 0x1900000000 is a fishy address for |
Nefsen402
force-pushed
the
scene-graph
branch
from
b4456b0
to
eecebd3
Compare
Consolatis
reviewed
Feb 22, 2022
Nefsen402
force-pushed
the
scene-graph
branch
3 times, most recently
from
875e534
to
a4bc9f8
Compare
panaman67
reviewed
Feb 24, 2022
Nefsen402
force-pushed
the
scene-graph
branch
5 times, most recently
from
6a92bba
to
1efda98
Compare
|
Thanks for working on this! I've patched my local install with this PR, and will dogfood it for a while. Some early notes (I'll update this list as I come across things):
Left kitty panel has borders drawn in the wrong place
|
Seems to be a regression of a recent memory leak fix, I changed out the layout of the tree structure and I forgot to disable non-visible borders.
Thanks for letting me know about this oddball application. I'm able to reproduce. None of my applications have transaction issues so I haven't noticed there's a problem. |
Screenshot
Screenshot
Screenshot
|
Nefsen402
force-pushed
the
scene-graph
branch
2 times, most recently
from
ade88c1
to
7afd6b8
Compare
Instead of doing this roundabout thing where we get the surface from the view, let's instead get it from the `wlr_surface_state` that we already track in `handle_commit`. This makes the NULL state impossible which is what the old `get_geometry` is checking for and generally cleans things up a little bit. Also don't check if the geometry x/y changed, those will always be 0 for xwayland.
The new scene graph abstraction handles this for us.
The scene graph abstraction does this for us
```
Program terminated with signal SIGSEGV, Segmentation fault.
warning: Section `.reg-xstate/3960717' in core file too small.
0 container_get_siblings (container=0x55bcde4797f0) at ../sway/tree/container.c:1228
1228 if (list_find(container->pending.workspace->tiling, container) != -1) {
[Current thread is 1 (Thread 0x7fa23b4a2940 (LWP 3960717))]
(gdb) bt full=
No symbol "full" in current context.
(gdb) bt full
0 container_get_siblings (container=0x55bcde4797f0) at ../sway/tree/container.c:1228
1 0x000055bcdb62c704 in edge_is_external (cont=0x55bcde4797f0, edge=(WLR_EDGE_TOP | WLR_EDGE_LEFT))
at ../sway/input/seatop_default.c:54
siblings = 0x55bcde4797f0
index = 32766
layout = L_NONE
__PRETTY_FUNCTION__ = "edge_is_external"
2 0x000055bcdb62c96f in find_resize_edge (cont=0x55bcde4797f0, surface=0x0, cursor=0x55bcddd5c2e0)
at ../sway/input/seatop_default.c:106
edge = (WLR_EDGE_TOP | WLR_EDGE_LEFT)
3 0x000055bcdb620b3c in cursor_update_image (cursor=0x55bcddd5c2e0, node=0x55bcde4797f0) at ../sway/input/cursor.c:144
edge = WLR_EDGE_NONE
4 0x000055bcdb62eb8f in handle_rebase (seat=0x55bcddd5a740, time_msec=488992944) at ../sway/input/seatop_default.c:773
e = 0x55bcddd5c8e0
cursor = 0x55bcddd5c2e0
surface = 0x0
sx = 0
sy = 0
5 0x000055bcdb62c531 in seatop_rebase (seat=0x55bcddd5a740, time_msec=488992944) at ../sway/input/seat.c:1585
6 0x000055bcdb620a7d in cursor_rebase (cursor=0x55bcddd5c2e0) at ../sway/input/cursor.c:126
time_msec = 488992944
7 0x000055bcdb620ac4 in cursor_rebase_all () at ../sway/input/cursor.c:136
seat = 0x55bcddd5a740
8 0x000055bcdb61cc95 in transaction_apply (transaction=0x55bcde5b28c0) at ../sway/desktop/transaction.c:704
9 0x000055bcdb61ccdb in transaction_progress () at ../sway/desktop/transaction.c:716
10 0x000055bcdb61d1f9 in transaction_commit_pending () at ../sway/desktop/transaction.c:836
transaction = 0x55bcde5b28c0
11 0x000055bcdb61d596 in _transaction_commit_dirty (server_request=true) at ../sway/desktop/transaction.c:912
12 0x000055bcdb61d5ac in transaction_commit_dirty () at ../sway/desktop/transaction.c:916
13 0x000055bcdb65f579 in view_unmap (view=0x55bcde2ff180) at ../sway/tree/view.c:847
parent = 0x55bcde489010
ws = 0x55bcdde19080
seat = 0x55bcddd5a198
14 0x000055bcdb61e461 in handle_unmap (listener=0x55bcde2ff368, data=0x0) at ../sway/desktop/xdg_shell.c:394
xdg_shell_view = 0x55bcde2ff180
view = 0x55bcde2ff180
__PRETTY_FUNCTION__ = "handle_unmap"
15 0x00007fa23c4ae87f in wlr_signal_emit_safe (signal=0x55bcde46cf38, data=0x0) at ../util/signal.c:29
pos = 0x55bcde2ff368
l = 0x55bcde2ff368
cursor = {link = {prev = 0x55bcde2ff368, next = 0x7ffe240702a0}, notify = 0x7fa23c4ae7c9 <handle_noop>}
end = {link = {prev = 0x7ffe24070280, next = 0x55bcde46cf38}, notify = 0x7fa23c4ae7c9 <handle_noop>}
16 0x00007fa23c47c3c7 in unmap_xdg_surface (surface=0x55bcde46ce30) at ../types/xdg_shell/wlr_xdg_surface.c:40
__PRETTY_FUNCTION__ = "unmap_xdg_surface"
popup = 0x55bcde46ce60
popup_tmp = 0x55bcde46ce60
configure = 0x7ffe24070360
tmp = 0x55bcde488020
17 0x00007fa23c47cd47 in xdg_surface_role_precommit (wlr_surface=0x55bcde488020, state=0x55bcde4881a8)
at ../types/xdg_shell/wlr_xdg_surface.c:330
surface = 0x55bcde46ce30
18 0x00007fa23c4813b2 in surface_commit_state (surface=0x55bcde488020, next=0x55bcde4881a8) at ../types/wlr_compositor.c:407
__PRETTY_FUNCTION__ = "surface_commit_state"
invalid_buffer = false
subsurface = 0xbd8e9aecae023300
--Type <RET> for more, q to quit, c to continue without paging--
19 0x00007fa23c48192a in surface_handle_commit (client=0x55bcde488850, resource=0x55bcde2fdb80) at ../types/wlr_compositor.c:523
surface = 0x55bcde488020
20 0x00007fa23bb5ed4a in () at /usr/lib/libffi.so.8
21 0x00007fa23bb5e267 in () at /usr/lib/libffi.so.8
22 0x00007fa23c517323 in () at /usr/lib/libwayland-server.so.0
23 0x00007fa23c5125cc in () at /usr/lib/libwayland-server.so.0
24 0x00007fa23c5151ca in wl_event_loop_dispatch () at /usr/lib/libwayland-server.so.0
25 0x00007fa23c512d37 in wl_display_run () at /usr/lib/libwayland-server.so.0
26 0x000055bcdb616885 in server_run (server=0x55bcdb68c5c0 <server>) at ../sway/server.c:307
27 0x000055bcdb61594e in main (argc=3, argv=0x7ffe24070af8) at ../sway/main.c:433
```
It seems to be happening because of this set of events all happening
in the span of a single transaction:
1. You kill a tiled window that is the only window in a workplace.
2. Sway will destroy the workspace but not yet the container - this
makes `con->pending.workspace` NULL.
3. Cursor glyphs get recomputed causing sway to recompute if the cursor
is on a container edge.
4. That computation causes an access to the NULL workspace. Crash.
Now that we use wlr_scene, wlroots handles these. If available use the wlroots debug options instead.
Nefsen402
force-pushed
the
scene-graph
branch
from
2f82cda
to
079ebb6
Compare
This was referenced Jan 18, 2024
Closed
|
@Nefsen402 you are appreciated :) thank you! can't wait to use this! |
emersion
pushed a commit
that referenced
this pull request
Jan 26, 2024
Since output layout is destroyed when the wayland display is destroyed we run into a destroy listener order problem: Either the display starts destroying the outputs first, in which case we're good: The existing handling will clean up. However, things go wrong if the display decides to destroy the output layout first. In this case, sway will hold invalid references to the output layout as part of each output so that when it finally goes to destroy them, sway will dereference destroyed output layout bits. Ref: #6844 (comment) (cherry picked from commit bbabb9a)
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.
Closes: #6562 #7120 #6914 #6821 #6960 #6148 #5867 #5497 #5460 #6469 #7458 #7463
Supersedes: #6820 #6747
This should be enough where if most people were on this patch, they wouldn't know it. However, there are some features still missing from the port:
Scene graph doesn't have support for specifying the opacity of a
wlr_scene_surface.However, implementing this would not be enough because we send in xdg surfaces
into relevant helper functions that hide the surfaces / subsurfaces away from us. The easiest
fix from the perspective of the user is the ability to specify the opacity of an entire subtree.
We're also missing support for specifying a scissor region for a surface node. This is useful
because it will prevent applications that don't respect a configure from possibly rendering
on top of other tiled applications. Again, the actual surface nodes are hidden from us, so it
would be easiest to specify a scissor region for an entire subtree.
Scenes already handle client output awareness for us (
wlr_surface_send_enter,wlr_surface_send_leave)However, we don't have this information for buffer nodes so we can do more optimized
text rending. For now text rendering is hard coded to a 2x screen scale and grey scale sub
pixel handling. An appropriate solution to this problem is to support
wl_signals foroutput enter/exit events and the ability to overwrite the buffer in a
wlr_scene_buffer.Scenes do linear filtering only. We need some way to specify this.
Wayland backend specific bugs (not regressions of this branch):
This seems like a wlroots bug.
This is probably a bug I introduced but I'm not sure. This also happens sometimes in
sway-git, maybe I messed the code up enough to make it 100% reproducible.
Although there are xwayland damage tracking issues, this does not mean that current git is unaffected
by damage tracking issues. In fact, this branch fixes damage tracking issues with subsurfaces on a
xdg_surface popup, so as far as I know, for wayland clients damage tracking is perfect.