[PATCH weston 21/25] protocol: add weston_touch_calibration

[Peter Hutterer]

On Fri, Apr 13, 2018 at 10:51:37AM +0300, Pekka Paalanen wrote:
> 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.

I'd argue that this is worthy of a custom event, given that the client will
have to provide a specific UI for this case. But it's also not worth
worrying about in the first iteration of the protocol.

> 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!

The cancel event would also be used for the multitouch-case, right?
If a second touch appears before the first one is fully processed, you also
need to cancel the first touch. Doesn't even need to be intentional by the
user, could be a palm touch or an accidental touch where they're holding the
screen on an edge.

Cheers,
   Peter

> > > > 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