Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ts_read_mt for 'single' touch devices #103

Closed
craftyguy opened this issue Sep 5, 2017 · 20 comments
Closed

ts_read_mt for 'single' touch devices #103

craftyguy opened this issue Sep 5, 2017 · 20 comments

Comments

@craftyguy
Copy link

craftyguy commented Sep 5, 2017

This is a question, not an issue.

My project is using DirectFB, which uses tslib for handling touch input. The current code in DirectFB is calling ts_read(), however this means that MT devices are pretty much ignored. Would ts_read_mt generally serve as a replacement in this case to handle both MT devices (w/o EV_KEY) and 'single' touch devices (where EV_KEY is used)? The relevant code in DirectFB is only using the x, y, and pressure members of the ts_sample_mt. I didn't see much documentation to suggest that ts_read_mt does (or doesn't) return touch events for all touch devices or only MT devices.

Thanks!

@merge
Copy link
Member

merge commented Sep 5, 2017

both ts_read and ts_read_mt should read all touchscreen devices, MT or not. So this is a bug. Do you have this problem in a tslib release or the current git version only?

I quickly ran tests/ts_print and it works using a multitouch device. So what error messages do you get using ts_read() with a multitouch device.

(currently in git, we check whether we have EV_KEY, and only if we do, we fail for singletouch devices without BTN_TOUCH or BTN_LEFT.)

@merge
Copy link
Member

merge commented Sep 5, 2017

also, the output of evtest would probably help.

@merge
Copy link
Member

merge commented Sep 5, 2017

Also, ts_print_mt works with such a singletouch device:


Supported events:
  Event type 0 (EV_SYN)
  Event type 1 (EV_KEY)
    Event code 330 (BTN_TOUCH)
  Event type 3 (EV_ABS)
    Event code 0 (ABS_X)
      Value   3236
      Min        0
      Max     4095
    Event code 1 (ABS_Y)
      Value   3455
      Min        0
      Max     4095
    Event code 24 (ABS_PRESSURE)
      Value      0
      Min        0
      Max      255

please specify your problem. thanks for reporting!

@craftyguy
Copy link
Author

Thank you. I'm being a little cautious here because this is probably not a bug in tslib but a misunderstanding on my part of some components in tslib.

ts_test_mt seems to work perfectly find on this device I am looking at.

Just to give you some insight into what I am doing, I am writing a patch (which can be seen here) to update DirectFB (original source here) to support ts_read_mt instead of ts_read.

Is ts_sample.x the same as ts_sample_mt.x or should I be using ts_sample_mt.tool_x as a replacement?

@merge
Copy link
Member

merge commented Sep 5, 2017

don't use tool_x. That's something different and not even used as of today. I don't even know a device that supports it.

@craftyguy
Copy link
Author

I see. Thank you for the clarification. Something else is likely going on here, I will close this ticket now since I don't have any other questions about tslib! Thanks for making a great, useful library!

@merge
Copy link
Member

merge commented Sep 5, 2017

make sure you allocate correctly, and only "once". We have a slightly unorthodox API here, with a pointer per " number of samples". See the example code and ask if not clear

@merge
Copy link
Member

merge commented Sep 5, 2017

I mean "nr of samples" pointers (like you have with the old ts_read() api, how many you to read non-blocking for example) each pointing to an array of real samples. those arrays are "slots" long.

@craftyguy
Copy link
Author

craftyguy commented Sep 5, 2017 via email

@merge
Copy link
Member

merge commented Sep 5, 2017

no. The fact that the ts_test_mt app works doesn't mean your app works. 2 different apps using tslib :) It seems like you're only allocating a pointer, but not the array of length 1 (1slot) that pointer should point at...

@craftyguy
Copy link
Author

craftyguy commented Sep 5, 2017 via email

@merge
Copy link
Member

merge commented Sep 5, 2017

If it helps I'll write what I mean in a blindly written patch against directfb tomorrow.

malloc, like in ts_test_mt or ts_print_mt has to be done outside of your tslibEventThread().

@craftyguy
Copy link
Author

craftyguy commented Sep 5, 2017 via email

@craftyguy
Copy link
Author

both ts_read and ts_read_mt should read all touchscreen devices, MT or not. So this is a bug.

Somehow I read this earlier but it didn't quite register until @Pneumaticat pointed it out. Yes, this is a bug since ts_test doesn't work on his device but ts_test_mt does. He will be filing a separate bug for this and providing more info. Maybe a patch to DirectFB isn't necessary afterall!

@merge
Copy link
Member

merge commented Sep 5, 2017

Your patch to directfb is a good start. What you have to consider too though is that you have to optionally add ts_read_mt(). Basically #ifdef TSLIB_VERSION_MT. It still has to be able to be built against an old tslib with only ts_read(). At least if you think about patching directfb's main source upstream.

While having only 1 slot with ts_read_mt() is netter than ts_read, wouldn't directfb benefit from having multitouch, i.e. a fixed number of available slots, 5 or so? It would then at least be a possible followup patch.

@craftyguy
Copy link
Author

craftyguy commented Sep 5, 2017 via email

@merge
Copy link
Member

merge commented Sep 6, 2017

Now I added a small change to ts_read() and it would be great if it could be tested if it works (for example with directfb).

I looked at directfb, but I don't know if it really would add value to have multitouch. I don't see other input driver that use multitouch or a way how to pass on multitouch data in directfb (no touch id in an input sample or something?). I might overlook something though.

It wouldn't be all that complicated. For the compiler, there is only one event thread (either ts_read of ts_read_mt) because you can do it with the preprocessor definition TSLIB_VERSION_MT. xf86-input-tslib does the same thing: https://github.com/merge/xf86-input-tslib/blob/master/src/tslib.c

And regarding slots at the tslib side of things: There is nothing you have to worry about. You allocate as many slots as you want, and pass the number to ts_read_mt() and the buffers will get filled up (a samples' "valid" variable gets 1 if you should care about it and 0 if have to ignore it). If it's a MT device, delivering multiple touch points currently, they just get valid in your allocated samples-array of length "slots". Just always read all slot's "valid" variable. If you allocate only 5 slots, and have a device capable of 10 slots, you simply limit the user to 5.

@merge
Copy link
Member

merge commented Sep 6, 2017

Something like this might work. Its completely untested:

merge/DirectFB@011444b

but as I said. internally I haven't yet checked if DirectFB could make use of having real multitouch data. If yes, now it would be trivially easy to support it.

@merge
Copy link
Member

merge commented Sep 6, 2017

but then again, please test the old ts_read() once again with your MT-only devices.

@craftyguy
Copy link
Author

Yea that was my conclusion as well (about adding multitouch support to directfb). I (wrongly) thought that using ts_read_mt was necessary to support MT devices until #104.

Your patch is very helpful in demonstrating how to correctly handle old & new tslib versions, thank you for that!

I am preparing a package now with the latest commit in it so some users with MT-only devices can test with ts_read()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants