D26858: Provide an implementation for the tablet interface

This revision was automatically updated to reflect the committed changes. Closed by commit R127:a009c8c0d91a: Provide an implementation for the tablet interface (authored by apol). REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=78028=78055 REVISION

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 78028. apol added a comment. rebase REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=78027=78028 BRANCH arcpatch-D26858 REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED FILES CMakeLists.txt

D26858: Provide an implementation for the tablet interface

davidedmundson accepted this revision. REPOSITORY R127 KWayland BRANCH arcpatch-D26858 REVISION DETAIL https://phabricator.kde.org/D26858 To: apol, #kwin, #frameworks, zzag, davidedmundson Cc: davidedmundson, zzag, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 78027. apol added a comment. Fix the refcounting Expose teh set_cursor interface as well REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=77926=78027 BRANCH arcpatch-D26858 REVISION DETAIL

D26858: Provide an implementation for the tablet interface

davidedmundson added inline comments. INLINE COMMENTS > davidedmundson wrote in tablet_interface.cpp:58 > This is still potentially crashy: > if all clients remove their tablet support we would have a dangling pointer > in m_tablets. > > Personally I would remove this delete q, and have the

D26858: Provide an implementation for the tablet interface

davidedmundson added inline comments. INLINE COMMENTS > tablet_interface.cpp:58 > +if (resourceMap().isEmpty()) { > +delete q; > +} This is still potentially crashy: if all clients remove their tablet support we would have a dangling pointer in m_tablets.

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 77926. apol added a comment. Only delete when all the subscribed resources are gone. REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=77861=77926 BRANCH arcpatch-D26858 REVISION DETAIL

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 77861. apol added a comment. Fix tablet removal REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=77839=77861 BRANCH arcpatch-D26858 REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED FILES

D26858: Provide an implementation for the tablet interface

zzag added inline comments. INLINE COMMENTS > test_tablet_interface.cpp:1-19 > +/ > +Copyright 2020 Aleix Pol Gonzalez > + > +This library is free software; you can redistribute it and/or > +modify it under the terms of the GNU

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 77839. apol added a comment. Address comment by David REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=75564=77839 BRANCH arcpatch-D26858 REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED FILES

D26858: Provide an implementation for the tablet interface

davidedmundson added inline comments. INLINE COMMENTS > tablet_interface.cpp:346 > + > +TabletToolInterface *TabletSeatInterface::toolByHardwareId(quint64 serialId) > const > +{ We're mixing up hardwareId and hardwareSerial this method is called toolByHardwareId calls an internal method

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 75564. apol added a comment. Only call destruction if it was an owned tablet, this way we don't crash. REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=75561=75564 BRANCH arcpatch-D26858 REVISION DETAIL

D26858: Provide an implementation for the tablet interface

apol added a comment. In D26858#605869 , @davidedmundson wrote: > Feedback from some testing: > > - we're adding the same tool a bunch of times when a new client is created > - sometimes we fail to enter the surface leaving the cursor

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 75561. apol added a comment. rebase on master REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=74473=75561 BRANCH arcpatch-D26858 REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED FILES

D26858: Provide an implementation for the tablet interface

davidedmundson added a comment. Feedback from some testing: - we're adding the same tool a bunch of times when a new client is created - sometimes we fail to enter the surface leaving the cursor "stuck" (with gtk3-demo) I'll see if I can spot why. FWIW, Qt have a tablet test

D26858: Provide an implementation for the tablet interface

zzag accepted this revision. zzag added a subscriber: davidedmundson. zzag added a comment. This revision is now accepted and ready to land. Code-wise, this change looks good. I don't have a tablet device to test this patch so you may want to wait for a +1 from @davidedmundson. INLINE

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 74473. apol added a comment. clang format REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=74241=74473 BRANCH apol/tablet-unstable-v2-1 REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED FILES

D26858: Provide an implementation for the tablet interface

zzag added a comment. Looks good to me. There are still many coding style issues. I suggest to run `clang-format` over individual files in this patch and then adjust the most problematic places (I don't think there are that many). You could borrow a `.clang-format` file from

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 74241. apol added a comment. Address comments REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=74217=74241 BRANCH master REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED FILES CMakeLists.txt

D26858: Provide an implementation for the tablet interface

apol marked 11 inline comments as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D26858 To: apol, #kwin, #frameworks, zzag Cc: zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26858: Provide an implementation for the tablet interface

zzag requested changes to this revision. zzag added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > test_tablet_interface.cpp:227 > +QCOMPARE(m_surfaces.count(), 3); > +for (SurfaceInterface* surface : m_surfaces) { > +

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 74217. apol added a comment. ehm.. REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=74214=74217 BRANCH master REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED FILES CMakeLists.txt

D26858: Provide an implementation for the tablet interface

apol reopened this revision. apol added a comment. Phabricator chokes on git branches... REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D26858 To: apol, #kwin, #frameworks, zzag Cc: zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26858: Provide an implementation for the tablet interface

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R127:958cb1777b8d: Use K_DOXYGEN instead of deprecated DOXYGEN_SHOULD_SKIP_THIS (authored by kossebau). CHANGED PRIOR TO

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 74213. apol added a comment. remove unrelated bits REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=74212=74213 BRANCH apol/tablet-unstable-v2 REVISION DETAIL https://phabricator.kde.org/D26858 AFFECTED

D26858: Provide an implementation for the tablet interface

apol updated this revision to Diff 74212. apol added a comment. Addressed zzag's comments, fixed the patch overall. REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26858?vs=74187=74212 BRANCH apol/tablet-unstable-v2 REVISION DETAIL

D26858: Provide an implementation for the tablet interface

apol marked 6 inline comments as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D26858 To: apol, #kwin, #frameworks, zzag Cc: zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26858: Provide an implementation for the tablet interface

zzag requested changes to this revision. zzag added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > test_tablet_interface.cpp:2 > +/ > +Copyright 2014 Martin Gräßlin > + Haven't you written

D26858: Provide an implementation for the tablet interface

apol added a dependent revision: D26859: Implement the tablet wayland protocol in kwin. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D26858 To: apol, #kwin, #frameworks Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26858: Provide an implementation for the tablet interface

apol created this revision. apol added reviewers: KWin, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. apol requested review of this revision. REVISION SUMMARY Implements the necessary classes to have proper support for the tablet and pen.