Fix wew enter/leave events (ee->detail behavior)

Ferdi265 commented 9 years ago

This fixes the incorrect behavior of wew when dealing with enter and leave events as discussed in #3.

Test case (copied from #3):

Here's an image illustrating the issue:

I moved my mouse cursor along the white line. On the left, you can see the output of a modified wew (modified to print out the response_type, detail, and event (contains wid of the window) properties instead of only response_type and wid).

As you can see, the only times that current master would have detected an enter/leave event are the start, the point where it changes directions and goes back, and the end. All these have in common that these are transitions to and from the root window, which is what detail == 0 means.

Event description (in order):

response_type: ENTER (7) detail: 0 wid: 0x00a00009 Enter border of the first window (from root) response_type: LEAVE (8) detail: 2 wid: 0x00a00009 Leave border of the first window (to content) response_type: ENTER (7) detail: 2 wid: 0x00a00009 Enter border of the first window (from content) response_type: LEAVE (8) detail: 3 wid: 0x00a00009 Leave border of the first window (to content of the second window) response_type: ENTER (7) detail: 4 wid: 0x00800009 Enter content of the second window (from border of the first window) response_type: ENTER (7) detail: 2 wid: 0x00800009 Enter border of the second window (from content) response_type: LEAVE (8) detail: 0 wid: 0x00800009 Leave border of the second window (to root) response_type: ENTER (7) detail: 0 wid: 0x00800009 Enter border of the second window (from root) response_type: LEAVE (8) detail: 2 wid: 0x00800009 Leave border of the second window (to content) response_type: LEAVE (8) detail: 4 wid: 0x00800009 Leave content of the second window (to border of the first window) response_type: ENTER (7) detail: 3 wid: 0x00800009 Enter border of the first window (from content of the second window) response_type: LEAVE (8) detail: 2 wid: 0x00800009 Leave border of the first window (to content) response_type: ENTER (7) detail: 2 wid: 0x00800009 Leave border of the first window (to content) response_type: LEAVE (8) detail: 0 wid: 0x00800009 Leave border of the first window (to root)

laserswald commented 8 years ago

Okay, I am so pulling to mine. Thanks! I'll let you know if things do work.

greduan commented 8 years ago

Could have done one block of

/**
 * boop
 * bap
 * beep
 */

For the comments, instead of starting and ending a comment every line.

Ferdi265 commented 8 years ago

This makes sense. I'll change this in my fork real quick

laserswald commented 8 years ago

Looks good to me! Great job!

But you might want to disable the debugging or make it a flag.

Ferdi265 commented 8 years ago

But you might want to disable the debugging or make it a flag.

Which debugging? I only changed the if in motherfuckingenterevent, all other files are as-is.

If debugging is enabled in wmutils, it is in my fork. The master branch of my fork is primarily for the purpose of tracking my progress on this issue (#3), not miscellaneous improvements. The wewtest branch is for a test case mentioned in #3, and not used otherwise

laserswald commented 8 years ago

Whoops. I pulled that branch, not master. Sorry. Carry on.

On Tue, Nov 17, 2015 at 2:31 PM, Ferdi265 notifications@github.com wrote:

But you might want to disable the debugging or make it a flag.

Which debugging? I only changed the if in motherfuckingenterevent, all other files are as-is.

If debugging is enabled in wmutils, it is in my fork. The master branch of my fork is primarily for the purpose of tracking my progress on this issue (#3 https://github.com/wmutils/opt/issues/3), not miscellaneous improvements. The wewtest branch is for a test case mentioned in #3 https://github.com/wmutils/opt/issues/3, and not used otherwise

— Reply to this email directly or view it on GitHub https://github.com/wmutils/opt/pull/5#issuecomment-157479538.

Ferdi265 commented 8 years ago

Removing inline documentation to avoid polluting the diff with unneccessary lines. #3 documented this enough already

Ferdi265 commented 8 years ago

Rebased to just one commit. This commit is just a one line change, and should not clobber the git history that much

z3bra commented 8 years ago

I totally forgot to test/review this! I'm sorry it took so long! So after a few tests, it seems to be a bit more natural this way. If you focus a window whenever an "enter" event is raised, then the window under the cursor will ALWAYS have the focus. This will change a bit how scripts managing mouse focus would work though, as raising the focus window everytime such an event occur is a no go. It would make using the mouse impractical, so users will have to rely on alternative ways to raise windows on top (using super + mousescroll and super + r myself, and auto-raising when focusing with the keyboard.

Another point worth noticing is that now, as the focus follow the mouse, one as to move the cursor when moving a window, otherwise it might focus the window underneath.

Thanks for the commit and all the debugging work you did!

z3bra commented 8 years ago

This might be subject to a rollback, in case it makes the process harder (in terms of scripting, for example), as wrapping the mouse to follow every window movement might be tedious.

lwilletts commented 8 years ago

This isn't working for some of my windows. Notably google-chrome, teamspeak3 and a couple of others. Seems okay for others though. No idea what's causing this yet.

Ferdi265 commented 8 years ago

Can you try to check what events are actually sent when you do this? I had a custom branch for wew a while ago that printed the events for easy diagnostics. I don't have it around atm. Either you try it yourself or I'll see if I find my old branch...

lwilletts commented 8 years ago

Well, wew just produces no output (expecting 7) when moving over the majority of windows, mostly GTK ones if that makes any difference like chrome, teamspeak, gpick are a few I use on a regular basis. On non-GTK apps wew does produce the out of 7 i.e. urxvt and xterm windows.

Ferdi265 commented 8 years ago

What I meant, was that wew probably does get some events, but it ignores some of them (see motherfuckingenterevent()). I had a version of wew that printed it regardless. Well, I'm gonna be off recreating that patch for the current version.

Ferdi265 commented 8 years ago

so... can you try recompiling with this patch and tell me what your output is?

diff --git a/wew.c b/wew.c
index 9f53efc..b34c731 100644
--- a/wew.c
+++ b/wew.c
@@ -99,6 +99,13 @@ motherfuckingenterevent(xcb_generic_event_t *e)
        xcb_enter_notify_event_t *ee;

        ee = (xcb_enter_notify_event_t*)e;
+
+       printf(
+               "response_type: %i, detail: %i, mode: %i, self: 0x%x, other: 0x%x\n",
+               ee->response_type, ee->detail, ee->mode, ee->event, ee->child
+       );
+       fflush(stdout);
+
        if (ee->detail != XCB_NOTIFY_DETAIL_INFERIOR &&
                        (ee->mode == XCB_NOTIFY_MODE_NORMAL ||
                         ee->mode == XCB_NOTIFY_MODE_UNGRAB))
@@ -227,8 +234,9 @@ handle_events(void)
                }

                if (wid > 0) {
+                       /* disable output
                        printf("%d:0x%08x\n", e->response_type, wid);
-                       fflush(stdout);
+                       fflush(stdout); */
                }

                free(e);

This patch disables normal output and instead gives detailed output, but just for enter/leave events.

Try starting wew with -m 48 to set the mask correctly.

lwilletts commented 8 years ago

Sorry for the long wait, I've finally gotten around to doing this. Anyway, here's some data for you. The first window 0x800006 is a urxvt window, with the other two being google-chrome and teamspeak3, windows that don't work with normal wew.

response_type: 7, detail: 2, mode: 0, self: 0x800006, other: 0x0
response_type: 8, detail: 0, mode: 0, self: 0x800006, other: 0x0
response_type: 7, detail: 0, mode: 0, self: 0x800006, other: 0x0
response_type: 8, detail: 2, mode: 0, self: 0x800006, other: 0x0
response_type: 7, detail: 2, mode: 0, self: 0x800006, other: 0x0
response_type: 8, detail: 0, mode: 0, self: 0x800006, other: 0x0
response_type: 7, detail: 0, mode: 0, self: 0x800006, other: 0x0
response_type: 8, detail: 2, mode: 0, self: 0x800006, other: 0x0
response_type: 7, detail: 2, mode: 0, self: 0x800006, other: 0x0
response_type: 8, detail: 3, mode: 0, self: 0x800006, other: 0x0
response_type: 7, detail: 4, mode: 0, self: 0x1c00001, other: 0x2000003
response_type: 7, detail: 2, mode: 0, self: 0x1c00001, other: 0x0
response_type: 8, detail: 3, mode: 0, self: 0x1c00001, other: 0x0
response_type: 7, detail: 3, mode: 0, self: 0x2800005, other: 0x0
response_type: 8, detail: 3, mode: 0, self: 0x2800005, other: 0x0
response_type: 7, detail: 3, mode: 0, self: 0x1c00001, other: 0x0
response_type: 8, detail: 2, mode: 0, self: 0x1c00001, other: 0x0
response_type: 8, detail: 4, mode: 0, self: 0x1c00001, other: 0x2000003
response_type: 7, detail: 3, mode: 0, self: 0x800006, other: 0x0
response_type: 8, detail: 2, mode: 0, self: 0x800006, other: 0x0

Ferdi265 commented 8 years ago

Thanks for the data!

So... reconstructing what should have happened and what current master wew should output (considering no bugs). Events that should NOT be emitted by current master wew are marked with a -, otherwise a +

- ENTER NotifyInferior (urxvt)
Mouse moved from content to border.
+ LEAVE NotifyAncestor (urxvt)
Mouse moved from border to root window.
+ ENTER NotifyAncestor (urxvt)
Mouse moved from root window to border.
- LEAVE NotifyInferior (urxvt)
Mouse moved from border to content.
- ENTER NotifyInferior (urxvt)
Mouse moved from content to border.
+ LEAVE NotifyAncestor (urxvt)
Mouse moved from border to root window.
+ ENTER NotifyAncestor (urxvt)
Mouse moved from root window to border.
- LEAVE NotifyInferior (urxvt)
Mouse moved from border to content.
- ENTER NotifyInferior (urxvt)
Mouse moved from content to border.
+ LEAVE NotifyNonLinear (urxvt)
Mouse moved from border to other window.
+ ENTER NotifyNonLinearVirtual (google-chrome ?)
Mouse entered window content without crossing border (window partially occluded by other window).
- ENTER NotifyInferior (google-chrome ?)
Mouse moved from content to border.
+ LEAVE NotifyNonLinear (google-chrome ?)
Mouse moved from border to other window.
+ ENTER NotifyNonLinear (ts3 ?) Mouse moved from other widow to border without crossing root window (windows directly adjacent/tiled?)
+ LEAVE NotifyNonLinear (ts3 ?)
Mouse moved from border to other window.
+ ENTER NotifyNonLinear (google-chrome ?) Mouse moved from other widow to border without crossing root window (windows directly adjacent/tiled?)
- LEAVE NotifyInferior (google-chrome ?)
Mouse moved from border to content.
+ LEAVE NotifyNonLinearVirtual (google-chrome ?)
Mouse moved from content to other window without crossing border.
+ ENTER NotifyNonLinear (urxvt)
Mouse moved from other window to border
- LEAVE NotifyInferior (urxvt)
Mouse moved from border to content.

Is this what you did?

~~I'll do a quick analysis on this and edit this post accordingly. See again in a few minutes.~~

So after looking at this it seems odd that your normal wew didn't recognize google-chrome and ts3... Because they ARE emitting events. Are you sure your regular wew is running code from at least commit 8754fc9ddcf31d99f79e214088389eb0af3e2966? (Merged Nov 17)

lwilletts commented 8 years ago

response_type: 7, detail: 2, mode: 0, self: 0x1800006, other: 0x0
response_type: 8, detail: 3, mode: 0, self: 0x1800006, other: 0x0
response_type: 7, detail: 4, mode: 0, self: 0x1c00001, other: 0x2000003
response_type: 8, detail: 4, mode: 0, self: 0x1c00001, other: 0x2000003
response_type: 7, detail: 3, mode: 0, self: 0x1800006, other: 0x0
response_type: 8, detail: 2, mode: 0, self: 0x1800006, other: 0x0

Yeah, the code is from 8754fc9ddcf31d99f79e214088389eb0af3e2966 is being used. I've simplified the data slightly. This time it's just moving between a urxvt window and google-chrome.

Wew is clearly getting the correct information, as 0x1c00001 refers correctly to the google-chrome window. What does window does 0x2000003 refer to? It doesn't show up in lsw -a output.

Ferdi265 commented 8 years ago

These two events have detail 4, NotifyNonLinearVirtual. The Virtual means that this event is actually for another window, namely a child window of google chrome.

root
 |-> urxvt (move from here) NotifyNonLinear
 \-> google-chrome NotifyNonLinearVirtual
      \-> child (to here) NotifyNonLinear

0x2000003 is probably that child window of chrome

And doing this test doesn't work with normal wew?

lwilletts commented 8 years ago

I finally found the issue..

Setting XCB_EVENT_MASK_BUTTON_PRESS on by either using -m or manually compiling it means that wew fails to produce window event 7 consistently or at all! I don't really know C at all but hope this helps.

Ferdi265 commented 8 years ago

... That is ... interesting.

Seems worthy of another issue to me.

I'll try to reproduce that some time soon, can you create an issue for that? (since you found it first so it is reasonable to give you the credit for this)

Oh boy, this simple change of one constant in the code has uncovered quite a lot here.

Thanks for all the debugging work. I personally know the core syntax of C, but don't have much experience with it yet. Helping to fix bugs in small projects like this works wonders for learning a programming language. And even a simple pointing out of something that seems strange can be a lot of help to us. I'll just cite Linus's Law here: "given enough eyeballs, all bugs are shallow". So thanks for everything.

wmutils / opt

Fix wew enter/leave events (ee->detail behavior) #5