<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126513/">https://git.reviewboard.kde.org/r/126513/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 10th, 2016, 6:43 p.m. UTC, <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/126513/diff/3/?file=425999#file425999line51" style="color: black; font-weight: bold; text-decoration: underline;">kcms/touchpad/src/backends/x11/propertyinfo.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">51</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">format</span> <span class="o">==</span> <span class="n">CHAR_BIT</span> <span class="o">&&</span> <span class="n">type</span> <span class="o">==</span> <span class="n">XA_INTEGER</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Effectively we end up switching on the data type twice; both here and in value()
Given we store the format, I'd merge this into value() and drop the b,i,f member variables.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">but this is the same as hte original code, so it's fine.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">synapthcs uses b,i,f fields directly, keep it as it is looks easier for those code to access the value instead of always using QVariant.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Maybe clean up that part later.</p></pre>
<br />
<p>- Xuetian</p>
<br />
<p>On December 25th, 2015, 7:02 p.m. UTC, Xuetian Weng wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma and David Edmundson.</div>
<div>By Xuetian Weng.</div>
<p style="color: grey;"><i>Updated Dec. 25, 2015, 7:02 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=349545">349545</a>,
<a href="https://bugs.kde.org/show_bug.cgi?id=356923">356923</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Currently, there are some mixed issue in kcm touchpad related to libinput backend and hot plug.
There are several issues:
1. only one backend maybe used at runtime, either synaptics or libinput. But libinput backend will only be used if there is a libinput backend present. Which means if libinput touchpad is not present at login, kded will not be able to properly support them.
2. hotplug touchpad will always set touchpad to disabled.
3. hotplug touchpad will not get configuration applied.
4. The libinput devices detection may pick some non-touchpad device. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To solve these problems, following changes are made:
1. Remove SynapticsBackend and LibinputBackend and add three new classes called XlibTouchpad/SynapticsTouchpad/LibinputTouchpad. So findTouchpad may pick up both synaptics or libinput device, and no need to depend on a fixed backend. This change also make it easier to support multiple touchpad device easier if needed.</p>
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">hotplug touchpad config not being applied (bug 356923) is mainly because one can't apply settings to a disabled device. Change the statement order in handleReset, apply settings first then set enable status.</p>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">findTouchpad() is changed to use XListInputDevices, because it provides necessary information of device type in order to filter out non-touchpad device. xf86-input-libinput doesn't have a unique atom for touchpad, so current identityAtom cannot guarantee that findTouchpad always returns a touchpad device for libinput.</p>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">On my surface pro 4 system, if touchpad is unpluged, the device will be set to disabled automatically first, then touchpadDetached is called, which makes m_enabled in TouchpadDisabled always to be false after unplug the touchapd. This patch makes TouchpadDisabler have two different enabled property. m_userRequestedState only stores the user requested status, so when handleReset is called, it can properly set enabled state to the last user requested state.</p>
</li>
</ol></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Without touchpad -> login -> plug touchpad -> settings applied to device using libinput driver.
unplug -> replug -> settings applied to device using libinput driver.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Disable touchpad -> applet shows up -> unplug touchpad -> applet hides -> replug touchpad -> touchpad is still disabled, and applet shows up -> enable touhcpad -> applet hides.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kcms/touchpad/src/applet/qml/contents/ui/touchpad.qml <span style="color: grey">(8dec7c2)</span></li>
<li>kcms/touchpad/src/applet/touchpadengine.h <span style="color: grey">(16b98c2)</span></li>
<li>kcms/touchpad/src/applet/touchpadengine.cpp <span style="color: grey">(eae429e)</span></li>
<li>kcms/touchpad/src/backends/x11.cmake <span style="color: grey">(c9fcea8)</span></li>
<li>kcms/touchpad/src/backends/x11/libinputproperties.c <span style="color: grey">(9dbf9ea)</span></li>
<li>kcms/touchpad/src/backends/x11/libinputtouchpad.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/backends/x11/libinputtouchpad.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/backends/x11/propertyinfo.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/backends/x11/propertyinfo.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/backends/x11/synapticstouchpad.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/backends/x11/synapticstouchpad.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/backends/x11/synclientproperties.h <span style="color: grey">(43d18d2)</span></li>
<li>kcms/touchpad/src/backends/x11/synclientproperties.c <span style="color: grey">(5fd1ed6)</span></li>
<li>kcms/touchpad/src/backends/x11/xlibbackend.h <span style="color: grey">(7cecb4a)</span></li>
<li>kcms/touchpad/src/backends/x11/xlibbackend.cpp <span style="color: grey">(b55a45f)</span></li>
<li>kcms/touchpad/src/backends/x11/xlibtouchpad.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/backends/x11/xlibtouchpad.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kcms/touchpad/src/kded/kded.h <span style="color: grey">(9b8fe6e)</span></li>
<li>kcms/touchpad/src/kded/kded.cpp <span style="color: grey">(409126b)</span></li>
<li>kcms/touchpad/src/touchpadbackend.h <span style="color: grey">(b225ed9)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126513/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>