On Fri, Feb 07, 2014 at 08:09:21AM +1000, Peter Hutterer wrote: > On Thu, Feb 06, 2014 at 10:23:57PM +0100, Jonas Ådahl wrote: > > On Thu, Feb 06, 2014 at 02:13:04PM +1000, Peter Hutterer wrote: > > > > > > This patchset revamps the path backend to allow for more than one > > > path-based > > > device per context. I thought the initial approach of having one context > > > per > > > device is sufficient but there are a few use-cases that can really only be > > > solved by having libinput control all devices. A common example is > > > disabling > > > the touchpad while typing, or making the trackstick buttons on the Lenovo > > > T440s useful. > > > > > > So for my little libinput-based xorg driver [1] I need to have a shared > > > context between the various devices added. The change looks bigger than it > > > is, it largely just replicates what the udev-seat backend already does > > > anyway. > > > > > > Most notable there are a few API changes: > > > - libinput_create_from_path() has been removed, a caller should now use > > > libinput_path_create_context(), then libinput_path_add_device() > > > > Why not just libinput_path_create()? > > I wanted to make it explicit that all this call does is create the context, > as opposed to e.g. udev_create_... which adds devices too.
I thought libinput_path_create() was clear enough as one didn't pass any device path. Anyway, libinput_path_create_context() is fine as well. > > > > I found this to be nicer in the caller code than having > > > libinput_path_create_from_device() and then adding devices. > > > - more devices can be added or removed with libinput_path_add_device and > > > libinput_path_remove_device > > > - to ensure proper namespacing, libinput_create_from_udev is now > > > libinput_udev_create_for_seat() > > > > These API changes looks good to me, but should maybe suspend and resume > > behaviour be documented? Is it required to re-add devices after having > > resumed? > > it's not required, the behaviour is that suspend/resume removes and re-adds > all devices, or fails if a device cannot be added. exactly the same behavour > as the udev seat. I'll expand on the suspend/resume documentation in a > separate patch. > > > > So far this looks flexible enough for the xorg drivers which have > > > different > > > use-cases than in weston, specifically each device can be disabled and > > > enabled individually. I just call remove/add device when that happens. > > > > > > What's not so nice is a shortcut in libinput_path_add_device(). Instead of > > > just succeeding and letting the caller handle the DEVICE_ADDED event, it > > > returns the struct libinput_device directly. At least within the xorg > > > driver > > > context I found it too hard to work with the events (long description on > > > request) but the short summary is that I'd need to cache any events before > > > DEVICE_ADDED and use timers to re-trigger the read once I have the device, > > > aside from the problem of not being able to figure out which device is > > > which > > > based on the events only (we don't expose the devnode to the caller). > > > > So I guess you can't just add and then flush and process the queue right > > there, as the DEVICE_ADDED event will already have been queued when > > calling libinput_path_add_device()? > > Not without doing some work to the server, the enable/disable bits are in a > quirky order and called from different places than the actual event > processing. So without auditing the call paths I can't guarantee that the > server is in the correct state for handling events when you get the first > couple of events. Returning the device pointer was the less-intrusive but > still somewhat-sensible approach :) Not going to argue with that :) It would be a bit awkward anyway to deal with events that way. > > > We could also simply expose the devnode to the caller as well, > > caching and matching later is not very nice. > > IMO we should do this anyway, otherwise we'd require the caller to use udev > and subsystem guessing* to match it up themselves. At least a devnode is > non-ambiguous. > > Cheers, > Peter > > * probably not much of an issue since we don't deal with serial devices, so > we can assume subsystem is always "input" _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel