Refactor x11 event handling #1819
Conversation
✅ Deploy Preview for conkyweb canceled.
|
| *consumed = false; | ||
| #endif /* BUILD_MOUSE_EVENTS */ | ||
|
|
||
| if (!own_window.get(*state)) return true; |
There was a problem hiding this comment.
This is the only part that changes functionality.
| @@ -1411,6 +1413,9 @@ void propagate_x11_event(XEvent &ev) { | |||
|
|
|||
| XUngrabPointer(display, CurrentTime); | |||
| XSendEvent(display, i_ev->common.window, True, ev_to_mask(i_ev->type), &ev); | |||
| if (focus) { | |||
| XSetInputFocus(display, i_ev->common.window, RevertToParent, CurrentTime); | |||
There was a problem hiding this comment.
I moved XSetInputFocus here, after sending the event, if it's a ButtonPress.
| break; | ||
| } | ||
| } | ||
| break; |
There was a problem hiding this comment.
These consecutive breaks made no sense to me, so I tracked down that the window type check was intended to avoid propagation of events (inner break) in the original code.
Over several commits, I seem to have made it do nothing.
| case window_type::TYPE_UTILITY: | ||
| // decorated normal windows always consume events | ||
| if (!TEST_HINT(own_window_hints.get(*state), HINT_UNDECORATED)) { | ||
| *consumed = true; |
There was a problem hiding this comment.
So now, it still passes the events to Lua, but force-disables propagation in case of normal decorated window, or desktop. Also added comments to make it clear in future.
Events now get correctly propagated to a window (or root if none) behind conky. This was a necessary change to handle cases such as MATE+caja where caja is used between conky and background to show icons and desktop menu. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Atoms should be faster than graph traversal, and also don't include decorations (windows) inserted by WM/DEs, nor special 1x1 windows. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Causes input focus flickering, especially with WMs where focus follows pointer. This looks weird (carret flashing) and effectively acheives nothing. So I'm, leaving it up to WMs to manage focus. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
display_output_x11::main_loop_wait was getting long and hard to read. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
I think splitting it out into separate functions makes each part much clearer and easier to follow, as well as reducing the complexity of define conditions Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Caellian
force-pushed
the
fix/separate-x11-events
branch
from
f613160
to
57efc63
Compare
This commit is final refactor commit that's fully functional and contains fixes to old bugs and previously introduced regressions. There's some extra arguments (cookie) that aren't used yet, but will be in commits that succeed this one. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Caellian
force-pushed
the
fix/separate-x11-events
branch
from
57efc63
to
cfdf5ac
Compare
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
| bool handle_event(conky::display_output_x11 *surface, Display *display, | ||
| XEvent &ev, bool *consumed, void **cookie) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Replaced EV_HANDLER macro with a templated function.
Nice perk with this is that there's no need for explicit NOOP_EV_HANDLER.
src/display-x11.cc
Outdated
| @@ -675,33 +708,30 @@ EV_HANDLER(damage) { | |||
| x11_stuff.part); | |||
| return true; | |||
| } | |||
| #else | |||
| NOOP_EV_HANDLER(damage) | |||
There was a problem hiding this comment.
Which removes these #else blocks
| if (handle_event<x_event_handler::event>(surface, display, ev, consumed, \ | ||
| cookie)) { \ | ||
| return true; \ | ||
| } |
There was a problem hiding this comment.
Can I keep a macro here?
It's right next to invocation, won't be called anywhere else, and quarters the code below. I would use a "compile-time for loop" to loop over all x_event_handler values, but those don't exist in C++.
There was a problem hiding this comment.
Yeah, I think this particular case is fine.
| if (handle_event<x_event_handler::event>(surface, display, ev, consumed, \ | ||
| cookie)) { \ | ||
| return true; \ | ||
| } |
There was a problem hiding this comment.
Yeah, I think this particular case is fine.
Move event handling code into separate functions.
Move event handling code into separate functions.
This PR refactors event handling into separate functions in order to make the event handling code more readable. It's separated out of #1807 in order to make that future PR cleaner.