Introduce an InputEventSpy for processing input events
ClosedPublic
Actions

Authored by graesslin on Dec 29 2016, 6:05 PM.

Details

Reviewers

Group Reviewers

Plasma
KWin

Commits

R108:84e33081499f: Introduce an InputEventSpy for processing input events

Summary

So far KWin's input event processing is mostly based on
InputEventFilters. A filter can - as the name suggest - filter out an
input event from further processing. Our code shows that this is not
sufficient for all input event processing.

We have several areas inside KWin where we need to have access to all
input events, where the processing needs to happen on all events and
filtering is not allowed. This results in sub-optimal code which has
classes which know too much and do too much.

Examples:

key-repeat handling done in KeyboardInputRedirection
Layout change OSD in Xkb
modifier only shortcuts in Xkb
emitting signals for Cursor class in KeyboardInputRedirection

Also there are misuses of the InputEventFilters and internal API

DebugConsole keyboard state (uses wrong information)
DebugConsole input events tab (uses Filter, should be a spy)

This change introduces the API needed to fix these problems. It
introduces an InputEventSpy which is modelled after the InputEventFilter
with the difference that it has only void messages and uses the KWin
introduced event classes.

The spies are always processed prior to the filters, thus we know it can
have all events.

Diff Detail

Repository

R108 KWin

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

graesslin updated this revision to Diff 9463.Dec 29 2016, 6:05 PM

graesslin retitled this revision from to Introduce an InputEventSpy for processing input events.

graesslin updated this object.

graesslin edited the test plan for this revision. (Show Details)

graesslin added reviewers: KWin, Plasma.

Restricted Application added a project: KWin. · View Herald TranscriptDec 29 2016, 6:05 PM

Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript

Some minor stuff, looks good overall.

input.cpp
1394	Wouldn't removeAll() be a bit safer here, or is there a good reason to have spies enqueued twice? (In that case, duplicates should probably be checked before insertion. In any case, it should be consistent with the filters' behaviour, so just something to think about.
input.h
192	Code example would be nice here. Not critical, since it's not public API anyway, but would help me personally to understand a bit better how to use it.
input_event_spy.h
51	spy, not filter (we should use that language consistently: filters may remove events, spies just snoop)
57	spy
63	spy

This revision is now accepted and ready to land.Dec 29 2016, 7:49 PM

graesslin added inline comments.Dec 29 2016, 9:05 PM

input.cpp
1394	Yep agree. Having multiple filters/spies installed doesn't make sense. Due to that I chose remove one here. But the filters should be adjusted. Not sure about checking proof to inserting. It's internal API, so I don't see a real benefit in adding the runtime cost. An asset could be a possibility, though.
input.h
192	Yeah I'm aware that std::bind is something we hardly see in kde code yet (Ivan excluded). The example ae in this change. Giving an explicit code example here - difficult. Too much template magic and I do hope that there are booked and online resources which can explain it better than I could;-) My suggestion would be to read the code example and check with the c++ documentation what it is doing

graesslin marked 3 inline comments as done.Dec 30 2016, 5:29 PM

Closed by commit R108:84e33081499f: Introduce an InputEventSpy for processing input events (authored by graesslin). · Explain WhyDec 30 2016, 5:33 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		input.h (31 lines)
M		input.cpp (12 lines)
A	M	input_event_spy.h (86 lines)
A	M	input_event_spy.cpp (116 lines)
M		keyboard_input.cpp (2 lines)
M		pointer_input.cpp (12 lines)
M		touch_input.cpp (4 lines)