koreader/koreader-base

Clobbering of slots in MT protocol handling

mapinguari opened this issue · 12 comments

Hi all,

If a device has multiple input fds each of which is conforming to the MT B protocol you end up with clobbering of MT slots.

For example, if I have two touch devices as above.

fd 1 produces:
`ABS_MT_SLOT 0

ABS_MT_TRACKING_ID 45

ABS_MT_POSITION_X 0

ABS_MT_POSITION_Y 0

SYN_REPORT
`
then some time later produces

`ABS_MT_POSITION_X 100

ABS_MT_POSITION_Y 100

SYN_REPORT`

and fd2 produces:
`ABS_MT_SLOT 0

ABS_MT_TRACKING_ID 6

ABS_MT_POSITION_X 1000

ABS_MT_POSITION_Y 1000

SYN_REPORT`

From two distinct touch events, say, one from a finger and one from a pen tool then depending on the order of the reads from the fds you can end up with any permutation of touches at (0,0), (100,100) and (1000,1000).

The input functions need to make sure they are keeping track of which fd was the last to be read from, what the last slot it was producing events for was and if it is a different fd to ensure an ABS_MT_SLOT event for the current fd's last slot is inserted into the table of events being generated. It probably also needs to maintain a mapping between an individual fd's slot number and a "globally unique" slot number so you don't end up with clobbering of slots between different input devices.

I don't think this is a driver issue, each individual driver is doing the correct thing and respecting the MT protocol, it is just when you combine multiple input sources together into a combined stream you end up with unfortunate interlacing of the event streams.

I am not sure how much appetite you have to look at this tho.

Any feedback apprecited!

It's a design simplification, as this should be an exceedingly rare occurrence in practice.

(Also, we also clobber slots on contact losses on some Kobo protocols later, anyway).

Fair enough. I am working with the remarkable and its happening regularly.

I will do by best to handle it further downstream.

Want me to close this?

Want me to close this?

Not necessarily, base's issue tracker isn't crowded, so leaving this open isn't really problematic.

As far as I'm concerned, this would be fairly low on the priority list, even if I actually had time to work on this (spoiler: I don't have any time ;p).

Could possibly be as simple as passing along the fd number from the C module to Lua-land and simply using fd + ABS_MT_SLOT in GestureManager, as we don't actually care about the slots value per se (in fact, some Kobo devices don't even emit a slot 0).

Yeah I did think about something along those lines, but I am not entirely sure what goes on in KOReader wrt slots and ids once they go into gesture detector.

I am just getting some funny double free exceptions from somewhere and it seems to be related to touch and tool events mingling.

Might need to be slightly more complex, because of the initial state in https://github.com/koreader/koreader/blob/19a607b5484299623cc57563f7a4baa3f49faff5/frontend/device/input.lua#L220-L226 (among possibly other things).

If we had a guarantee that the first event device we open is the touch screen, we might be able to use the inputfds index instead, but we don't.

It would probably not be too gnarly to deal with, I just have zero time to devote to this (or anything else, really :/) for the foreseeable future.

ok cool.

I will leave it be.

There might be a similar issue with cross-device ABS_MT_TRACKING_ID values, too (although a collision there is possibly much rarer).

From my little bit of watching the event streams on this one device then yes, it looks rarer. One device always uses the same ABS_MT_TRACKING_ID 0, and the other just continuously increments it.

As far as passing along the fd info, it should only take some minor contortions to stuff it in the table ;).

that is probably the simplest unobtrusive fix