| Message ID | 20220520084514.3451193-1-svv@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2a96271fb66c499e4a89d76a89d3d01170c10bef |
| Headers | show |
| Series | Document the units for resolution of size axes | expand |
Hi Siarhei, On Fri, May 20, 2022 at 01:45:14AM -0700, Siarhei Vishniakou wrote: > Today, the resolution of size axes is not documented. As a result, it's > not clear what the canonical interpretation of this value should be. On > Android, there is a need to calculate the size of the touch ellipse in > physical units (millimeters). > > After reviewing linux source, it turned out that most of the existing > usages are already interpreting this value as "units/mm". This > documentation will make it explicit. This will help device > implementations with correctly following the linux specs, and will > ensure that the devices will work on Android without needing further > customized parameters for scaling of major/minor values. > > Signed-off-by: Siarhei Vishniakou <svv@google.com> > Change-Id: I4a2de9e6d02e5fd707e5d312f5c3325734266a6e > --- > include/uapi/linux/input.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h > index ee3127461ee0..328cf545c029 100644 > --- a/include/uapi/linux/input.h > +++ b/include/uapi/linux/input.h > @@ -78,10 +78,13 @@ struct input_id { > * Note that input core does not clamp reported values to the > * [minimum, maximum] limits, such task is left to userspace. > * > - * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z) > - * is reported in units per millimeter (units/mm), resolution > - * for rotational axes (ABS_RX, ABS_RY, ABS_RZ) is reported > - * in units per radian. > + * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z, > + * ABS_MT_POSITION_X, ABS_MT_POSITION_Y) is reported in units > + * per millimeter (units/mm), resolution for rotational axes > + * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian. > + * The resolution for the size axes (ABS_MT_TOUCH_MAJOR, > + * ABS_MT_TOUCH_MINOR, ABS_MT_WIDTH_MAJOR, ABS_MT_WIDTH_MINOR) > + * is reported in units per millimeter (units/mm). > * When INPUT_PROP_ACCELEROMETER is set the resolution changes. > * The main axes (ABS_X, ABS_Y, ABS_Z) are then reported in > * units per g (units/g) and in units per degree per second > -- > 2.36.1.124.g0e6072fb45-goog > Thanks for raising this point; it's a valid one. However, I'm not convinced this is the right approach. On all the controllers I've worked on, ABS_X and ABS_Y are mapped to arbitrary resolution values that don't necessarily map to real- world units. I don't think we can make any assumption at the input layer as to the physical size of the touch surface. It is the same problem for ABS_MT_PRESSURE; the values are typically controller-specific and we can't reasonably try to map this axis to any standard unit (e.g. Pascals). If user space needs to understand the mapping between axis range and physical size, maybe it is better to adopt the approach from the IIO subsystem wherein the input_dev offers a property that maps each axis (i.e. "raw" value) to some SI unit? In that case, dts could define the scaling factor between raw values and physical dimensions. At any rate, that is just my $.02. Kind regards, Jeff LaBundy
Hi Jeff, I assume you are talking about touch IC controllers and not gamepad controllers. The units for ABS_X and ABS_Y are already documented. The proposal here is to expand the documentation to include the ABS_MT_TOUCH_MAJOR and ABS_MT_TOUCH_MINOR axes in the definition. We can't fix existing devices, but the documentation would specify the "correct" behaviour going forward. Based on my understanding of hidinput_calc_abs_res function, the units for the resolution of major/minor are already "units/mm". This behaviour is already the default in linux, just not documented. If you were to develop a HID touchscreen, you would already get this behaviour today. I don't think the problem is the same as for ABS_MT_PRESSURE. In the case of pressure, the value comes from arbitrary algorithms using capacitive data to guess "pressure" rather than using physical pressure sensors. So most touch ICs simply can't report pressure. I think ABS_MT_PRESSURE in general shouldn't even be reported, partially because of the lack of appropriate sensors, and partially because of the lack of explanation of what these values means (no resolution). For pressure, the userspace doesn't know what to do with it, and has to guess about what it means. As a result, on Android for example, we use pressure in an "on-off" fashion - zero pressure means hover and non-zero pressure means contact. Arguably, that's misusing the APIs. In the case of major and minor axes, it's very clear how they should be used. There's a well-defined way to calibrate these values to physical units. You can put an oval object on the screen, calculate its dimensions using a caliper, and then look at the size of the oval in the capacitive data.. So the devices *can* provide an accurate value here if they wanted. Do you mind explaning a bit more about how your proposal would work? Would the user space have to scrape the linux folders in order to find this new property that we would define to report the resolution, rather than using ioctls to read the existing value from the fd of /dev/input/eventX? With the current approach, my expectation is that the touch driver could certainly use the dts to read out some values like screen size, scale factor, etc., but it would then be responsible to set the resolutions accordingly and to scale these values as needed when reporting to user space. On Fri, May 20, 2022 at 9:00 AM Jeff LaBundy <jeff@labundy.com> wrote: > > Hi Siarhei, > > On Fri, May 20, 2022 at 01:45:14AM -0700, Siarhei Vishniakou wrote: > > Today, the resolution of size axes is not documented. As a result, it's > > not clear what the canonical interpretation of this value should be. On > > Android, there is a need to calculate the size of the touch ellipse in > > physical units (millimeters). > > > > After reviewing linux source, it turned out that most of the existing > > usages are already interpreting this value as "units/mm". This > > documentation will make it explicit. This will help device > > implementations with correctly following the linux specs, and will > > ensure that the devices will work on Android without needing further > > customized parameters for scaling of major/minor values. > > > > Signed-off-by: Siarhei Vishniakou <svv@google.com> > > Change-Id: I4a2de9e6d02e5fd707e5d312f5c3325734266a6e > > --- > > include/uapi/linux/input.h | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h > > index ee3127461ee0..328cf545c029 100644 > > --- a/include/uapi/linux/input.h > > +++ b/include/uapi/linux/input.h > > @@ -78,10 +78,13 @@ struct input_id { > > * Note that input core does not clamp reported values to the > > * [minimum, maximum] limits, such task is left to userspace. > > * > > - * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z) > > - * is reported in units per millimeter (units/mm), resolution > > - * for rotational axes (ABS_RX, ABS_RY, ABS_RZ) is reported > > - * in units per radian. > > + * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z, > > + * ABS_MT_POSITION_X, ABS_MT_POSITION_Y) is reported in units > > + * per millimeter (units/mm), resolution for rotational axes > > + * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian. > > + * The resolution for the size axes (ABS_MT_TOUCH_MAJOR, > > + * ABS_MT_TOUCH_MINOR, ABS_MT_WIDTH_MAJOR, ABS_MT_WIDTH_MINOR) > > + * is reported in units per millimeter (units/mm). > > * When INPUT_PROP_ACCELEROMETER is set the resolution changes. > > * The main axes (ABS_X, ABS_Y, ABS_Z) are then reported in > > * units per g (units/g) and in units per degree per second > > -- > > 2.36.1.124.g0e6072fb45-goog > > > > Thanks for raising this point; it's a valid one. However, I'm not > convinced this is the right approach. > > On all the controllers I've worked on, ABS_X and ABS_Y are mapped > to arbitrary resolution values that don't necessarily map to real- > world units. I don't think we can make any assumption at the input > layer as to the physical size of the touch surface. > > It is the same problem for ABS_MT_PRESSURE; the values are typically > controller-specific and we can't reasonably try to map this axis to > any standard unit (e.g. Pascals). > > If user space needs to understand the mapping between axis range and > physical size, maybe it is better to adopt the approach from the IIO > subsystem wherein the input_dev offers a property that maps each axis > (i.e. "raw" value) to some SI unit? > > In that case, dts could define the scaling factor between raw values > and physical dimensions. At any rate, that is just my $.02. > > Kind regards, > Jeff LaBundy
Hi Siarhei, Apologies for the delayed response. On Tue, May 24, 2022 at 09:53:22AM -0700, Siarhei Vishniakou wrote: > Hi Jeff, > > I assume you are talking about touch IC controllers and not gamepad controllers. > The units for ABS_X and ABS_Y are already documented. The proposal > here is to expand the documentation to include the ABS_MT_TOUCH_MAJOR > and ABS_MT_TOUCH_MINOR axes in the definition. I meant any multitouch controller really. Good point about existing single-contact axes though. > > We can't fix existing devices, but the documentation would specify the > "correct" behaviour going forward. Based on my understanding of > hidinput_calc_abs_res function, the units for the resolution of > major/minor are already "units/mm". > This behaviour is already the default in linux, just not documented. > If you were to develop a HID touchscreen, you would already get this > behaviour today. ACK. > > I don't think the problem is the same as for ABS_MT_PRESSURE. In the > case of pressure, the value comes from arbitrary algorithms using > capacitive data to guess "pressure" rather than using physical > pressure sensors. > So most touch ICs simply can't report pressure. I think > ABS_MT_PRESSURE in general shouldn't even be reported, partially > because of the lack of appropriate sensors, and partially because of > the lack of explanation of what these values means (no resolution). > For pressure, the userspace doesn't know what to do with it, and has > to guess about what it means. As a result, on Android for example, we > use pressure in an "on-off" fashion - zero pressure means hover and > non-zero pressure means contact. Arguably, that's misusing the APIs. What I was trying to argue was whether or not it makes sense to define any axes at all, if some axes can't possibly be defined (i.e., all or nothing). However, you're absolutely correct that the problem statement doesn't apply to ABS_MT_PRESSURE because we do not generally use touch surfaces to report granular pressure. In fact, most controllers I'm aware of that do report a pressure value are actually just reporting area anyway. > > In the case of major and minor axes, it's very clear how they should > be used. There's a well-defined way to calibrate these values to > physical units. You can put an oval object on the screen, calculate > its dimensions using a caliper, and then look at the size of the oval > in the capacitive data.. So the devices *can* provide an accurate > value here if they wanted. Agreed. > > Do you mind explaning a bit more about how your proposal would work? > Would the user space have to scrape the linux folders in order to find > this new property that we would define to report the resolution, > rather than using ioctls to read the existing value from the fd of > /dev/input/eventX? Yes, that's what I had in mind, but it's admittedly overkill here. > > With the current approach, my expectation is that the touch driver > could certainly use the dts to read out some values like screen size, > scale factor, etc., but it would then be responsible to set the > resolutions accordingly and to scale these values as needed when > reporting to user space. Yup, looks like your idea is the best of both worlds. In fact I see we already have touchscreen-x-mm and touchscreen-y-mm defined in common touchscreen bindings, albeit not many drivers are using them yet. For what it's worth, as a fellow customer of input: Reviewed-by: Jeff LaBundy <jeff@labundy.com> Thanks for the great discussion! > > > On Fri, May 20, 2022 at 9:00 AM Jeff LaBundy <jeff@labundy.com> wrote: > > > > Hi Siarhei, > > > > On Fri, May 20, 2022 at 01:45:14AM -0700, Siarhei Vishniakou wrote: > > > Today, the resolution of size axes is not documented. As a result, it's > > > not clear what the canonical interpretation of this value should be. On > > > Android, there is a need to calculate the size of the touch ellipse in > > > physical units (millimeters). > > > > > > After reviewing linux source, it turned out that most of the existing > > > usages are already interpreting this value as "units/mm". This > > > documentation will make it explicit. This will help device > > > implementations with correctly following the linux specs, and will > > > ensure that the devices will work on Android without needing further > > > customized parameters for scaling of major/minor values. > > > > > > Signed-off-by: Siarhei Vishniakou <svv@google.com> > > > Change-Id: I4a2de9e6d02e5fd707e5d312f5c3325734266a6e > > > --- > > > include/uapi/linux/input.h | 11 +++++++---- > > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h > > > index ee3127461ee0..328cf545c029 100644 > > > --- a/include/uapi/linux/input.h > > > +++ b/include/uapi/linux/input.h > > > @@ -78,10 +78,13 @@ struct input_id { > > > * Note that input core does not clamp reported values to the > > > * [minimum, maximum] limits, such task is left to userspace. > > > * > > > - * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z) > > > - * is reported in units per millimeter (units/mm), resolution > > > - * for rotational axes (ABS_RX, ABS_RY, ABS_RZ) is reported > > > - * in units per radian. > > > + * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z, > > > + * ABS_MT_POSITION_X, ABS_MT_POSITION_Y) is reported in units > > > + * per millimeter (units/mm), resolution for rotational axes > > > + * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian. > > > + * The resolution for the size axes (ABS_MT_TOUCH_MAJOR, > > > + * ABS_MT_TOUCH_MINOR, ABS_MT_WIDTH_MAJOR, ABS_MT_WIDTH_MINOR) > > > + * is reported in units per millimeter (units/mm). > > > * When INPUT_PROP_ACCELEROMETER is set the resolution changes. > > > * The main axes (ABS_X, ABS_Y, ABS_Z) are then reported in > > > * units per g (units/g) and in units per degree per second > > > -- > > > 2.36.1.124.g0e6072fb45-goog > > > > > > > Thanks for raising this point; it's a valid one. However, I'm not > > convinced this is the right approach. > > > > On all the controllers I've worked on, ABS_X and ABS_Y are mapped > > to arbitrary resolution values that don't necessarily map to real- > > world units. I don't think we can make any assumption at the input > > layer as to the physical size of the touch surface. > > > > It is the same problem for ABS_MT_PRESSURE; the values are typically > > controller-specific and we can't reasonably try to map this axis to > > any standard unit (e.g. Pascals). > > > > If user space needs to understand the mapping between axis range and > > physical size, maybe it is better to adopt the approach from the IIO > > subsystem wherein the input_dev offers a property that maps each axis > > (i.e. "raw" value) to some SI unit? > > > > In that case, dts could define the scaling factor between raw values > > and physical dimensions. At any rate, that is just my $.02. > > > > Kind regards, > > Jeff LaBundy Kind regards, Jeff LaBundy
Thanks Jeff for the feedback. Bumping this back up. Maintainers - could you please have a look? On Fri, May 27, 2022 at 3:36 PM Jeff LaBundy <jeff@labundy.com> wrote: > > Hi Siarhei, > > Apologies for the delayed response. > > On Tue, May 24, 2022 at 09:53:22AM -0700, Siarhei Vishniakou wrote: > > Hi Jeff, > > > > I assume you are talking about touch IC controllers and not gamepad controllers. > > The units for ABS_X and ABS_Y are already documented. The proposal > > here is to expand the documentation to include the ABS_MT_TOUCH_MAJOR > > and ABS_MT_TOUCH_MINOR axes in the definition. > > I meant any multitouch controller really. Good point about existing > single-contact axes though. > > > > > We can't fix existing devices, but the documentation would specify the > > "correct" behaviour going forward. Based on my understanding of > > hidinput_calc_abs_res function, the units for the resolution of > > major/minor are already "units/mm". > > This behaviour is already the default in linux, just not documented. > > If you were to develop a HID touchscreen, you would already get this > > behaviour today. > > ACK. > > > > > I don't think the problem is the same as for ABS_MT_PRESSURE. In the > > case of pressure, the value comes from arbitrary algorithms using > > capacitive data to guess "pressure" rather than using physical > > pressure sensors. > > So most touch ICs simply can't report pressure. I think > > ABS_MT_PRESSURE in general shouldn't even be reported, partially > > because of the lack of appropriate sensors, and partially because of > > the lack of explanation of what these values means (no resolution). > > For pressure, the userspace doesn't know what to do with it, and has > > to guess about what it means. As a result, on Android for example, we > > use pressure in an "on-off" fashion - zero pressure means hover and > > non-zero pressure means contact. Arguably, that's misusing the APIs. > > What I was trying to argue was whether or not it makes sense to define > any axes at all, if some axes can't possibly be defined (i.e., all or > nothing). > > However, you're absolutely correct that the problem statement doesn't > apply to ABS_MT_PRESSURE because we do not generally use touch surfaces > to report granular pressure. In fact, most controllers I'm aware of that > do report a pressure value are actually just reporting area anyway. > > > > > In the case of major and minor axes, it's very clear how they should > > be used. There's a well-defined way to calibrate these values to > > physical units. You can put an oval object on the screen, calculate > > its dimensions using a caliper, and then look at the size of the oval > > in the capacitive data.. So the devices *can* provide an accurate > > value here if they wanted. > > Agreed. > > > > > Do you mind explaning a bit more about how your proposal would work? > > Would the user space have to scrape the linux folders in order to find > > this new property that we would define to report the resolution, > > rather than using ioctls to read the existing value from the fd of > > /dev/input/eventX? > > Yes, that's what I had in mind, but it's admittedly overkill here. > > > > > With the current approach, my expectation is that the touch driver > > could certainly use the dts to read out some values like screen size, > > scale factor, etc., but it would then be responsible to set the > > resolutions accordingly and to scale these values as needed when > > reporting to user space. > > Yup, looks like your idea is the best of both worlds. In fact I see we > already have touchscreen-x-mm and touchscreen-y-mm defined in common > touchscreen bindings, albeit not many drivers are using them yet. > > For what it's worth, as a fellow customer of input: > > Reviewed-by: Jeff LaBundy <jeff@labundy.com> > > Thanks for the great discussion! > > > > > > > On Fri, May 20, 2022 at 9:00 AM Jeff LaBundy <jeff@labundy.com> wrote: > > > > > > Hi Siarhei, > > > > > > On Fri, May 20, 2022 at 01:45:14AM -0700, Siarhei Vishniakou wrote: > > > > Today, the resolution of size axes is not documented. As a result, it's > > > > not clear what the canonical interpretation of this value should be. On > > > > Android, there is a need to calculate the size of the touch ellipse in > > > > physical units (millimeters). > > > > > > > > After reviewing linux source, it turned out that most of the existing > > > > usages are already interpreting this value as "units/mm". This > > > > documentation will make it explicit. This will help device > > > > implementations with correctly following the linux specs, and will > > > > ensure that the devices will work on Android without needing further > > > > customized parameters for scaling of major/minor values. > > > > > > > > Signed-off-by: Siarhei Vishniakou <svv@google.com> > > > > Change-Id: I4a2de9e6d02e5fd707e5d312f5c3325734266a6e > > > > --- > > > > include/uapi/linux/input.h | 11 +++++++---- > > > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h > > > > index ee3127461ee0..328cf545c029 100644 > > > > --- a/include/uapi/linux/input.h > > > > +++ b/include/uapi/linux/input.h > > > > @@ -78,10 +78,13 @@ struct input_id { > > > > * Note that input core does not clamp reported values to the > > > > * [minimum, maximum] limits, such task is left to userspace. > > > > * > > > > - * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z) > > > > - * is reported in units per millimeter (units/mm), resolution > > > > - * for rotational axes (ABS_RX, ABS_RY, ABS_RZ) is reported > > > > - * in units per radian. > > > > + * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z, > > > > + * ABS_MT_POSITION_X, ABS_MT_POSITION_Y) is reported in units > > > > + * per millimeter (units/mm), resolution for rotational axes > > > > + * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian. > > > > + * The resolution for the size axes (ABS_MT_TOUCH_MAJOR, > > > > + * ABS_MT_TOUCH_MINOR, ABS_MT_WIDTH_MAJOR, ABS_MT_WIDTH_MINOR) > > > > + * is reported in units per millimeter (units/mm). > > > > * When INPUT_PROP_ACCELEROMETER is set the resolution changes. > > > > * The main axes (ABS_X, ABS_Y, ABS_Z) are then reported in > > > > * units per g (units/g) and in units per degree per second > > > > -- > > > > 2.36.1.124.g0e6072fb45-goog > > > > > > > > > > Thanks for raising this point; it's a valid one. However, I'm not > > > convinced this is the right approach. > > > > > > On all the controllers I've worked on, ABS_X and ABS_Y are mapped > > > to arbitrary resolution values that don't necessarily map to real- > > > world units. I don't think we can make any assumption at the input > > > layer as to the physical size of the touch surface. > > > > > > It is the same problem for ABS_MT_PRESSURE; the values are typically > > > controller-specific and we can't reasonably try to map this axis to > > > any standard unit (e.g. Pascals). > > > > > > If user space needs to understand the mapping between axis range and > > > physical size, maybe it is better to adopt the approach from the IIO > > > subsystem wherein the input_dev offers a property that maps each axis > > > (i.e. "raw" value) to some SI unit? > > > > > > In that case, dts could define the scaling factor between raw values > > > and physical dimensions. At any rate, that is just my $.02. > > > > > > Kind regards, > > > Jeff LaBundy > > Kind regards, > Jeff LaBundy
On Fri, May 20, 2022 at 01:45:14AM -0700, Siarhei Vishniakou wrote: > Today, the resolution of size axes is not documented. As a result, it's > not clear what the canonical interpretation of this value should be. On > Android, there is a need to calculate the size of the touch ellipse in > physical units (millimeters). > > After reviewing linux source, it turned out that most of the existing > usages are already interpreting this value as "units/mm". This > documentation will make it explicit. This will help device > implementations with correctly following the linux specs, and will > ensure that the devices will work on Android without needing further > customized parameters for scaling of major/minor values. > > Signed-off-by: Siarhei Vishniakou <svv@google.com> > Change-Id: I4a2de9e6d02e5fd707e5d312f5c3325734266a6e Applied, thank you.
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index ee3127461ee0..328cf545c029 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -78,10 +78,13 @@ struct input_id { * Note that input core does not clamp reported values to the * [minimum, maximum] limits, such task is left to userspace. * - * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z) - * is reported in units per millimeter (units/mm), resolution - * for rotational axes (ABS_RX, ABS_RY, ABS_RZ) is reported - * in units per radian. + * The default resolution for main axes (ABS_X, ABS_Y, ABS_Z, + * ABS_MT_POSITION_X, ABS_MT_POSITION_Y) is reported in units + * per millimeter (units/mm), resolution for rotational axes + * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian. + * The resolution for the size axes (ABS_MT_TOUCH_MAJOR, + * ABS_MT_TOUCH_MINOR, ABS_MT_WIDTH_MAJOR, ABS_MT_WIDTH_MINOR) + * is reported in units per millimeter (units/mm). * When INPUT_PROP_ACCELEROMETER is set the resolution changes. * The main axes (ABS_X, ABS_Y, ABS_Z) are then reported in * units per g (units/g) and in units per degree per second
Today, the resolution of size axes is not documented. As a result, it's not clear what the canonical interpretation of this value should be. On Android, there is a need to calculate the size of the touch ellipse in physical units (millimeters). After reviewing linux source, it turned out that most of the existing usages are already interpreting this value as "units/mm". This documentation will make it explicit. This will help device implementations with correctly following the linux specs, and will ensure that the devices will work on Android without needing further customized parameters for scaling of major/minor values. Signed-off-by: Siarhei Vishniakou <svv@google.com> Change-Id: I4a2de9e6d02e5fd707e5d312f5c3325734266a6e --- include/uapi/linux/input.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)