linux-surface / iptsd

Userspace daemon for Intel Precise Touch & Stylus
GNU General Public License v2.0
94 stars 46 forks source link

Iptsd compatibility with Chrome / Chromium OS #22

Closed sebanc closed 3 years ago

sebanc commented 3 years ago

Hi linux-surface team,

First of all, thanks a lot for your work on the new ipts driver / daemon, I use it in linux and it works very well.

This issue is related to Chrome / Chromium OS (only tested ChromeOS but I guess it affects all chromium distros) and I would completely understand if you are not interested in spending time on this so don't hesitate to close it if it is the case.

1) The first part of this issue is that under the main ChromeOS desktop, touch events do not work by default with iptsd. I started discussing it with @kitakar5525 (https://github.com/kitakar5525/linux-kernel/issues/3) and he had already figured out that ABS_X and ABS_Y events are needed for ChromeOS to consider the device as a touchscreen and process the events. I believe it is due to:

bool EventDeviceInfo::HasTouchscreen() const {
  return HasAbsXY() && HasDirect();
}

in https://source.chromium.org/chromium/chromium/src/+/master:ui/events/ozone/evdev/event_device_info.cc

2) After adding support for ABS_X and ABS_Y events in the driver, the touchscreen is recognized and touch seems to work fine in Wayland windows but taps are behaving weirdly on the main desktop (even though HID events look correct in evtest). I have tried different things over the last week but nothing I tried seem to have any effect on this behavior. You will find attached a short screen-capture of this issue (touches are captured as white circles) and you can see that:

iptsd.zip

If you have any idea of which part of the code could be responsible for this weird behavior, it would already be a great help.

Note: My device is a Surface Pro 2017 and I believe Kitakar experience the same issue on his Surface Book 1.

Thanks a lot !

StollD commented 3 years ago

After adding support for ABS_X and ABS_Y events in the driver

Have you tried to not actually send these events and instead only advertise them? I could imagine that if you send both, ABS_X and ABS_MT_POSITION_X the event system could get confused and interpret both events as two seperate touches. That would kinda explain your issues, because it would see two inputs (and possibly a gesture) instead of one (and a simple touch).

diff --git a/src/devices.c b/src/devices.c
index 300a0e7..fd76102 100644
--- a/src/devices.c
+++ b/src/devices.c
@@ -135,6 +135,18 @@ static int iptsd_devices_create_touch(struct iptsd_devices *devices,
    int resX = iptsd_devices_res(IPTS_MAX_X, devices->config.width);
    int resY = iptsd_devices_res(IPTS_MAX_Y, devices->config.height);

+   abs_setup.code = ABS_X;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_X;
+   abs_setup.absinfo.resolution = resX;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
+   abs_setup.code = ABS_Y;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_Y;
+   abs_setup.absinfo.resolution = resY;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
    abs_setup.code = ABS_MT_SLOT;
    abs_setup.absinfo.minimum = 0;
    abs_setup.absinfo.maximum = devices->device_info.max_contacts;
StollD commented 3 years ago

Also the patch from @kitakar5525 has a kinda critical bug because it will send all inputs as ABS_X. ABS_X is meant for single touches, so you need to pick the first one and only send that. Otherwise the input will jump between both fingers all the time.

sebanc commented 3 years ago

Thanks StollD for helping with this :)

Actually, I made the same assumption regarding ABS_X and ABS_Y values so I tried only advertising ABS_X and ABS_Y without actually sending the events (the video I attached in the first post was taken when ABS_X and ABS_Y were only advertised and not sent). However, I had added ABS_X and ABS_Y setup after ABS_MT_POSITION_Y so I tried your patch to see if the order mattered and unfortunately the issue persists.

Regardless of whether we only advertise events without sending them, use Kitakar's patch or send the first finger position for ABS_X and ABS_Y values, the issue appears to be the same.

I also tried adding BUTTON_TOUCH in case it would somehow be related but it did not have noticeable effects.

StollD commented 3 years ago

Hmm. You were running the old GuC based driver without issues under ChromeOS right? Could you get me an evtest dump of that (including the dump of supported events from the start)? I tried to make the devices in iptsd 1:1 copies of the devices that the old driver created, but its been probably a year since I touched that driver so I might have missed something.

sebanc commented 3 years ago

Indeed, until now I was still using 4.19 kernel with the old driver in chromeos and it appeared to work correctly.

You will find below the requested logs and corresponding screen-captures, do not hesitate to ask if you need something else.

logs.zip

Thanks for all the help :)

StollD commented 3 years ago

Ok, what about this?

diff --git a/src/devices.c b/src/devices.c
index 300a0e7..4c0e12d 100644
--- a/src/devices.c
+++ b/src/devices.c
@@ -2,6 +2,7 @@

 #include <errno.h>
 #include <fcntl.h>
+#include <linux/input-event-codes.h>
 #include <linux/uinput.h>
 #include <math.h>
 #include <stdint.h>
@@ -130,11 +131,28 @@ static int iptsd_devices_create_touch(struct iptsd_devices *devices,
    memset(&abs_setup, 0, sizeof(struct uinput_abs_setup));

    iptsd_utils_ioctl(file, UI_SET_EVBIT, (void *)EV_ABS);
+   iptsd_utils_ioctl(file, UI_SET_EVBIT, (void *)EV_KEY);
+   iptsd_utils_ioctl(file, UI_SET_EVBIT, (void *)EV_MSC);
+
    iptsd_utils_ioctl(file, UI_SET_PROPBIT, (void *)INPUT_PROP_DIRECT);
+   iptsd_utils_ioctl(file, UI_SET_KEYBIT, (void *)BTN_TOUCH);
+   iptsd_utils_ioctl(file, UI_SET_MSCBIT, (void *)MSC_TIMESTAMP);

    int resX = iptsd_devices_res(IPTS_MAX_X, devices->config.width);
    int resY = iptsd_devices_res(IPTS_MAX_Y, devices->config.height);

+   abs_setup.code = ABS_X;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_X;
+   abs_setup.absinfo.resolution = resX;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
+   abs_setup.code = ABS_Y;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_Y;
+   abs_setup.absinfo.resolution = resY;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
    abs_setup.code = ABS_MT_SLOT;
    abs_setup.absinfo.minimum = 0;
    abs_setup.absinfo.maximum = devices->device_info.max_contacts;
diff --git a/src/touch.c b/src/touch.c
index 88c3cb3..41777fd 100644
--- a/src/touch.c
+++ b/src/touch.c
@@ -4,6 +4,7 @@
 #include <linux/uinput.h>
 #include <stdint.h>
 #include <string.h>
+#include <time.h>

 #include "context.h"
 #include "devices.h"
@@ -13,23 +14,38 @@
 #include "touch-processing.h"
 #include "utils.h"

-static void iptsd_touch_emit(int dev, struct iptsd_touch_input in, bool palm)
+static void iptsd_touch_emit_multi(int dev, struct iptsd_touch_input in)
 {
-   if (palm) {
-       iptsd_devices_emit(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
-       iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_X, 0);
-       iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, 0);
-       return;
-   }
-
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TRACKING_ID, in.index);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_X, in.x);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, in.y);
 }

+static void iptsd_touch_emit_multi_empty(int dev)
+{
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_X, 0);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, 0);
+}
+
+static void iptsd_touch_emit_single(int dev, struct iptsd_touch_input in)
+{
+   iptsd_devices_emit(dev, EV_KEY, BTN_TOUCH, 1);
+   iptsd_devices_emit(dev, EV_ABS, ABS_X, in.x);
+   iptsd_devices_emit(dev, EV_ABS, ABS_Y, in.y);
+}
+
+static void iptsd_touch_emit_single_empty(int dev)
+{
+   iptsd_devices_emit(dev, EV_KEY, BTN_TOUCH, 0);
+   iptsd_devices_emit(dev, EV_ABS, ABS_X, 0);
+   iptsd_devices_emit(dev, EV_ABS, ABS_Y, 0);
+}
+
 static int iptsd_touch_handle_heatmap(struct iptsd_context *iptsd,
        struct heatmap *hm)
 {
+   bool found = false;
    bool blocked = false;

    struct iptsd_touch_device *touch = &iptsd->devices.touch;
@@ -42,6 +58,9 @@ static int iptsd_touch_handle_heatmap(struct iptsd_context *iptsd,
            blocked = blocked || touch->processor.inputs[i].is_palm;
    }

+   iptsd_devices_emit(touch->dev, EV_MSC, MSC_TIMESTAMP,
+           iptsd->control.current_doorbell);
+
    for (int i = 0; i < max_contacts; i++) {
        struct iptsd_touch_input in = touch->processor.inputs[i];

@@ -49,9 +68,23 @@ static int iptsd_touch_handle_heatmap(struct iptsd_context *iptsd,
        if (in.index != -1 && !in.is_stable)
            continue;

-       iptsd_touch_emit(touch->dev, in, in.is_palm || blocked);
+       if (in.is_palm || blocked) {
+           iptsd_touch_emit_multi_empty(touch->dev);
+           continue;
+       }
+
+       iptsd_touch_emit_multi(touch->dev, in);
+
+       if (found || in.index == -1)
+           continue;
+
+       found = true;
+       iptsd_touch_emit_single(touch->dev, in);
    }

+   if (!found)
+       iptsd_touch_emit_single_empty(touch->dev);
+
    int ret = iptsd_devices_emit(touch->dev, EV_SYN, SYN_REPORT, 0);
    if (ret < 0)
        iptsd_err(ret, "Failed to emit touch report");

This adds all of the ABS_X stuff, as well as MSC_TIMESTAMP. In the past we had some issues on the stylus (which also sends timestamps), where the cursor in Gnome wouldn't behave properly if we didn't spam the timestamp event. Maybe its similar in this case?

sebanc commented 3 years ago

Thanks a lot for the new patch,

I just tried it and even though touchscreen reactivity feels a bit different (difficult to describe it), the main issue seems to still be present. You will find the new evtest logs and screen-capture below:

ipts_patch_20201122.zip

What's weird is that touch points are correctly represented as white circles and events seem correct but the UI reacts differently.

StollD commented 3 years ago

What I find a bit weird is that the singletouch events get lifted before the multitouch events are.

Event: time 1606082648.567453, -------------- SYN_REPORT ------------
Event: time 1606082648.578343, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP), value 15930
Event: time 1606082648.578343, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 1606082648.578343, type 3 (EV_ABS), code 0 (ABS_X), value 0
Event: time 1606082648.578343, type 3 (EV_ABS), code 1 (ABS_Y), value 0
Event: time 1606082648.578343, -------------- SYN_REPORT ------------
Event: time 1606082648.589355, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP), value 15931
Event: time 1606082648.589355, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 1606082648.589355, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 0
Event: time 1606082648.589355, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 0

Could you maybe try to change this line in touch.c:

if (in.index != -1 && !in.is_stable)
    continue;

to this

if (in.index != -1 && !in.is_stable) {
    found = true;
    continue;
}

I am not sure though if that will change anything. Probably not, but hey, its not like I would have any other ideas :/

sebanc commented 3 years ago

Sure, I will try it and let you know :)

Thanks for the suggestion, any idea is good to take.

sebanc commented 3 years ago

Unfortunately, it did not solve the issue. You will find the logs below so that you can check if your patch had the expected effect.

ipts_patch_20201123.zip

I will try to modify some more things in the driver and see if I can find something which has a noticeable impact on this issue in order to give you more clues.

Thanks a lot for your help already :)

StollD commented 3 years ago

Since that last change changed exactly nothing (singletouch and multitouch events are still lifted in seperate chunks), I looked into this a bit deeper. Turns out, the issue isn't even triggered by the changes we did earlier, they just expose it more visibly. The actual issue is in the code for finger tracking.

diff --git a/src/finger.c b/src/finger.c
index d8f0b65..882d6dd 100644
--- a/src/finger.c
+++ b/src/finger.c
@@ -193,10 +193,16 @@ void iptsd_finger_track(struct iptsd_touch_processor *tp, int count)
     * prevent libinput errors.
     */
    for (int i = 0; i < max_contacts; i++) {
-       if (tp->inputs[i].index != -1) {
-           tp->inputs[i].slot = tp->inputs[i].index;
+       if (tp->inputs[i].index == -1)
+           continue;
+
+       tp->inputs[i].slot = tp->inputs[i].index;
+       tp->free_indices[tp->inputs[i].index] = false;
+   }
+
+   for (int i = 0; i < max_contacts; i++) {
+       if (tp->inputs[i].index != -1)
            continue;
-       }

        for (int k = 0; k < max_contacts; k++) {
            if (!tp->free_indices[k])
diff --git a/src/touch-processing.c b/src/touch-processing.c
index fb0cf5a..a46d1f8 100644
--- a/src/touch-processing.c
+++ b/src/touch-processing.c
@@ -121,8 +121,6 @@ static void iptsd_touch_processing_save(struct iptsd_touch_processor *tp)

        if (tp->inputs[i].index == -1)
            continue;
-
-       tp->free_indices[tp->inputs[i].index] = false;
    }
 }

Although I still don't think it will change anything, could you give that a try?

sebanc commented 3 years ago

Unfortunately it did not fix the issue: ipts_patch_20201124.zip

However, the evtest output indeed seems much closer to the original one so I would still call it progress :)

StollD commented 3 years ago

I guess we should just continue to add stuff that the original driver sends until it magically starts working.

The driver from intel sends the touch coordinates twice, once as ABS_MT_POSITION_* and once as ABS_MT_TOOL_*. So lets do that too.

diff --git a/src/devices.c b/src/devices.c
index 4c0e12d..f69479d 100644
--- a/src/devices.c
+++ b/src/devices.c
@@ -3,6 +3,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/input-event-codes.h>
+#include <linux/input.h>
 #include <linux/uinput.h>
 #include <math.h>
 #include <stdint.h>
@@ -177,6 +178,24 @@ static int iptsd_devices_create_touch(struct iptsd_devices *devices,
    abs_setup.absinfo.resolution = resY;
    iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);

+   abs_setup.code = ABS_MT_TOOL_X;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_X;
+   abs_setup.absinfo.resolution = resX;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
+   abs_setup.code = ABS_MT_TOOL_Y;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_Y;
+   abs_setup.absinfo.resolution = resY;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
+   abs_setup.code = ABS_MT_TOOL_TYPE;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = MT_TOOL_MAX;
+   abs_setup.absinfo.resolution = 0;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
    setup.id.bustype = BUS_VIRTUAL;
    setup.id.vendor = devices->device_info.vendor;
    setup.id.product = devices->device_info.product;
diff --git a/src/touch.c b/src/touch.c
index 8394a66..ed125e6 100644
--- a/src/touch.c
+++ b/src/touch.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later

 #include <linux/input-event-codes.h>
+#include <linux/input.h>
 #include <linux/uinput.h>
 #include <stdint.h>
 #include <string.h>
@@ -20,6 +21,8 @@ static void iptsd_touch_emit_multi(int dev, struct iptsd_touch_input in)
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TRACKING_ID, in.index);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_X, in.x);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, in.y);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_X, in.x);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_Y, in.y);
 }

 static void iptsd_touch_emit_multi_empty(int dev)
@@ -27,6 +30,8 @@ static void iptsd_touch_emit_multi_empty(int dev)
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_X, 0);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, 0);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_X, 0);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_Y, 0);
 }

 static void iptsd_touch_emit_single(int dev, struct iptsd_touch_input in)

If that doesn't work, we could try sending data for the contact area / pressure thats being applied. Since we don't currently calculate that from the heatmap, lets just randomly pick two values from the log you sent earlier and use these.

diff --git a/src/devices.c b/src/devices.c
index f69479d..2f56292 100644
--- a/src/devices.c
+++ b/src/devices.c
@@ -196,6 +196,24 @@ static int iptsd_devices_create_touch(struct iptsd_devices *devices,
    abs_setup.absinfo.resolution = 0;
    iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);

+   abs_setup.code = ABS_MT_TOUCH_MAJOR;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_X;
+   abs_setup.absinfo.resolution = resX;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
+   abs_setup.code = ABS_MT_TOUCH_MINOR;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = IPTS_MAX_Y;
+   abs_setup.absinfo.resolution = resY;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
+   abs_setup.code = ABS_MT_ORIENTATION;
+   abs_setup.absinfo.minimum = 0;
+   abs_setup.absinfo.maximum = 1;
+   abs_setup.absinfo.resolution = 0;
+   iptsd_utils_ioctl(file, UI_ABS_SETUP, &abs_setup);
+
    setup.id.bustype = BUS_VIRTUAL;
    setup.id.vendor = devices->device_info.vendor;
    setup.id.product = devices->device_info.product;
diff --git a/src/touch.c b/src/touch.c
index ed125e6..dddf105 100644
--- a/src/touch.c
+++ b/src/touch.c
@@ -23,6 +23,8 @@ static void iptsd_touch_emit_multi(int dev, struct iptsd_touch_input in)
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, in.y);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_X, in.x);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_Y, in.y);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOUCH_MAJOR, 320);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOUCH_MINOR, 192);
 }

 static void iptsd_touch_emit_multi_empty(int dev)
@@ -32,6 +34,8 @@ static void iptsd_touch_emit_multi_empty(int dev)
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, 0);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_X, 0);
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_Y, 0);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
+   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
 }

 static void iptsd_touch_emit_single(int dev, struct iptsd_touch_input in)

Finally, we could try to not send 0 for the coordinates when lifting a touch (the original driver doesnt do this).

diff --git a/src/touch.c b/src/touch.c
index dddf105..036f6c9 100644
--- a/src/touch.c
+++ b/src/touch.c
@@ -30,12 +30,6 @@ static void iptsd_touch_emit_multi(int dev, struct iptsd_touch_input in)
 static void iptsd_touch_emit_multi_empty(int dev)
 {
    iptsd_devices_emit(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
-   iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_X, 0);
-   iptsd_devices_emit(dev, EV_ABS, ABS_MT_POSITION_Y, 0);
-   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_X, 0);
-   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOOL_Y, 0);
-   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
-   iptsd_devices_emit(dev, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
 }

 static void iptsd_touch_emit_single(int dev, struct iptsd_touch_input in)
@@ -48,8 +42,6 @@ static void iptsd_touch_emit_single(int dev, struct iptsd_touch_input in)
 static void iptsd_touch_emit_single_empty(int dev)
 {
    iptsd_devices_emit(dev, EV_KEY, BTN_TOUCH, 0);
-   iptsd_devices_emit(dev, EV_ABS, ABS_X, 0);
-   iptsd_devices_emit(dev, EV_ABS, ABS_Y, 0);
 }

 static int iptsd_touch_handle_heatmap(struct iptsd_context *iptsd,
@@ -80,7 +72,7 @@ static int iptsd_touch_handle_heatmap(struct iptsd_context *iptsd,
            continue;
        }

-       if (in.is_palm || blocked) {
+       if (in.is_palm || blocked || in.index == -1) {
            iptsd_touch_emit_multi_empty(touch->dev);
            continue;
        }

At this point the events from iptsd should be a complete imitation of the old driver, so if it doesnt work, ChromeOS must be haunted.

sebanc commented 3 years ago

Just letting you know that touch works very well with all patches applied, you really did an incredible job here !

I had previously implemented ABS_MT_TOOL and ABS_MT_TOUCH so I think that the last patch fixed it (or finished fixing it) but I will try different patch combinations and provide detailed results soon.

Thanks a lot for your kind help :)

StollD commented 3 years ago

Just letting you know that touch works very well with all patches applied, you really did an incredible job here !

Nice to hear that!

I had previously implemented ABS_MT_TOOL and ABS_MT_TOUCH so I think that the last patch fixed it (or finished fixing it) but I will try different patch combinations and provide detailed results soon.

Yeah, I would kinda prefer it if the last patch would fix it heh. I really dont want to send fake values (ABS_MT_TOUCH_*), and the ABS_MT_TOOL_* stuff is just duplicated and noisy (although I would be fine with that if it improves compatibility).

Thanks a lot for your kind help :)

You're welcome.

sebanc commented 3 years ago

Following my previous comment, here is the full report:

It seems ABS_MT_TOOL and ABS_MT_TOUCH are indeed not necessary as I am currently using the below patch (the patches in your first comments + only the 3rd part of your last patch) and it works very well: chromeos_iptsd.txt

There is still a minor issue, the app drawer opens itself from time to time when touching icons on the shelf but the touchscreen is nevertheless totally usable. In case you might have an idea regarding this, below is an evtest log with 3 touches and where the app drawer opened on the 3rd touch: app_drawer_evtest.txt

In any case, it is just a minor bug and the touchscreen is now fully usable.

Thanks again for all the work you put into this :)

StollD commented 3 years ago

It seems ABS_MT_TOOL and ABS_MT_TOUCH are indeed not necessary as I am currently using the below patch (the patches in your first comments + only the 3rd part of your last patch) and it works very well:

Thanks for checking! I updated the changes a bit and pushed them to master. I decided to include the ABS_MT_TOOL_* events, because thats what the old driver (or linux HID subsystem) do, so it can't hurt to have it. Also, I didn't include the MSC_TIMESTAMP code because I would like to avoid spamming evtest with these values. Could you check if that makes a difference on ChromeOS?

There is still a minor issue, the app drawer opens itself from time to time when touching icons on the shelf but the touchscreen is nevertheless totally usable.

Hmm, does ChromeOS support gestures? It makes a bit of a jump there, but I am not sure what could be causing it. Maybe some kind of noise in the heatmap data (some surfaces have that from time to time, but its much more extreme usually).

sebanc commented 3 years ago

Just tried the master branch with the new commits and it works very well :) (with the same minor issue)

I think ChromeOS does not have very advanced gestures: https://support.google.com/chromebook/answer/2766492?hl=en

I close this issue for now and I will update it if I find something for the minor issue.

sebanc commented 3 years ago

Hi @StollD

Sorry to comment on this closed issue but would you mind adding a "service_manager=none" meson build option as ChromeOS does not have the necessary pkgconfig and uses custom init scripts ?

StollD commented 3 years ago

meson build -Dservice_manager="" works fine for me to disable any init scripts. If its possible, we can also add the scripts for ChromeOS as a new service manager option.

sebanc commented 3 years ago

Thanks a lot for your quick answer :) Sorry I did not know that I could leave the variable empty... It works perfectly !

I don't think you really need to add a new service_manager option as currently chrome / chromium os distros do not include a toolchain which allows building iptsd from source.

Nevertheless, in case someone might need an ipts bootscript for chromeos, I create "/etc/init/ipts.conf" with the below content:

start on stopped udev-trigger

script
    insmod /lib/modules/$(cat /proc/version |  cut -d' ' -f3)/ipts.ko
    iptsd
end script