Re: [Linuxwacom-devel] [PATCH 1/2] Increase full-scale pressure range from 0..2047 to 0..65535

[Ping Cheng]

On Thursday, October 27, 2016, Peter Hutterer 
wrote:

> On Wed, Oct 26, 2016 at 11:56:57PM -0700, Ping Cheng wrote:
> > On Wednesday, October 26, 2016, Peter Hutterer  >
> > wrote:
> >
> > > On Wed, Oct 26, 2016 at 06:24:48PM -0700, Ping Cheng wrote:
> > > > On Fri, Oct 7, 2016 at 10:25 AM, Jason Gerecke  
> > > > wrote:
> > > > > The driver has historically normalized the pressure range of all
> kernel
> > > > > devices to 0..2047 rather than using their native range to keep
> things
> > > > > like the application of the pressure curve simple. Pens that report
> > > more
> > > > > than 2048 pressure levels are also normalized down to this range
> > > though,
> > > > > reducing their precision. In order to accomodate the new 8K pen
> (and
> > > any
> > > > > future pens with even higher precision), this patch bumps up the
> full-
> > > > > scale range to be 0..65535. This number was chosen both because it
> far
> > > > > exceeds anything currently known about, and also because it
> matches the
> > > > > normalization range used over the wire by the Wayland tablet
> protocol.
> > > > >
> > > > > Note that the WACOM_PROP_PRESSURE_THRESHOLD value has been tied to
> the
> > > > > normalized (2048-level) pressure range for some time, meaning that
> we
> > > > > cannot simply change the range without causing a change in the
> > > perceived
> > > > > threshold for users. To ensure compatibility, the value is
> interpreted
> > > > > as a fraction of 2048 and then scaled to the actual normalization
> > > range.
> > > > >
> > > > > Signed-off-by: Jason Gerecke   >
> > > >
> > > > Reviewed-by: Ping Cheng 
> > for the set of
> > > 2 patches.
> > > >
> > > > Thank you for considering backward compatibility. That's a bonus for
> > > > many system admins who use xsetwacom/xinput to configure the driver.
> > > >
> > > > Please update this patch as Peter suggested though.
> > >
> > > uhm, just something else I noticed. we precalculate the pressure
> curve, so
> > > we now have 256 KB in the driver allocated just for the values. We
> should
> > > probably split this out to be allocated on demand
> >
> >
> > How do you plan to make it on demand? Will it add a new API?
>
> Changing pPressCurve[FILTER_PRESSURE_RES + 1] to a dynamically allocated
> memory should be enough, no new API is needed, this is just an
> implementation detail.


Great! That's smart. Then go for it ;).

Cheers,

Ping
--
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Re: [Linuxwacom-devel] [PATCH 1/2] Increase full-scale pressure range from 0..2047 to 0..65535

[Peter Hutterer]

On Wed, Oct 26, 2016 at 06:24:48PM -0700, Ping Cheng wrote:
> On Fri, Oct 7, 2016 at 10:25 AM, Jason Gerecke  wrote:
> > The driver has historically normalized the pressure range of all kernel
> > devices to 0..2047 rather than using their native range to keep things
> > like the application of the pressure curve simple. Pens that report more
> > than 2048 pressure levels are also normalized down to this range though,
> > reducing their precision. In order to accomodate the new 8K pen (and any
> > future pens with even higher precision), this patch bumps up the full-
> > scale range to be 0..65535. This number was chosen both because it far
> > exceeds anything currently known about, and also because it matches the
> > normalization range used over the wire by the Wayland tablet protocol.
> >
> > Note that the WACOM_PROP_PRESSURE_THRESHOLD value has been tied to the
> > normalized (2048-level) pressure range for some time, meaning that we
> > cannot simply change the range without causing a change in the perceived
> > threshold for users. To ensure compatibility, the value is interpreted
> > as a fraction of 2048 and then scaled to the actual normalization range.
> >
> > Signed-off-by: Jason Gerecke 
> 
> Reviewed-by: Ping Cheng  for the set of 2 patches.
> 
> Thank you for considering backward compatibility. That's a bonus for
> many system admins who use xsetwacom/xinput to configure the driver.
> 
> Please update this patch as Peter suggested though.

uhm, just something else I noticed. we precalculate the pressure curve, so
we now have 256 KB in the driver allocated just for the values. We should
probably split this out to be allocated on demand (not everyone needs the
pressure curve) and to avoid blowing up our struct sizes even more.

Or, event better would be to have the pressure curve calculated based on the
device's actual min/max, so we only carry around as much as we need.

Cheers,
   Peter
 
> >  src/wcmXCommand.c   | 6 +-
> >  src/xf86WacomDefs.h | 2 +-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> > index 02278ba..403bc84 100644
> > --- a/src/wcmXCommand.c
> > +++ b/src/wcmXCommand.c
> > @@ -256,6 +256,7 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
> > }
> >
> > values[0] = (!common->wcmMaxZ) ? 0 : common->wcmThreshold;
> > +   values[0] /= (FILTER_PRESSURE_RES / 2048); /* backwards 
> > compatibility */
> > prop_threshold = InitWcmAtom(pInfo->dev, 
> > WACOM_PROP_PRESSURE_THRESHOLD, XA_INTEGER, 32, 1, values);
> >
> > values[0] = common->wcmSuppress;
> > @@ -827,6 +828,7 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
> > XIPropertyValuePtr prop,
> > common->wcmCursorProxoutDist = value;
> > } else if (property == prop_threshold)
> > {
> > +   const INT32 MAXIMUM = 2048; /* backwards compatibility */
> > INT32 value;
> >
> > if (prop->size != 1 || prop->format != 32)
> > @@ -836,8 +838,10 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
> > XIPropertyValuePtr prop,
> >
> > if (value == -1)
> > value = DEFAULT_THRESHOLD;
> > -   else if ((value < 1) || (value > FILTER_PRESSURE_RES))
> > +   else if ((value < 1) || (value > MAXIMUM))
> > return BadValue;
> > +   else
> > +   value *= (FILTER_PRESSURE_RES / MAXIMUM);
> >
> > if (!checkonly)
> > common->wcmThreshold = value;
> > diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> > index 1575960..9de9cab 100644
> > --- a/src/xf86WacomDefs.h
> > +++ b/src/xf86WacomDefs.h
> > @@ -183,7 +183,7 @@ struct _WacomModel
> >
> >  #define IsUSBDevice(common) ((common)->wcmDevCls == )
> >
> > -#define FILTER_PRESSURE_RES2048/* maximum points in pressure curve 
> > */
> > +#define FILTER_PRESSURE_RES65536   /* maximum points in pressure curve 
> > */
> >  /* Tested result for setting the pressure threshold to a reasonable value 
> > */
> >  #define THRESHOLD_TOLERANCE (FILTER_PRESSURE_RES / 125)
> >  #define DEFAULT_THRESHOLD (FILTER_PRESSURE_RES / 75)
> > --
> > 2.10.0
> >

--
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Re: [Linuxwacom-devel] [PATCH 1/2] Increase full-scale pressure range from 0..2047 to 0..65535

[Ping Cheng]

On Fri, Oct 7, 2016 at 10:25 AM, Jason Gerecke  wrote:
> The driver has historically normalized the pressure range of all kernel
> devices to 0..2047 rather than using their native range to keep things
> like the application of the pressure curve simple. Pens that report more
> than 2048 pressure levels are also normalized down to this range though,
> reducing their precision. In order to accomodate the new 8K pen (and any
> future pens with even higher precision), this patch bumps up the full-
> scale range to be 0..65535. This number was chosen both because it far
> exceeds anything currently known about, and also because it matches the
> normalization range used over the wire by the Wayland tablet protocol.
>
> Note that the WACOM_PROP_PRESSURE_THRESHOLD value has been tied to the
> normalized (2048-level) pressure range for some time, meaning that we
> cannot simply change the range without causing a change in the perceived
> threshold for users. To ensure compatibility, the value is interpreted
> as a fraction of 2048 and then scaled to the actual normalization range.
>
> Signed-off-by: Jason Gerecke 

Reviewed-by: Ping Cheng  for the set of 2 patches.

Thank you for considering backward compatibility. That's a bonus for
many system admins who use xsetwacom/xinput to configure the driver.

Please update this patch as Peter suggested though.

Ping

> ---
>  src/wcmXCommand.c   | 6 +-
>  src/xf86WacomDefs.h | 2 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> index 02278ba..403bc84 100644
> --- a/src/wcmXCommand.c
> +++ b/src/wcmXCommand.c
> @@ -256,6 +256,7 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
> }
>
> values[0] = (!common->wcmMaxZ) ? 0 : common->wcmThreshold;
> +   values[0] /= (FILTER_PRESSURE_RES / 2048); /* backwards compatibility 
> */
> prop_threshold = InitWcmAtom(pInfo->dev, 
> WACOM_PROP_PRESSURE_THRESHOLD, XA_INTEGER, 32, 1, values);
>
> values[0] = common->wcmSuppress;
> @@ -827,6 +828,7 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
> XIPropertyValuePtr prop,
> common->wcmCursorProxoutDist = value;
> } else if (property == prop_threshold)
> {
> +   const INT32 MAXIMUM = 2048; /* backwards compatibility */
> INT32 value;
>
> if (prop->size != 1 || prop->format != 32)
> @@ -836,8 +838,10 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
> XIPropertyValuePtr prop,
>
> if (value == -1)
> value = DEFAULT_THRESHOLD;
> -   else if ((value < 1) || (value > FILTER_PRESSURE_RES))
> +   else if ((value < 1) || (value > MAXIMUM))
> return BadValue;
> +   else
> +   value *= (FILTER_PRESSURE_RES / MAXIMUM);
>
> if (!checkonly)
> common->wcmThreshold = value;
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index 1575960..9de9cab 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -183,7 +183,7 @@ struct _WacomModel
>
>  #define IsUSBDevice(common) ((common)->wcmDevCls == )
>
> -#define FILTER_PRESSURE_RES2048/* maximum points in pressure curve */
> +#define FILTER_PRESSURE_RES65536   /* maximum points in pressure curve */
>  /* Tested result for setting the pressure threshold to a reasonable value */
>  #define THRESHOLD_TOLERANCE (FILTER_PRESSURE_RES / 125)
>  #define DEFAULT_THRESHOLD (FILTER_PRESSURE_RES / 75)
> --
> 2.10.0
>

--
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Re: [Linuxwacom-devel] [PATCH 1/2] Increase full-scale pressure range from 0..2047 to 0..65535

[Peter Hutterer]

Sorry about the delay, only been back from holidays since mid last week.

On Fri, Oct 07, 2016 at 10:25:39AM -0700, Jason Gerecke wrote:
> The driver has historically normalized the pressure range of all kernel
> devices to 0..2047 rather than using their native range to keep things
> like the application of the pressure curve simple. Pens that report more
> than 2048 pressure levels are also normalized down to this range though,
> reducing their precision. In order to accomodate the new 8K pen (and any
> future pens with even higher precision), this patch bumps up the full-
> scale range to be 0..65535. This number was chosen both because it far
> exceeds anything currently known about, and also because it matches the
> normalization range used over the wire by the Wayland tablet protocol.
> 
> Note that the WACOM_PROP_PRESSURE_THRESHOLD value has been tied to the
> normalized (2048-level) pressure range for some time, meaning that we
> cannot simply change the range without causing a change in the perceived
> threshold for users. To ensure compatibility, the value is interpreted
> as a fraction of 2048 and then scaled to the actual normalization range.
> 
> Signed-off-by: Jason Gerecke 
> ---
>  src/wcmXCommand.c   | 6 +-
>  src/xf86WacomDefs.h | 2 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> index 02278ba..403bc84 100644
> --- a/src/wcmXCommand.c
> +++ b/src/wcmXCommand.c
> @@ -256,6 +256,7 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
>   }
>  
>   values[0] = (!common->wcmMaxZ) ? 0 : common->wcmThreshold;
> + values[0] /= (FILTER_PRESSURE_RES / 2048); /* backwards compatibility */

Add a pair of inline helper functions, that way you can have the 2048 in
only one place and you can add the documentation in one place too.

Cheers,
   Peter

>   prop_threshold = InitWcmAtom(pInfo->dev, WACOM_PROP_PRESSURE_THRESHOLD, 
> XA_INTEGER, 32, 1, values);
>  
>   values[0] = common->wcmSuppress;
> @@ -827,6 +828,7 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
> XIPropertyValuePtr prop,
>   common->wcmCursorProxoutDist = value;
>   } else if (property == prop_threshold)
>   {
> + const INT32 MAXIMUM = 2048; /* backwards compatibility */
>   INT32 value;
>  
>   if (prop->size != 1 || prop->format != 32)
> @@ -836,8 +838,10 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
> XIPropertyValuePtr prop,
>  
>   if (value == -1)
>   value = DEFAULT_THRESHOLD;
> - else if ((value < 1) || (value > FILTER_PRESSURE_RES))
> + else if ((value < 1) || (value > MAXIMUM))
>   return BadValue;
> + else
> + value *= (FILTER_PRESSURE_RES / MAXIMUM);
>  
>   if (!checkonly)
>   common->wcmThreshold = value;
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index 1575960..9de9cab 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -183,7 +183,7 @@ struct _WacomModel
>  
>  #define IsUSBDevice(common) ((common)->wcmDevCls == )
>  
> -#define FILTER_PRESSURE_RES  2048/* maximum points in pressure curve */
> +#define FILTER_PRESSURE_RES  65536   /* maximum points in pressure curve */
>  /* Tested result for setting the pressure threshold to a reasonable value */
>  #define THRESHOLD_TOLERANCE (FILTER_PRESSURE_RES / 125)
>  #define DEFAULT_THRESHOLD (FILTER_PRESSURE_RES / 75)
> -- 
> 2.10.0
> 

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

[Linuxwacom-devel] [PATCH 1/2] Increase full-scale pressure range from 0..2047 to 0..65535

[Jason Gerecke]

The driver has historically normalized the pressure range of all kernel
devices to 0..2047 rather than using their native range to keep things
like the application of the pressure curve simple. Pens that report more
than 2048 pressure levels are also normalized down to this range though,
reducing their precision. In order to accomodate the new 8K pen (and any
future pens with even higher precision), this patch bumps up the full-
scale range to be 0..65535. This number was chosen both because it far
exceeds anything currently known about, and also because it matches the
normalization range used over the wire by the Wayland tablet protocol.

Note that the WACOM_PROP_PRESSURE_THRESHOLD value has been tied to the
normalized (2048-level) pressure range for some time, meaning that we
cannot simply change the range without causing a change in the perceived
threshold for users. To ensure compatibility, the value is interpreted
as a fraction of 2048 and then scaled to the actual normalization range.

Signed-off-by: Jason Gerecke 
---
 src/wcmXCommand.c   | 6 +-
 src/xf86WacomDefs.h | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
index 02278ba..403bc84 100644
--- a/src/wcmXCommand.c
+++ b/src/wcmXCommand.c
@@ -256,6 +256,7 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
}
 
values[0] = (!common->wcmMaxZ) ? 0 : common->wcmThreshold;
+   values[0] /= (FILTER_PRESSURE_RES / 2048); /* backwards compatibility */
prop_threshold = InitWcmAtom(pInfo->dev, WACOM_PROP_PRESSURE_THRESHOLD, 
XA_INTEGER, 32, 1, values);
 
values[0] = common->wcmSuppress;
@@ -827,6 +828,7 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
common->wcmCursorProxoutDist = value;
} else if (property == prop_threshold)
{
+   const INT32 MAXIMUM = 2048; /* backwards compatibility */
INT32 value;
 
if (prop->size != 1 || prop->format != 32)
@@ -836,8 +838,10 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property, 
XIPropertyValuePtr prop,
 
if (value == -1)
value = DEFAULT_THRESHOLD;
-   else if ((value < 1) || (value > FILTER_PRESSURE_RES))
+   else if ((value < 1) || (value > MAXIMUM))
return BadValue;
+   else
+   value *= (FILTER_PRESSURE_RES / MAXIMUM);
 
if (!checkonly)
common->wcmThreshold = value;
diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
index 1575960..9de9cab 100644
--- a/src/xf86WacomDefs.h
+++ b/src/xf86WacomDefs.h
@@ -183,7 +183,7 @@ struct _WacomModel
 
 #define IsUSBDevice(common) ((common)->wcmDevCls == )
 
-#define FILTER_PRESSURE_RES2048/* maximum points in pressure curve */
+#define FILTER_PRESSURE_RES65536   /* maximum points in pressure curve */
 /* Tested result for setting the pressure threshold to a reasonable value */
 #define THRESHOLD_TOLERANCE (FILTER_PRESSURE_RES / 125)
 #define DEFAULT_THRESHOLD (FILTER_PRESSURE_RES / 75)
-- 
2.10.0


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel