[RFC v2] xf86-input-evdev: Slotted MT events and mixed mode devices

[Peter Hutterer]

On Sat, Jul 03, 2010 at 01:39:14PM -0400, Chase Douglas wrote:
> Changes from v1:
> * A few small bug fixes
> * Make mtdev configure detection automatic by default
> * Rename mtdevPtr to mtdev
> * Split out mtdev open and close to separate functions
> * Some coding style fixes

please send patches that don't have an ACK yet to the list. A pull request
is much harder to review than a patch.
Worse, I expect pull requests to be reviewed patches so I spend half my time
figuring out whether a patch is really new or whether I'm just going senile.
 
> The following changes since commit 421585fda6ce67c209d43952109dda056ee40941:
>   Alex Warg (1):
>         Fix out-of-bounds access if more than MAX_VALUATORS are present. (#28809)
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/cndougla/xf86-input-evdev.git master
> 
> Benjamin Tissoires (2):
>       Add the names of the valuators for the multitouch properties

Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

>       Add multitouch support

I haven't seen this patch on the list yet, no? Some parts look familiar, but
the coreEvents setting is new.
 
> Chase Douglas (3):
>       Use valuator mask for posting events

for this patch:
+    uint8_t val_mask[(MAX_VALUATORS - 1) / 8 + 1]; /* Mask of modified vals

can we use bits_to_bytes() here?

also, why are we still passing num_v around here? if the valuators array is
already fixed size (MAX_VALUATORS) and we have a mask argument, can't we
just interpret that as fixed size as well.

>       Use mtdev to receive all events using MT Slots protocol

I'd prefer an approach like this:
    +static void
    +MtdevOpen(InputInfoPtr pInfo)
    +{
    + #ifdef USE_MTDEV
    +    EvdevPtr pEvdev = pInfo->private;
    +    pEvdev->mtdev = malloc(sizeof(struct mtdev));
    +    if (!pEvdev->mtdev || mtdev_open(pEvdev->mtdev, pInfo->fd)) {
    +        free(pEvdev->mtdev);
    +        pEvdev->mtdev = NULL;
    +        xf86Msg(X_WARNING, "%s: failed to initialize mtdev.\n",
    +                pInfo->name);
    +    }
    + #endif
    +}


from the configure.ac hunk (yeah, copy/paste kinda broke it):

+AC_ARG_WITH(mtdev,
+            AC_HELP_STRING([--with-mtdev],
+                           [Use mtdev library for multitouch evdev protocol
translation [default=auto]]),
+            [],
+            [with_mtdev=check])
+AS_IF([test "x$with_mtdev" != xno],
+      [AS_IF([test "x$with_mtdev" != xyes],
+             [PKG_CHECK_MODULES(MTDEV, mtdev,
+                                AC_DEFINE(USE_MTDEV, 1, Whether to use
mtdev or not),
+                                AC_DEFINE(USE_MTDEV, 0, Whether to use
mtdev or not))],
+             [PKG_CHECK_MODULES(MTDEV, mtdev, AC_DEFINE(USE_MTDEV, 1,
Whether to use mtdev or not))])],
+      AC_DEFINE(USE_MTDEV, 0, Whether to use mtdev or not))

please look at the xserver's configure.ac file (e.g. the Xnest "auto"
setting) for a somewhat less confusing approach.

fwiw, I want this patch Reviewed-by henrik, he knows the mtdev API.

>       Add support for mixed mode (Absolute/Relative) devices

ok, I give up with copy/paste, please send this one to the list as well.
well, all of them please once you've fixed them up. Benjamin's valuator
properties patch is fine, please don't modify this one anymore.

Cheers,
  Peter