Uses the tablet classes introduced in kwayland.
Depends on D26858
Details
Scratched my tablet with a magic stick and it did things depending on the pressure.
https://youtu.be/GGx0TlNJlzs
Diff Detail
- Repository
- R108 KWin
- Branch
- master
- Lint
Lint Errors Excuse: coconut Severity Location Code Message Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro Error input.h:333 Cppcheck unknownMacro - Unit
No Unit Test Coverage - Build Status
Buildable 21576 Build 21594: arc lint + arc unit
Good stuff! There are a couple of coding style issues.
autotests/libinput/mock_libinput.h | ||
---|---|---|
108 | I know that you want to match libinput_device_tablet_pad_get_{value}, but please follow coding style. uint32_t buttonCount = 0; | |
input.cpp | ||
1550 | ||
1741 | No short names. Toplevel *toplevel = ... | |
1742–1743 | Should have braces. | |
input_event.h | ||
183 | It's off by a single space. | |
input_event_spy.h | ||
89 | Align pointers and reference to right. | |
libinput/connection.cpp | ||
510–512 | Please don't submit commented code unless there is a reason. |
input.cpp | ||
---|---|---|
1559 | Use explicit type const char *devnode = udev_device_get_devnode(udev_device); | |
1621 | Align pointers to right. | |
1623–1626 | No short names. Toplevel *toplevel = input()->findToplevel(event->globalPos()); if (!toplevel || !toplevel->surface()) { return false; } | |
1628 | Align pointers to right. | |
1631–1632 | Add missing braces. | |
2065–2066 | Please put a single space between for and the opening parenthesis, add missing braces and use explicit type. | |
input_event_spy.cpp | ||
124 | Align pointers to right. |
Addressed the comments. I hope that all that really is a problem with the patch is styling issues...
Code-wise, +1
Please wait for a +1 from @davidedmundson
input.cpp | ||
---|---|---|
1564 | Naming nitpick: maybe addDevice? | |
input_event.h | ||
193 | Make it const? |
input.cpp | ||
---|---|---|
1586 | I think this should be event->uniqueId() |
input.cpp | ||
---|---|---|
1586 | We are using the same value as the second argument of addTablet() (se below). If we change here the serialId, we'd have to change it there too and we'd have the same behaviour. |
We are using the same value as the second argument of addTablet() (se below). If we change here the serialId, we'd have to change it there too and we'd have the same behaviour.
You might be right about serialId having read the docs, but I'm still seeing waaaaaay too many:
zwp_tablet_seat_v2.tool_added()
one for every move.
I have kwin on master + this patch, and master of kwayland.
I'll try and debug and find out.
#0 0x00007f9962c49ce5 in raise () at /usr/lib/libc.so.6 #1 0x00007f9962c33857 in abort () at /usr/lib/libc.so.6 #2 0x00007f9963357faf in qt_message_fatal(QtMsgType, QMessageLogContext const&, QString const&) (context=..., message=...) at /home/david/projects/qt/src/qtbase/src/corelib/global/qlogging.cpp:1914 #3 0x00007f99633545e6 in QMessageLogger::fatal(char const*, ...) const (this=0x7ffdb72ca9f0, msg=0x7f9963719ef0 "ASSERT failure in %s: \"%s\", file %s, line %d") at /home/david/projects/qt/src/qtbase/src/corelib/global/qlogging.cpp:893 #4 0x00007f996334adf3 in qt_assert_x(char const*, char const*, char const*, int) (where=0x7f9965176763 "zwp_tablet_v2::removed", what=0x7f99651756ba "Uninitialised resource", file=0x7f9965175653 "src/server/qwayland-server-tablet-unstable-v2.cpp", line=1229) at /home/david/projects/qt/src/qtbase/src/corelib/global/qglobal.cpp:3362 #5 0x00007f9965156770 in QtWaylandServer::zwp_tablet_v2::send_removed() (this=0x56487ab837c0) at src/server/qwayland-server-tablet-unstable-v2.cpp:1229 #6 0x00007f996511c79d in KWayland::Server::TabletSeatInterface::removeTablet(QString const&) (this=0x56487ab9cdb0, sysname=...) at /home/david/projects/kde5/src/frameworks/kwayland/src/server/tablet_interface.cpp:341 #7 0x00007f9967e1c3a4 in KWin::TabletInputFilter::removeDevice(QString const&) (this=0x56487acc7f50, sysname=...) at /home/david/projects/kde5/src/kde/workspace/kwin/input.cpp:1578 #8 0x00007f9967e270a3 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QString>, void, void (KWin::TabletInputFilter::*)(QString const&)>::call(void (KWin::TabletInputFilter::*)(QString const&), KWin::TabletInputFilter*, void**) (f= (void (KWin::TabletInputFilter::*)(KWin::TabletInputFilter * const, const QString &)) 0x7f9967e1c378 <KWin::TabletInputFilter::removeDevice(QString const&)>, o=0x56487acc7f50, arg=0x7f99540179c8) at /opt/qt5/include/QtCore/qobjectdefs_impl.h:152
input.cpp | ||
---|---|---|
1651 | Yes, x and y in the motion event must be in the surface-local coordinates, i.e. relative to the upper left corner of the main surface. |
input.cpp | ||
---|---|---|
1651 | btw, maybe it might be worth to introduce a method to map points from the global screen coordinates to surface-local coordinates, e.g. QPoint Toplevel::mapToLocal(const QPoint &point) const { return point - bufferGeometry().topLeft(); } QPointF Toplevel::mapToLocal(const QPointF &point) const { return point - QPointF(bufferGeometry().topLeft()); } |