[PATCH weston 21/25] protocol: add weston_touch_calibration

Fri Apr 13 07:51:37 UTC 2018

On Fri, 13 Apr 2018 14:31:39 +1000
Peter Hutterer <peter.hutterer at who-t.net> wrote:

> On Wed, Apr 11, 2018 at 02:00:49PM +0300, Pekka Paalanen wrote:
> > On Wed, 11 Apr 2018 10:16:46 +1000
> > Peter Hutterer <peter.hutterer at who-t.net> wrote:  

> > > > > > +  <interface name="weston_touch_calibrator" version="1">
> > > > > > +    <description summary="calibrator surface for a specific touch device">
> > > > > > +      On creation, this object is tied to a specific touch device. The
> > > > > > +      server sends a configure event which the client must obey with the
> > > > > > +      associated wl_surface.
> > > > > > +
> > > > > > +      Once the client has committed content to the surface, the server can
> > > > > > +      grab the touch input device, prevent it from emitting normal touch events,
> > > > > > +      show the surface on the correct output, and relay input events from the
> > > > > > +      touch device via this protocol object.
> > > > > > +
> > > > > > +      Touch events from other touch devices than the one tied to this object
> > > > > > +      must generate wrong_touch events on at least touch-down and must not
> > > > > > +      generate normal or calibration touch events.
> > > > > > +
> > > > > > +      At any time, the server can choose to cancel the calibration procedure by
> > > > > > +      sending the cancel_calibration event. This should also be used if the
> > > > > > +      touch device disappears or anything else prevents the calibration from
> > > > > > +      continuing on the server side.
> > > > > > +
> > > > > > +      If the wl_surface is destroyed, the server must cancel the calibration.
> > > > > > +
> > > > > > +      The touch event coordinates and conversion results are delivered in
> > > > > > +      calibration units. Calibration units are in the closed interval
> > > > > > +      [0.0, 1.0] mapped into 32-bit unsigned integers. An integer can be      
> > > > > 
> > > > > should probably add what 0.0 and 1.0 represent on each axis, i.e. nominal
> > > > > width and height of the device's sensor. Or is this something we need to
> > > > > hide?    
> > > > 
> > > > I'm not sure. The underlying assumption is that there is a finite and
> > > > closed range of values a sensor can output, and that is mapped to [0,
> > > > 1]. I don't know what nominal width and height are or can the values
> > > > extend to negative. It might be easier to define the calibration units
> > > > without defining those first.    
> > > 
> > > sorry, I used "nominal" before I rewrote to use "sensor", probably made it
> > > more confusing. On some devices, the sensor is larger than the screen so you
> > > can get coordinates outside the screen area. This is not a technical
> > > problem, just a user perception mismatch that doens't need to be exposed and
> > > after all that's what calibration is about.
> > > 
> > > Once you apply calibration though you can get real negative coordinates,
> > > especially if the device advertises a [n:m] range but then sends events less
> > > than n. For example, all synaptics touchpads (pre 2014, some after that) do
> > > that.  
> > 
> > But wait, if there are actually devices that report the range [n:m] but
> > can still send values less than n or greater than m, it means I cannot
> > transmit those with the encoding set up here, because
> > libinput_event_touch_get_x_transformed(touch_event, 1) can return
> > values outside of [0, 1] even when the loaded calibration matrix is
> > identity. Is that right?  
> 
> correct, that can happen.
> 
> > Do I need to find an encoding to cope with that, or can I just reject
> > such touches?
> > 
> > I mean, the range [n:m] is in raw input coordinates, before any
> > normalization or calibration in userspace, right?  
> 
> uhm. let me just detail this and you can pick which one was the answer to
> that question:
> - the kernel exposes [n:m] axis ranges for ABS_X/Y and the MT equivalents
> - libinput converts this into mm by default, see libinput_event_touch_get_x()
> - libinput converts this to a ranged value [0:N] when calling
>   libnput_event_touch_get_x_transformed(N). This is what weston uses to get
>   the screen coordinates.
> 
> Both libinput values have the calibration factored in already.
> 
> Weston doesn't ever see raw input coordinates. But if you use
> get_x_transformed, you can get values outside [0:N]. This happens when
> the [n:m] range is incorrect. Accommodating for some error margin, libinput
> will print a warning to the log when this happens.
> 
> The correct solution would be to warn the user that the device is garbage
> and make them put a 60-evdev.hwdb quirk in to fix the axis ranges.
> Depending on the device, this could be a generic solution but let's not bet
> on that...
> 
> The user-friendly solution would be to have two layers of mapping. By
> default, you map the libinput range into [0, 1]. Whenever you see negative
> values or values > 1 you notify the user that this screen is garbage. The
> only option you have now is to get the user to touch the furthest extents of
> all axes to get the lowest and highest values the axis provides (similar to
> the touchpad-edge-detector tool). Then you can map those into the libinput
> provided range so that e.g. libinput's -0.25 is normalized 0 and 1.25 is
> normalized 1.0.
> 
> Now you can restart calibration with that extra mapping on top. 
> 
> A few caveats, beyond the effort required:
> - even with the calibration matrix applied, the libinput ranges will still
>   be outside the [0:N] range. Only a real axis fix can fix that
> - you cannot know the true min/max until you get respective event from the
>   screen
> 
> So yeah, the situation is less than ideal...
> 
> I've considered auto-scaling libinput into the new axis range but this is
> likely going to hurt even more because it's undiscoverable and for most
> devices, a quirk in the hwdb file can solve this problem properly.

Thank you very much for the thorough explanation.

I think the appropriate solution for me here is to check whether the
normalized value I expect to be in [0, 1] falls outside of the range,
and if so, I will a) print a warning in Weston's log that the device
may need a quirk like you explained, and b) send the "invalid touch"
event instead of a touch event to the calibrator app.

This makes the calibrator app show a big red cross, so the user will
know something is strange if he thinks he did touch the correct spot.

Well, it does touch-downs at least. I think if motion event would end
up out of range, I'll send cancel event followed by invalid touch.
Whee, I found use for the cancel event!

> > > These devices are a problem because we rely on the coordinate range for the
> > > size and then on the calibration on top to normalize the input. So they may
> > > need two fixes, one for the range adjustment, one for calibration.  
> > 
> > If I can just transmit whatever values I get for "normalized" with
> > identity in the above, then the calibration matrix will automatically
> > account for any out-of-range values - I mean they become a non-issue,
> > because the compositor just tranforms them to the output or a global
> > coordinate system first and then clips to the actual output area. Or
> > doesn't clip, makes no difference.
> >   
> > > > It doesn't really matter, the important point is that when the
> > > > resulting matrix is loaded into libinput and then libinput is used
> > > > according to its API, the result is as expected. I'd be tempted to just
> > > > punt all this to libinput.    
> > > 
> > > libinput will sort-of punt it back :)
> > > https://wayland.freedesktop.org/libinput/doc/latest/tablet-support.html#tablet-bounds
> > >   
> > 
> > I don't see a problem in that section. The goal of this calibration is
> > to match input event coordinates to output pixel coordinates in the
> > complete pipeline by adjusting the normalized calibration matrix in
> > libinput. It's ok for calibrated input coordinates go beyond any
> > limits, the input and output coordinate planes can be thought as
> > infinite.
> > 
> > Or do you refer to the functions returning positions in millimeters? I
> > never looked at those, they're not used with touchscreens. I don't even
> > know if a calibration matrix affects them.  
> 
> There's a corner-case with touchscreens and mm. Plenty of cheap touchscreens
> don't have a resolution set, so the mm value is calculated using the default
> resolution of 1.
> 
> The calibration matrix is applied to the mm as well. That's largely a
> side-effect for the matrix originally being used for rotation rather than
> scaling or translation.

Right. If someone needs this calibration method to work on devices that
user by millimeter units, I'll let them figure out what to fix if
anything. :-)

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180413/294931b7/attachment.sig>