Sway: zwp_pointer_constraints_v1@12: error 1:

[shoober420]

Please fill out the following:

• Sway Version:

  • sway version 1.5-ab8ded62 (Jul 19 2020, branch 'master')

• Debug Log:

  • sway.log

• Configuration File:

  • https://gist.github.com/shoober420/59a2713bb09487a935a45ef4341a9d46

• Description:

  • When launching Chocolate Doom, the game attempt to render the video, but it reverts back to desktop with the game running in the background. An error appears in the terminal.

zwp_pointer_constraints_v1@12: error 1: a pointer constraint with a wl_pointer of the same wl_seat is already on this surface

[shoober420]

Attempting some debugging, I added the following setting to my sway config to ensure no other program was stealing pointer focus.

exec swaymsg “workspace 7; exec chocolate-doom;”

Just as before, the game started, but no video was rendered. The desktop was only displayed with Doom sounds playing. Doom continued running even after closing Sway. This was confirmed in the TTY with htop.

I would like to add that quakespasm is also not working. I’ve had quakespasm running with sway before, so there is a regression somewhere.

[shoober420]

Upon further debugging, this error is triggered when using full screen mode with games, such as tyrquake and Chocolate Doom. They launch when in windowed mode.

When starting a new game in Chocolate Doom in windowed mode, the game will revert back to desktop and show the same error message again.

When playing tyrquake in windowed mode, the game will play until trying to enable the mouse. Then the game reverts back to desktop with the same error message.

Devilutionx still works though.

[Xyene]

Seems like the game is running into this part of the pointer constraints spec. From the log:

00:00:12.414 [DEBUG] [types/wlr_relative_pointer_v1.c:145] relative_pointer_v1 0x55d76625a590 created for client 0x55d766281160
00:00:12.414 [DEBUG] [types/wlr_pointer_constraints_v1.c:242] new locked_pointer 0x55d76629a200 (res 0x55d766299cb0)
00:00:12.414 [DEBUG] [types/wlr_pointer_constraints_v1.c:343] constrained 0x55d76629a200
00:00:12.415 [sway/input/cursor.c:865] denying request to set cursor from unfocused client
00:00:12.415 [sway/input/cursor.c:865] denying request to set cursor from unfocused client
00:00:12.415 [INFO] [wayland] error in client communication (pid 2270)

seems suspicious if pid 2270 was the game. It already has a locked pointer active; creating another one would be an error if it didn't destroy it beforehand.

Could you run the game with WAYLAND_DEBUG=1, reproduce the issue in as short a time as possible, and upload the resulting log? That'd confirm whether it's an application bug or a sway/wlroots bug.

[shoober420]

Here is the WAYLAND_DEBUG log. I tested with Chocolate Doom. Same behavior with tyrquake. These games were working very recently, within the last release.

chocodoomlog.txt

[Xyene]

Thanks for providing the trace. This does look to me like an application bug:

[1202324.914] [email protected](537, wl_surface@20, array)
[1202325.010]  -> [email protected]_cursor(0, nil, 0, 0)
[1202325.127]  -> [email protected]_pointer(new id zwp_confined_pointer_v1@28, wl_surface@20, wl_pointer@16, nil, 2)
[1202325.256] [email protected](538, 0, 0, 16, 0)
[1202325.376] [email protected](nil)
[1202325.419] [email protected](1920, 1080, array)
[1202325.496] [email protected](534)
[1202325.536] [email protected](wl_output@15)
[1202325.649] [email protected]()
[1202354.048] [email protected](zwp_pointer_constraints_v1@12, 1, "a pointer constraint with a wl_pointer of the same wl_seat is already on this surface")
zwp_pointer_constraints_v1@12: error 1: a pointer constraint with a wl_pointer of the same wl_seat is already on this surface

I'm not sure that this sequence of events would have succeeded even in sway 1.4; the only change to constraints on the wlroots side between 1.4 and 1.5 was https://github.com/swaywm/wlroots/commit/8b18d389b3369a2797021ea6ede3b35e90edfb49, which is unrelated. Perhaps "something else" changed that is now causing the game to perform this seemingly-buggy logic. It might be useful to downgrade to 1.4 and record a trace there as well to compare.

Fundamentally, there's no -> [email protected] after the locked, and it's trying to create a new one. The constraint was created with LIFETIME_PERSISTENT (3rd parameter is 2) rather than LIFETIME_ONESHOT, so it's not the case that the constraint got destroyed through other means.

@emersion, am I missing something here?

[shoober420]

Found it, it’s a SDL2 regression. I’m not entirely sure where though. It’s pretty recent. I installed sdl2 2.0.12 and the games work fine.

[Xyene]

2.0.12 is the latest stable, right? What version were you running before?

~SDL2 stores the locked pointer handle in SDL_WindowData, but can have multiple windows. I think it's possible for this loop to end up trying to create another locked pointer, if there are two windows and one of them already has the pointer locked. The relevant guard is per-window.~ The constraint is allowed to be per-surface, so this isn't the problem.

[shoober420]

@Xyene
Yes, that is stable in the Arch repositories. I was using the development master branch I compiled myself. I’m going to dig further into this tonight and see if it’s an error on my part with how I compile SDL2.

Sorry for the confusion, and thank you very much for the information.

[Xyene]

No problem; if it's indeed the case that it's failing due to multiple windows, then moving the locked pointer handle into SDL_WaylandInput (like it's done for pointer confines, in the same file) might be the way to go.

[shoober420]

I can confirm that this is for sure a regression from a commit in SDL2 master branch. I don’t know which one though. I’ll make a post about it on SDL bugzilla.

[Xyene]

Sounds good, since I'm curious as well please post the link here too after you do :)

I think if you were to git bisect it, it'd be doable in at most 9 steps -- there have only been 350 commits since the 2.0.12 release.

[shoober420]

Here’s a link to the bugzilla thread.

https://bugzilla.libsdl.org/show_bug.cgi?id=5240

I use Arch PKGBUILD’s to compile git packages, I’m sure there’s a way I can bisect using them.

[Xyene]

I have an idea of what might be happening. These are commits that happened between 2.0.12 and master:

$ git log master~350..master --oneline | grep wayland
587f7fcc7 wayland: update pointer position on initial enter event
35374aac7 wayland: Changed output removal in handle_surface_leave()
047425748 wayland: Move buffer copy into mime_data_list_add()
a0030e7bc wayland: assert that mmap() didn't return NULL.
03c1db395 docs: Linux systems don't need to install wayland-protocols anymore.
3d9cfceac wayland: add support for SDL_SetWindowGrab
0dd862c96 wayland: Support wayland compositors with wl_seat version < 5 (thanks, Nia!).
9f8dbdb36 configure: Remove wayland-protocols check from configure and CMake scripts.

Notably, one of them is http://hg.libsdl.org/SDL/rev/4709c1dfeabb, which implements SDL_SetWindowGrab. The issue is that locked pointers and confined pointers are mutually exclusive, but there was no logic included to disable the lock if a confine is created. If the game is calling SDL_SetRelativeMouseMode(SDL_TRUE) or SDL_SetRelativeMouseMode (which creates a locked pointer) followed by SDL_SetWindowGrab(...) (which creates a pointer confine), I think this issue will arise.

If you were to add wlr_log(WLR_INFO, "confine type %d", type); before this line and rebuild wlroots, what does the log say?

This might just be a bug I introduced... just not in sway.

[rcgordon]

If the game is calling SDL_SetRelativeMouseMode(SDL_TRUE) or SDL_SetRelativeMouseMode (which creates a locked pointer) followed by SDL_SetWindowGrab(...)

This is a bug in SDL's wayland code that we don't handle this gracefully, but to be clear: a game should not be calling both of these functions. When you get a moment, can you comment out the SDL_SetWindowGrab call in Chocolate Doom and see if it fixes the problem?

[Xyene]

@rcgordon thanks for chiming in -- a grep through Chocolate Doom didn't bring any hits for SDL_SetWindowGrab (it only calls SDL_SetRelativeMouseMode, as far as I could tell), which is why I suggested printing the confine type in wlroots directly instead (in case it somehow ends up going down that path in a "weird" way).

Since SDL_SetRelativeMouseMode is a more restricted version of SDL_SetWindowGrab in terms of where the cursor can move, I was thinking of sending in a patch that keeps track of whether a grab was requested, and downgrades from relative mouse mode to grabbed mode when necessary rather than crashing. So:

• SDL_SetRelativeMouseMode(SDL_TRUE) + SDL_SetWindowGrab = locked pointer, no confine
• if SDL_SetRelativeMouseMode(SDL_FALSE) is called after, locked pointer destroyed, confine created
• if SDL_SetRelativeMouseMode(SDL_TRUE) is called again, confine destroyed, locked pointer created, etc.

That might not be the bug here, but I guess we'll find out soon.

[shoober420]

I’m at work, I’ll compile wlroots with that code string in about 2 hours.

[Xyene]

No rush :)

Below is a speculative patch for SDL implementing the behavior I described above, if that indeed ends up being the issue. I wrote it headlessly, and am unable to test it apart from the fact it compiles.

SDL patch

# HG changeset patch
# User Tudor Brindus <[email protected]>
# Date 1595351664 14400
#      Tue Jul 21 13:14:24 2020 -0400
# Node ID 0f47353e99b08052895e34f2537321206ea917cd
# Parent  48cd4acb8483803337dc43b8aa876e9844895f7c
wayland: defer pointer confine creation until pointer unlock

It is a protocol error to attempt to create a pointer confine (i.e.
`SDL_SetWindowGrab`) while a locked pointer is active, and vice-versa.
Instead of aborting due to a protocol error, this commit makes SDL
gracefully downgrade locked pointers to confines when appropriate.

diff -r 48cd4acb8483 -r 0f47353e99b0 src/video/wayland/SDL_waylandevents.c
--- a/src/video/wayland/SDL_waylandevents.c Thu May 21 00:06:09 2020 -0400
+++ b/src/video/wayland/SDL_waylandevents.c Tue Jul 21 13:14:24 2020 -0400
@@ -66,6 +66,7 @@
     SDL_WaylandDataDevice *data_device;
     struct zwp_relative_pointer_v1 *relative_pointer;
     struct zwp_confined_pointer_v1 *confined_pointer;
+    SDL_Window *confined_pointer_window;
     SDL_WindowData *pointer_focus;
     SDL_WindowData *keyboard_focus;

@@ -1211,6 +1212,14 @@
     w->locked_pointer = locked_pointer;
 }

+static void pointer_confine_destroy(struct SDL_WaylandInput *input)
+{
+    if (input->confined_pointer) {
+        zwp_confined_pointer_v1_destroy(input->confined_pointer);
+        input->confined_pointer = NULL;
+    }
+}
+
 int Wayland_input_lock_pointer(struct SDL_WaylandInput *input)
 {
     SDL_VideoDevice *vd = SDL_GetVideoDevice();
@@ -1227,6 +1236,10 @@
     if (!input->pointer)
         return -1;

+    /* If we have a pointer confine active, we must destroy it here because
+     * creating a locked pointer otherwise would be a protocol error. */
+    pointer_confine_destroy(input);
+
     if (!input->relative_pointer) {
         relative_pointer =
             zwp_relative_pointer_manager_v1_get_relative_pointer(
@@ -1265,6 +1278,9 @@

     d->relative_mouse_mode = 0;

+    if (input->confined_pointer_window)
+        Wayland_input_confine_pointer(input->confined_pointer_window, input);
+
     return 0;
 }

@@ -1302,6 +1318,13 @@
     if (input->confined_pointer)
         Wayland_input_unconfine_pointer(input);

+    input->confined_pointer_window = window;
+
+    /* We cannot create a confine if the pointer is already locked. Defer until
+     * the pointer is unlocked. */
+    if (d->relative_mouse_mode)
+        return 0;
+
     confined_pointer =
         zwp_pointer_constraints_v1_confine_pointer(d->pointer_constraints,
                                                    w->surface,
@@ -1318,11 +1341,8 @@

 int Wayland_input_unconfine_pointer(struct SDL_WaylandInput *input)
 {
-    if (input->confined_pointer) {
-        zwp_confined_pointer_v1_destroy(input->confined_pointer);
-        input->confined_pointer = NULL;
-    }
-
+    pointer_confine_destroy(input);
+    input->confined_pointer_window = NULL;
     return 0;
 }

[shoober420]

Here is the custom wlroots build log:

sway.log

[Xyene]

Great, that confirms the Chocolate Doom/SDL is creating a confine while the pointer is already locked. I'm not entirely sure how it gets there since I didn't see any direct calls to it in the game source.

00:00:09.734 [DEBUG] [types/wlr_relative_pointer_v1.c:145] relative_pointer_v1 0x564bf1080570 created for client 0x564bf10555d0
00:00:09.734 [DEBUG] [types/wlr_pointer_constraints_v1.c:243] new locked_pointer 0x564bf107bb40 (res 0x564bf1093660)
00:00:09.735 [DEBUG] [types/wlr_pointer_constraints_v1.c:344] constrained 0x564bf107bb40
00:00:09.735 [sway/input/cursor.c:865] denying request to set cursor from unfocused client
00:00:09.735 [sway/input/cursor.c:865] denying request to set cursor from unfocused client
00:00:09.735 [INFO] [types/wlr_pointer_constraints_v1.c:184] confine type 1
00:00:09.735 [INFO] [wayland] error in client communication (pid 1888)

https://github.com/swaywm/wlroots/blob/bd387da62d78f25027f1f80d25d85970fd09c235/include/wlr/types/wlr_pointer_constraints_v1.h#L21-L24

Does the SDL patch, applied to master, fix the issue for you?

If you're curious you can also add an abort() in Wayland_input_confine_pointer and bt full the resulting core dump to see how the game got there.

[shoober420]

Your patch does indeed fix the bug. Awesome! Thank you so much.

[Xyene]

Happy to hear it 👍

I've attached the patch over on the SDL side; I'll go ahead and close this ticket since there's nothing to do here.

[shoober420]

Hey @Xyene ,

I was wondering the status of this patch. As of this SDL2 master commit (https://hg.libsdl.org/SDL/rev/c77235726409), the bug still remains.

[Xyene]

I sent it in in the bug report you opened: https://bugzilla.libsdl.org/show_bug.cgi?id=5240#c1

It doesn't look like it's been reviewed yet, but the bug itself is marked as P2 so it seems likely to happen sometime.

[rcgordon]

I just reviewed it, looks good. Fixed in upstream in https://hg.libsdl.org/SDL/rev/50d34ec3ac71 ...thanks for the work on this @Xyene, and everyone else for the research, too! We greatly appreciate it. :)

[rcgordon]

(Also we're locking down the 2.0.14 release, so this fix will be in a formal release very soon.)