This includes support for them on libinput and turns it into fake
pointer actions.
This doesn't implement zwp_tablet, this will have to happen in an
iteration later.
Details
- Reviewers
romangg - Group Reviewers
Plasma KWin - Commits
- R108:f5a73b87e3e7: Initial support for tablets on Wayland
Been playing around with it, see video.
https://www.youtube.com/watch?v=GF1WbO8FVvU
Diff Detail
- Repository
- R108 KWin
- Branch
- input1
- Lint
Lint Skipped Excuse: x - Unit
No Unit Test Coverage - Build Status
Buildable 19655 Build 19673: arc lint + arc unit
- I suppose this is related to T11660? If so relate the diff and the object.
- Improve commit message, explain your future steps.
- For touch events we already have a fallback to pointer events in KWayland in case the client does not support touch events. I would prefer that we follow the same pattern with tablet events. Otherwise argue why a input event filter is better suited in this case.
- Adhere to styling rules. From what I have seen:
- Line length
- Brackets
- Enum definition
- libinput interfacing code looks good
input.cpp | ||
---|---|---|
1540 | unused | |
1563 | There is a input()->pointer() getter. Remove member variable. | |
libinput/connection.cpp | ||
471 | Do not use single letter variable names. Minimum two letters. Same below with e. | |
libinput/events.h | ||
220 | Implement or add a notice why these are commented out / a TODO. | |
pointer_input.h | ||
60 ↗ | (On Diff #70691) | Change to hash and getter in separate patch. Under the assumption we keep the new input event filter this needs to go into input.h/input.cpp or some other device-independent file and not anymore in pointer_input. |
- Still needs several styling corrections.
- I would say as with touch and pointer input there should be a focus update when the pen touches down (update() function)
tablet_input.cpp | ||
---|---|---|
93 | No inline comments, make it mutliple lines, put comment as // comments behind arguments. | |
tablet_input.h | ||
77 | Implement in cpp file. There use Q_UNUSED macro. |
I would say as with touch and pointer input there should be a focus update when the pen touches down (update() function)
It doesn't make much sense that we need to update() focus from input collection, especially in this patch that is about exposing the data. I'm not sure though.
Ok, we can look at that later.
See remaining style comments.
input.cpp | ||
---|---|---|
1975 | These lines are still longer than 100 chars. | |
libinput/events.h | ||
227 | These lines are still longer than 100 chars. | |
tablet_input.cpp | ||
113 | These lines are still longer than 100 chars. | |
tablet_input.h | ||
56 | These lines are still longer than 100 chars. |
Now it's fucked up all over the place.
debug_console.cpp | ||
---|---|---|
494 | better before. + in front | |
510 | brackets | |
523 | brackets | |
527 | + in front | |
536 | + in front | |
546 | + in front | |
input.cpp | ||
1975 | clang-format stupid | |
input.h | ||
98 | clang-format stupid | |
libinput/connection.h | ||
134 | unnecessary many lines. group xTilt, yTilt etc. | |
libinput/events.h | ||
200 | clang-format stupid |
I don't care about any whitespace stuff, but code-wise this looks fine.
+1 when Roman's happy.
input.h | ||
---|---|---|
233 | bracket | |
libinput/events.h | ||
200 | If you define member functions in the class itself these brackets must be behind the function name, i.e.: bool xHasChanged() const { .... } | |
281 | For code comments please use proper punctuation and capitalization. | |
tablet_input.cpp | ||
124 | Split like this: input()->processSpies(std::bind(&InputEventSpy::tabletToolButtonEvent, std::placeholders::_1, m_toolPressedButtons)); Below as well. | |
131 | Brackets! |
input.h | ||
---|---|---|
233 | From Frameworks coding style wiki page
So, the opening brace must be on the next line. |
input.h | ||
---|---|---|
233 | Are we doing it somewhere in KWin this way? |
input.h | ||
---|---|---|
233 | Only in a few places... but I'd like to point out that putting the opening brace and the method prototype on the same line goes against the coding style. |
input.h | ||
---|---|---|
233 | I haven't seen these places. I always see in KWin code opening brackets on same line in this case. So I would prefer to be consistent in deviating from coding style here. Note also that getters keyboard(), pointers(),... directly above this new getter have opening brackets on the same line. In the end best would be to always have member function definitions in the cpp file. But for small getters it's really a hassle... |
input.h | ||
---|---|---|
233 | There are places where we use the other style, but not that many. Hopefully, we'll fix these annoying style issues one day with a little help from beloved clang-format. ;-)
++ |