On Thu, Apr 10, 2014 at 08:17:37AM +0200, Jonas Ådahl wrote: > On Thu, Apr 10, 2014 at 03:43:25PM +1000, Peter Hutterer wrote: > > On Wed, Apr 09, 2014 at 09:02:08PM +0200, Jonas Ådahl wrote: > > > Don't have a hard coded (previously 16) slot array size; instead > > > allocate dynamically depending what slots are assigned. There is still a > > > hard coded max though, to protect from invalid input, but its changed > > > to 60. > > > > > > Signed-off-by: Jonas Ådahl <jad...@gmail.com> > > > --- > > > src/evdev.c | 82 > > > +++++++++++++++++++++++++++++++++++++++++++++---------------- > > > src/evdev.h | 13 ++++++---- > > > 2 files changed, 69 insertions(+), 26 deletions(-) > > > > > > diff --git a/src/evdev.c b/src/evdev.c > > > index 72e4086..cc0c4bf 100644 > > > --- a/src/evdev.c > > > +++ b/src/evdev.c > > > @@ -280,6 +280,36 @@ evdev_process_key(struct evdev_device *device, > > > struct input_event *e, int time) > > > } > > > } > > > > > > +static int > > > +alloc_mt_slot_states(struct evdev_device *device, int len) > > > > I'd prefer this to be size_t, and nslots instead of len to be more obvious > > re:malloc. > > To use size_t here I had to change cast size_t to a signed integer (or > other way around), but sure, I'll make the change.
maybe ssize_t? either way, I'm more concerned about len vs nslots, the int/size_t doesn't matter that much. > > > > > > +{ > > > + struct mt_slot *slots = device->mt.slots; > > > + int i; > > > + > > > + slots = realloc(slots, sizeof(struct mt_slot) * len); > > > + if (!slots) > > > + return -1; > > > + > > > + for (i = device->mt.slots_len; i < len; ++i) { > > > + slots[i].seat_slot = -1; > > > + slots[i].x = 0; > > > + slots[i].y = 0; > > > + } > > > + device->mt.slots = slots; > > > + device->mt.slots_len = len; > > > + > > > + return 0; > > > +} > > > + > > > +static void > > > +set_active_slot(struct evdev_device *device, int32_t slot) > > > +{ > > > + slot = min(slot, MAX_SLOTS); > > > + if (slot >= device->mt.slots_len) > > > + alloc_mt_slot_states(device, device->mt.slots_len * 2); > > > + device->mt.slot = min(slot, device->mt.slots_len - 1); > > > +} > > > > couldn't we just do alloc_mt_slots(dev, libevdev_num_slots(evdev)) during > > configure? the kernel filters events that are outside of the announced slot > > range, libevdev filters events outside of the slot range so this code seems > > to churn a lot for no reason. > > > > libevdev 1.2 also gets rid of the MAX_SLOTS define, so we don't need this > > limit anymore. > > MAX_SLOTS is defined in libinput/src/evdev.h. I guess we could allocate > less ad-hoc; until now I've kept MAX_SLOTS check to protect against > potentially misbehaving driver (if a driver lazilly advertises max slots > to some very high number for example). Will libevdev protect against > this? no, it doesn't. OTOH we could assume that if this happens it's a kernel bug and should be fixed there. Cheers, Peter > > > > > > + > > > static void > > > evdev_process_touch(struct evdev_device *device, > > > struct input_event *e, > > > @@ -288,7 +318,7 @@ evdev_process_touch(struct evdev_device *device, > > > switch (e->code) { > > > case ABS_MT_SLOT: > > > evdev_flush_pending_event(device, time); > > > - device->mt.slot = e->value; > > > + set_active_slot(device, e->value); > > > break; > > > case ABS_MT_TRACKING_ID: > > > if (device->pending_event != EVDEV_NONE && > > > @@ -543,9 +573,11 @@ evdev_device_dispatch(void *data) > > > static int > > > evdev_configure_device(struct evdev_device *device) > > > { > > > + struct libevdev *evdev = device->evdev; > > > > tbh, I'd have preferred this to be a separate change. without this change, > > the next hunks would only be 3 lines or so and much easier to review. > > Sure, I'll split it out. > > > > > Cheers, > > Peter > > > > > > > const struct input_absinfo *absinfo; > > > int has_abs, has_rel, has_mt; > > > int has_button, has_keyboard, has_touch; > > > + int active_slot; > > > unsigned int i; > > > > > > has_rel = 0; > > > @@ -555,14 +587,14 @@ evdev_configure_device(struct evdev_device *device) > > > has_keyboard = 0; > > > has_touch = 0; > > > > > > - if (libevdev_has_event_type(device->evdev, EV_ABS)) { > > > + if (libevdev_has_event_type(evdev, EV_ABS)) { > > > > > > - if ((absinfo = libevdev_get_abs_info(device->evdev, ABS_X))) { > > > + if ((absinfo = libevdev_get_abs_info(evdev, ABS_X))) { > > > device->abs.min_x = absinfo->minimum; > > > device->abs.max_x = absinfo->maximum; > > > has_abs = 1; > > > } > > > - if ((absinfo = libevdev_get_abs_info(device->evdev, ABS_Y))) { > > > + if ((absinfo = libevdev_get_abs_info(evdev, ABS_Y))) { > > > device->abs.min_y = absinfo->minimum; > > > device->abs.max_y = absinfo->maximum; > > > has_abs = 1; > > > @@ -570,56 +602,58 @@ evdev_configure_device(struct evdev_device *device) > > > /* We only handle the slotted Protocol B in weston. > > > Devices with ABS_MT_POSITION_* but not ABS_MT_SLOT > > > require mtdev for conversion. */ > > > - if (libevdev_has_event_code(device->evdev, EV_ABS, > > > ABS_MT_POSITION_X) && > > > - libevdev_has_event_code(device->evdev, EV_ABS, > > > ABS_MT_POSITION_Y)) { > > > - absinfo = libevdev_get_abs_info(device->evdev, > > > ABS_MT_POSITION_X); > > > + if (libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_X) && > > > + libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_Y)) { > > > + absinfo = libevdev_get_abs_info(evdev, > > > ABS_MT_POSITION_X); > > > device->abs.min_x = absinfo->minimum; > > > device->abs.max_x = absinfo->maximum; > > > - absinfo = libevdev_get_abs_info(device->evdev, > > > ABS_MT_POSITION_Y); > > > + absinfo = libevdev_get_abs_info(evdev, > > > ABS_MT_POSITION_Y); > > > device->abs.min_y = absinfo->minimum; > > > device->abs.max_y = absinfo->maximum; > > > device->is_mt = 1; > > > has_touch = 1; > > > has_mt = 1; > > > > > > - if (!libevdev_has_event_code(device->evdev, EV_ABS, > > > ABS_MT_SLOT)) { > > > + if (!libevdev_has_event_code(evdev, > > > + EV_ABS, ABS_MT_SLOT)) { > > > device->mtdev = mtdev_new_open(device->fd); > > > if (!device->mtdev) > > > return -1; > > > - device->mt.slot = > > > device->mtdev->caps.slot.value; > > > + active_slot = device->mtdev->caps.slot.value; > > > } else { > > > - device->mt.slot = > > > libevdev_get_current_slot(device->evdev); > > > + active_slot = libevdev_get_current_slot(evdev); > > > } > > > + set_active_slot(device, active_slot); > > > } > > > } > > > - if (libevdev_has_event_code(device->evdev, EV_REL, REL_X) || > > > - libevdev_has_event_code(device->evdev, EV_REL, REL_Y)) > > > + if (libevdev_has_event_code(evdev, EV_REL, REL_X) || > > > + libevdev_has_event_code(evdev, EV_REL, REL_Y)) > > > has_rel = 1; > > > > > > - if (libevdev_has_event_type(device->evdev, EV_KEY)) { > > > - if (libevdev_has_event_code(device->evdev, EV_KEY, > > > BTN_TOOL_FINGER) && > > > - !libevdev_has_event_code(device->evdev, EV_KEY, > > > BTN_TOOL_PEN) && > > > + if (libevdev_has_event_type(evdev, EV_KEY)) { > > > + if (libevdev_has_event_code(evdev, EV_KEY, BTN_TOOL_FINGER) && > > > + !libevdev_has_event_code(evdev, EV_KEY, BTN_TOOL_PEN) && > > > (has_abs || has_mt)) { > > > device->dispatch = evdev_mt_touchpad_create(device); > > > } > > > for (i = KEY_ESC; i < KEY_MAX; i++) { > > > if (i >= BTN_MISC && i < KEY_OK) > > > continue; > > > - if (libevdev_has_event_code(device->evdev, EV_KEY, i)) { > > > + if (libevdev_has_event_code(evdev, EV_KEY, i)) { > > > has_keyboard = 1; > > > break; > > > } > > > } > > > - if (libevdev_has_event_code(device->evdev, EV_KEY, BTN_TOUCH)) > > > + if (libevdev_has_event_code(evdev, EV_KEY, BTN_TOUCH)) > > > has_touch = 1; > > > for (i = BTN_MISC; i < BTN_JOYSTICK; i++) { > > > - if (libevdev_has_event_code(device->evdev, EV_KEY, i)) { > > > + if (libevdev_has_event_code(evdev, EV_KEY, i)) { > > > has_button = 1; > > > break; > > > } > > > } > > > } > > > - if (libevdev_has_event_type(device->evdev, EV_LED)) > > > + if (libevdev_has_event_type(evdev, EV_LED)) > > > has_keyboard = 1; > > > > > > if ((has_abs || has_rel) && has_button) > > > @@ -670,7 +704,6 @@ evdev_device_create(struct libinput_seat *seat, > > > device->mtdev = NULL; > > > device->devnode = strdup(devnode); > > > device->sysname = strdup(sysname); > > > - device->mt.slot = -1; > > > device->rel.dx = 0; > > > device->rel.dy = 0; > > > device->dispatch = NULL; > > > @@ -680,6 +713,12 @@ evdev_device_create(struct libinput_seat *seat, > > > > > > libinput_seat_ref(seat); > > > > > > + device->mt.slot = -1; > > > + device->mt.slots = NULL; > > > + device->mt.slots_len = 0; > > > + if (alloc_mt_slot_states(device, 4) == -1) > > > + goto err; > > > + > > > if (evdev_configure_device(device) == -1) > > > goto err; > > > > > > @@ -786,6 +825,7 @@ evdev_device_destroy(struct evdev_device *device) > > > > > > libinput_seat_unref(device->base.seat); > > > libevdev_free(device->evdev); > > > + free(device->mt.slots); > > > free(device->devnode); > > > free(device->sysname); > > > free(device); > > > diff --git a/src/evdev.h b/src/evdev.h > > > index 0ab9572..4ec387a 100644 > > > --- a/src/evdev.h > > > +++ b/src/evdev.h > > > @@ -31,7 +31,7 @@ > > > > > > #include "libinput-private.h" > > > > > > -#define MAX_SLOTS 16 > > > +#define MAX_SLOTS 60 > > > > > > enum evdev_event_type { > > > EVDEV_NONE, > > > @@ -50,6 +50,11 @@ enum evdev_device_seat_capability { > > > EVDEV_DEVICE_TOUCH = (1 << 2) > > > }; > > > > > > +struct mt_slot { > > > + int32_t seat_slot; > > > + int32_t x, y; > > > +}; > > > + > > > struct evdev_device { > > > struct libinput_device base; > > > > > > @@ -74,10 +79,8 @@ struct evdev_device { > > > > > > struct { > > > int slot; > > > - struct { > > > - int32_t seat_slot; > > > - int32_t x, y; > > > - } slots[MAX_SLOTS]; > > > + struct mt_slot *slots; > > > + int slots_len; > > > } mt; > > > struct mtdev *mtdev; > > > > > > -- > > > 1.8.3.2 > > > > > > _______________________________________________ > > > wayland-devel mailing list > > > wayland-devel@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel