This adds support for following in fakeinput interface:
- touch_down
- touch_motion
- touch_up
- touch_cancel
- touch_frame
bshah | |
graesslin |
Plasma |
This adds support for following in fakeinput interface:
added autotests passes
No Linters Available |
No Unit Test Coverage |
autotests/client/test_fake_input.cpp | ||
---|---|---|
328 | In addition to what @graesslin said, one point about coding style, KWayland is part of KF5 which follows KDE Frameworks 5 Coding style guide for its code. You seem to have used inconsistent coding style for code here and as well other places. Please go over all the code you have added and correct it. | |
src/server/fakeinput_interface.h | ||
128–132 | Given this will be part of public API, please add the documentation for this methods like other pointer methods. |
src/client/fakeinput.h | ||
---|---|---|
133 | why a delta? In KWin all touchMotion related code takes the new position. | |
src/client/protocols/fake-input.xml | ||
52–68 | if you have a new enough wayland-scanner installed this should generate errors because it expects documentation. I'm aware that the existing interface is missing documentation, but I think for new code we should make sure that wayland-scanner is happy. | |
src/server/fakeinput_interface.cpp | ||
194 | nitpick: empty line | |
src/server/fakeinput_interface.h | ||
139 | same here |
I'm wondering: should we ensure that the ids are correct. E.g. a touchUp for id 1 doesn't make sense if we never got a touchDown for id 1. This would require tracking the used ids in FakeInputInterface. But it must be done somewhere - either in the library or by the user of the library.
src/server/fakeinput_interface.cpp | ||
---|---|---|
196 | The isAuthenticated check is not verified in the autotest. That's the same with the other new methods. |
IMO client ideally will not send bogus event. And in any case if it does, compositor itself should ignore bogus events because it is the one which is processing in end..
a client could also use this to attack (crash) the Compositor by sending bogus events. That's security relevant from my point of view.
Martin, do you prefer this change to be done in this review only or in separate review.
Hello, I've understood the problem and how to fix it in theory, but i am not sure on implementation details? For instance, I am not sure on where to track ids? On server side FakeInputInterface, FakeInputDevice or client side in FakeInput?
So, can you please help me on this?
On server side FakeInputInterface, FakeInputDevice or client side in FakeInput?
Needs to be done on server side. The thought is that the Client might send incorrect data (the interface is the protocol, not your client code), so the server needs to do input validation.
Looks good, but please fix the coding style.
src/server/fakeinput_interface.cpp | ||
---|---|---|
199–200 | please watch coding style - same in all other areas you just adjusted. |
Output of arc land fake-input-touch:
TARGET Landing onto "master", the default target under git.
REMOTE Using remote "origin", the default remote under git.
FETCH Fetching origin/master...
This commit will be landed:
Can i still submit it?