Fix cursor events on screen lock/unlock

[OkamiW]

After the scene graph refactor, node_at_coords no longer return the lock surface after the screen is locked, thereby cursor_rebase_all in handle_surface_map won't work.

This commit make node_at_coords return lock surface when locked.

I also found that the cursor would disappear after screen is unlocked, thereby adding a cursor_rebase_all in handle_unlock.

[emersion]

focused represents the keyboard focus, but this codepath is about the pointer. Moreover, the surface-local coordinates sx/sy are left unset.

The session lock surfaces are added to output->layers.session_lock, so I wonder why this doesn't work?

cc @Nefsen402

[OkamiW]

Last time I checked, when screen is just locked(without any cursor movement), node_at_coords would get a WLR_SCENE_NODE_RECT instead of a WLR_SCENE_NODE_BUFFER, which is probably the red surface created to show when session lock crashed.

[OkamiW]

Related: #8343
Maybe wlr_scene_node_at can only get the lock surface after it's committed(not mapped)?

[OkamiW]

      ┗━tree 0,0 
          ┗━tree 0,0 
              ┗━tree 0,0 
                  ┣━rect 0,0 1280x720 
                  ┗━tree 0,0 
                      ┣━buffer 0,0 0x0 
                      ┗━tree 0,0  disabled
                          ┗━buffer 0,0 0x0 

      ┗━tree 0,0 
          ┗━tree 0,0 
              ┗━tree 0,0 
                  ┣━rect 0,0 1280x720 
                  ┗━tree 0,0 
                      ┣━buffer 0,0 1280x720 
                      ┗━tree 582,300 
                          ┗━buffer 582,300 121x121 

It seems that during handle_surface_map, the lock surface scene buffer size is 0x0. Thereby wlr_scene_node_at won't return it.
After cursor movement, the lock surface scene buffer size is normal.
@emersion

[kennylevinsen]

Hmm, this is a bit of an event ordering issue with wlr_scene.

The order of events here is:

1. Surface is committed
2. Surface role commit implementations are called
3. Surface role commit handlers call wlr_surface_map
4. Surface role map implementations are called
5. Map event fires
6. Commit event fires
7. Scene surface helper attaches new buffer to the surface

Dealing with this would require an "initial commit" sort of commit handler... :/

[OkamiW]

@kennylevinsen
I added commit event emitting in lock_surface_role_commit, that seems to fix the issue.

--- a/types/wlr_session_lock_v1.c
+++ b/types/wlr_session_lock_v1.c
@@ -185,6 +185,8 @@ static void lock_surface_role_commit(struct wlr_surface *surface) {
 		return;
 	}
 
+	wl_signal_emit_mutable(&lock_surface->surface->events.commit, lock_surface);
+
 	wlr_surface_map(surface);
 }

Though we still need cursor_rebase_all in handle_unlock.

[Nefsen402]

I've been pondering for a while if we should get rid of all the cursor rebase stuff and add some sort of callback or just go through with wlr_scene_cursor finally. Scene could implement cursor rebasing on its own and all these problems would go away similar to how we handled xwayland restacking.

[kennylevinsen]

It could be useful, but there are some caveats.

One is that the effect of cursor_rebase is currently conditional on the ongoing seatop, with only the default seatop performing focus updates. It could also be argued that it's a bug that cursor_rebase doesn't also update keyboard focus when focus_follows_mouse always is set.

Another is that, even if scene have a helper, there should preferably still be a way for compositors to do things on their own.

[OkamiW]

It seems that Labwc has a fix for the same problem: labwc/labwc#1858
wl_event_loop_add_idle can be used here to delay the actual operations after the buffer has been attached.
This should be a proper fix for the issue.

[OkamiW]

Just fixed another edge case. I think this PR is ready.
Is it OK to merge this? @emersion

[OkamiW]

Gentle ping.

[OkamiW]

Any update on this? @emersion @kennylevinsen @Nefsen402

[emersion]

In general the approach of delaying the focus updates are more of a workaround than a proper fix. However it's not clear what the proper fix could look like.

[OkamiW]

In general the approach of delaying the focus updates are more of a workaround than a proper fix. However it's not clear what the proper fix could look like.

How about: #8374 (comment)
That requires changing wlroots though.