Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pointer constraints on layer-shell surfaces broken in recent commit #4465

Closed
feschber opened this issue Jan 17, 2024 · 23 comments
Closed

pointer constraints on layer-shell surfaces broken in recent commit #4465

feschber opened this issue Jan 17, 2024 · 23 comments
Labels
bug Something isn't working

Comments

@feschber
Copy link

feschber commented Jan 17, 2024

Hyprland Version

System/Version info

c4da4b0

Bug or Regression?

Regression

Description

This commit (c4da4b0) breaks pointer constraints. See feschber/lan-mouse#79

How to reproduce

Instantiate a pointer lock instance on a layershell surface -> The pointer does not get locked anymore

Crash reports, logs, images, videos

No response

@feschber feschber added the bug Something isn't working label Jan 17, 2024
vaxerski added a commit that referenced this issue Jan 17, 2024
@vaxerski
Copy link
Member

check with above

@feschber
Copy link
Author

I'm sorry, I have tested wrong and it is precisely not the above ...

Now I double checked and it is because of this:

diff --git a/src/Compositor.cpp b/src/Compositor.cpp
index 2b3fc7d3..fdb1dd93 100644
--- a/src/Compositor.cpp
+++ b/src/Compositor.cpp
@@ -1101,6 +1101,13 @@ wlr_surface* CCompositor::vectorToLayerSurface(const Vector2D& pos, std::vector<
             continue;
 
         auto SURFACEAT = wlr_layer_surface_v1_surface_at(ls->layerSurface, pos.x - ls->geometry.x, pos.y - ls->geometry.y, &sCoords->x, &sCoords->y);
+        if (ls->layerSurface->current.keyboard_interactive && ls->layer >= ZWLR_LAYER_SHELL_V1_LAYER_TOP) {
+            if (!SURFACEAT)
+                SURFACEAT = ls->layerSurface->surface;
+
+            *ppLayerSurfaceFound = ls.get();
+            return SURFACEAT;
+        }
 
         if (SURFACEAT) {
             if (!pixman_region32_not_empty(&SURFACEAT->input_region))

Again, sorry for the confusion!

@feschber
Copy link
Author

I'm unsure what exactly #4401 was trying to achieve here but the above code seemed to be there to implement this:

"For the top and overlay layers, the seat will always give exclusive keyboard focus to the top-most layer which has keyboard interactivity set to exclusive. If this layer contains multiple surfaces with keyboard interactivity set to exclusive, the compositor determines the one receiving keyboard events in an implementation- defined manner. In this case, no guarantee is made when this surface will receive keyboard focus (if ever)."

(from https://wayland.app/protocols/wlr-layer-shell-unstable-v1#zwlr_layer_surface_v1:enum:keyboard_interactivity).

However the check for "exclusive" was not in place. Maybe changing this to check for keyboard_interactive == EXCLUSIVE would be correct.

@vaxerski
Copy link
Member

patch.txt
try

@earboxer
Copy link
Contributor

The proposed solutions are bad because keyboard_interactivity should not affect pointer interactivity.

So I think pointer constraints should be used instead: https://wayland.app/protocols/pointer-constraints-unstable-v1

@feschber
Copy link
Author

feschber commented Jan 17, 2024

To clarify: I use pointer-constraints, shortcut inhibit and keyboard focus exclusive.

I expect

  1. pointer constraints to keep the pointer on the surface
  2. keyboard mode exclusive to get exclusive access to the keyboard
  3. shortcut inhibit to prevent any keyboard shortcut from somehow unfocusing the surface

I'm unsure if (1) ever worked properly (see #4464) but the mentioned commit definitely broke it completely

(2) was working before but now also doesn't work anymore: key codes are now send to other surfaces in addition to the layer surface even with exclusive access!

So this is the position I'm currently in.

@feschber
Copy link
Author

@vaxerski your proposed patch fixes (1) except for the mentioned issue #4464 but (2) remains broken

@vaxerski
Copy link
Member

was 2 a regression from #4401 ?

@feschber
Copy link
Author

was 2 a regression from #4401 ?

Now that you mention it - no that seems to be yet another regression.

I can bisect that tomorrow.

@vaxerski
Copy link
Member

would help, thanks. :) I'll try to fix that tomorrow. On holiday, you know.

@feschber
Copy link
Author

Might also be just modifier keys I'm not sure anymore. In that case it may also be that shortcut inhibit is broken.
I'm starting to mix all these things up in my head, sorry about that. I will report back tomorrow as soon as I can.

@vaxerski
Copy link
Member

shortcut inhibit is not impl'd in hl.

@feschber
Copy link
Author

Are you sure?

wayland-info | grep shortcut
interface: 'zwp_keyboard_shortcuts_inhibit_manager_v1',  version:  1, name: 21
interface: 'hyprland_global_shortcuts_manager_v1',       version:  1, name: 46

@feschber
Copy link
Author

Ah its just a dummy implementation.
But something must have changed regardless because I remember that I could previously not switch workspaces with an exclusively focused layershell window.

@earboxer
Copy link
Contributor

I think (2) was broken by 307dd8f

vaxerski added a commit that referenced this issue Jan 19, 2024
@vaxerski
Copy link
Member

2 should be fixed

@feschber
Copy link
Author

I did some bisecting again and it seems like the keyboard actually always worked.
Its only the shortcuts that do not work (I did not realize this half a year ago, when I tested it and assumed there was a regression because of that).

So yeah, the only problem is the pointer constraints, which were broken in c4da4b0

@q234rty
Copy link
Contributor

q234rty commented Jan 21, 2024

Is there any issue w/ reverting both 307dd8f and 17339e0? See also #4498.

@feschber
Copy link
Author

Is there any issue w/ reverting both 307dd8f and 17339e0? See also #4498.

Not on my end

@earboxer
Copy link
Contributor

Is there any issue w/ reverting both 307dd8f and 17339e0? See also #4498.

I don't have a problem with it. In fact, reverting these two seems to fix an edge-case where I can't type (with the virtual keyboard) on a layer surface when I don't have an open window beneath it.

@vaxerski
Copy link
Member

please check #4889

@feschber
Copy link
Author

feschber commented Mar 1, 2024

Working perfectly! #4464 is also fixed :)

@feschber
Copy link
Author

closed in #4889

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants