D8748: [libinput] Find screen for touch screen devices and calibrate accordingly

[David Edmundson]

davidedmundson added a comment.


  One minor comment (the third one), otherwise all good.

INLINE COMMENTS

> connection.cpp:395
>              case LIBINPUT_EVENT_TOUCH_DOWN: {
> +#ifndef KWIN_BUILD_TESTING
>                  TouchEvent *te = static_cast<TouchEvent*>(event.data());

not that it's a problem, but why? 
You have screens mocked already.

> connection.cpp:397
>                  TouchEvent *te = static_cast<TouchEvent*>(event.data());
> -                emit touchDown(te->id(), te->absolutePos(m_size), te->time(), te->device());
> +                const auto &size = screens()->size(te->device()->screenId());
> +                emit touchDown(te->id(), screens()->geometry(te->device()->screenId()).topLeft() + te->absolutePos(size), te->time(), te->device());

if you fetch the geometry here you'll get both the size and topLeft in an easier to read way.

> connection.cpp:488
>  
> +void Connection::updateScreens()
> +{

you're not updating in deviceAdded(), so hypothetically if one adds a touch screen at runtime, and plugs in the HDMI cable before the USB cable it won't have the right screen.

You could put this method (and the screens changed connect) in Device as you're not doing anything with Connection in this method.

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D8748

To: graesslin, #kwin, #plasma
Cc: davidedmundson, plasma-devel, kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171110/f8865685/attachment-0001.html>