On Sat, Mar 26, 2011 at 6:26 PM, Chris Bagwell <ch...@cnpbagwell.com> wrote:
> On Thu, Mar 24, 2011 at 5:35 PM, Ping Cheng <pingli...@gmail.com> wrote:
> > Change oldHwProx to oldCursorHwProx to better refect its use.
> >
> > Protocol 5 distance starts from the MaxCursorDist (256) when
> > getting in the prox while protocol 4 distance is 0 when
> > getting in the prox.
> >
> > oldCursorHwProx keeps the hardware in/out prox state so we can
> > we set the MaxCursorDist for the next round of relative cursor
> > movement when tool first comes in prox.
>
> I do appreciate the variable clarification. It wasn't obvious to me
> in past its scope was so limited.
>
> >
> > Signed-off-by: Ping Cheng <pingli...@gmail.com>
> > ---
> > src/wcmCommon.c | 24 ++++++++++--------------
> > src/wcmConfig.c | 2 +-
> > src/wcmUSB.c | 4 ----
> > src/xf86Wacom.c | 6 ------
> > src/xf86WacomDefs.h | 2 +-
> > 5 files changed, 12 insertions(+), 26 deletions(-)
> >
> > diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> > index 0f858eb..845cf6e 100644
> > --- a/src/wcmCommon.c
> > +++ b/src/wcmCommon.c
> > @@ -31,6 +31,8 @@
> > #define THRESHOLD_TOLERANCE (FILTER_PRESSURE_RES / 125)
> > #define DEFAULT_THRESHOLD (FILTER_PRESSURE_RES / 75)
> >
> > +#define MAX_CURSOR_DISTANCE 256
> > +
> > /* X servers pre 1.9 didn't copy data passed into xf86Post*Event.
> > * Data passed in would be modified, requiring the driver to copy the
> > * data beforehand.
> > @@ -1138,14 +1140,6 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
> > /* Device transformations come first */
> > priv = pInfo->private;
> >
> > - if (IsUSBDevice(common))
> > - {
> > - if (IsTouch(priv) && !ds->proximity)
> > - priv->oldHwProx = 0;
> > - else if (IsStylus(priv) || IsEraser(priv))
> > - priv->oldHwProx = 1;
> > - }
> > -
> > /* send a touch out for USB Tablet PCs */
> > if (IsUSBDevice(common) && !IsTouch(priv)
> > && common->wcmTouchDefault && !priv->oldProximity)
> > @@ -1180,19 +1174,18 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
> > filtered.pressure = applyPressureCurve(priv,&filtered);
> > }
> >
> > - else if (IsCursor(priv) && !priv->oldHwProx)
> > + else if (IsCursor(priv) && !priv->oldCursorHwProx)
> > {
> > /* initial current max distance for Intuos series */
> > if ((TabletHasFeature(common, WCM_ROTATION)) ||
> > (TabletHasFeature(common, WCM_DUALINPUT)))
> > - common->wcmMaxCursorDist = 256;
> > + common->wcmMaxCursorDist = MAX_CURSOR_DISTANCE;
> > else
> > common->wcmMaxCursorDist = 0;
> > }
> >
> > - /* Store current hard prox for next use */
> > - if (!IsTouch(priv))
> > - priv->oldHwProx = ds->proximity;
> > + /* Store cursor hardware prox for next use */
> > + priv->oldCursorHwProx = ds->proximity;
>
> Should it be wrapped with "if (IsCursor(priv))" to prevent a pen from
> updating it? I can't tell from diff alone if its an issue.
>
Good point. It normally should not be an issue. But better safe than sorry.
> What ever the answer:
>
> Reviewed-by: Chris Bagwell <ch...@cnpbagwell.com>
>
> The reset are questions for my understanding.
>
Thank you for the review. I'll wait a few days for more comments. V2 will be
sent sometime next week. My answers to your other questions are below.
>
> > /* User-requested filtering comes next */
> >
> > @@ -1227,14 +1220,17 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
> > /* force out-prox when distance is outside wcmCursorProxoutDist
> for pucks */
> > if (IsCursor(priv))
> > {
> > - /* force out-prox when distance is outside
> wcmCursorProxoutDist. */
> > if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
> > {
> > + /* protocol 5 distance starts from the
> MaxCursorDist
> > + * when getting in the prox.
> > + */
> > if (common->wcmMaxCursorDist > filtered.distance)
> > common->wcmMaxCursorDist =
> filtered.distance;
> > }
> > else
> > {
> > + /* protocol 4 distance is 0 when getting in the
> prox */
> > if (common->wcmMaxCursorDist < filtered.distance)
> > common->wcmMaxCursorDist =
> filtered.distance;
>
> These if()'s always confused me and the comments help but what is
> behavior of 4 vs. 5 when out of prox?
>
The difference lies in the distance we get. For P5 devices, distance is
reported as the maximum when tool is first detect then gradually reduced to
0 when getting closer to the tablet. The P4 is the opposite: distance is 0
when tool is first in prox. The values increases as the tool gets closer to
the tablet.
Maybe we should normalize it in wcmUSB so they all have some behavior
> here? Not for this patch of course.
>
Yeah, that can be considered in wcmUSB.c.
>
> > }
> > diff --git a/src/wcmConfig.c b/src/wcmConfig.c
> > index 2bf0ed3..6235d3c 100644
> > --- a/src/wcmConfig.c
> > +++ b/src/wcmConfig.c
> > @@ -62,7 +62,7 @@ static int wcmAllocate(InputInfoPtr pInfo)
> > priv->next = NULL;
> > priv->pInfo = pInfo;
> > priv->common = common; /* common info pointer */
> > - priv->oldHwProx = 1; /* previous hardware proximity */
> > + priv->oldCursorHwProx = 0; /* previous cursor hardware
> proximity */
> > priv->nPressCtrl [0] = 0; /* pressure curve x0 */
> > priv->nPressCtrl [1] = 0; /* pressure curve y0 */
> > priv->nPressCtrl [2] = 100; /* pressure curve x1 */
> > diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> > index 5f0dbe2..e65f24f 100644
> > --- a/src/wcmUSB.c
> > +++ b/src/wcmUSB.c
> > @@ -394,10 +394,6 @@ static void usbInitProtocol5(WacomCommonPtr common,
> const char* id,
> >
> > /* tilt enabled */
> > common->wcmFlags |= TILT_ENABLED_FLAG;
> > -
> > - /* reinitialize max here since 0 is for Graphire series */
> > - common->wcmMaxCursorDist = 256;
> > -
> > }
> >
> > static void usbInitProtocol4(WacomCommonPtr common, const char* id,
> > diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c
> > index 6762a3d..a124ed3 100644
> > --- a/src/xf86Wacom.c
> > +++ b/src/xf86Wacom.c
> > @@ -442,12 +442,6 @@ static int wcmDevInit(DeviceIntPtr pWcm)
> >
> > wcmRotateTablet(pInfo, common->wcmRotate);
> >
> > - if (IsTouch(priv))
> > - {
> > - /* hard prox out */
> > - priv->oldHwProx = 0;
> > - }
> > -
> > InitWcmDeviceProperties(pInfo);
> > XIRegisterPropertyHandler(pInfo->dev, wcmSetProperty, NULL,
> wcmDeleteProperty);
> >
> > diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> > index 5f5d969..0e57087 100644
> > --- a/src/xf86WacomDefs.h
> > +++ b/src/xf86WacomDefs.h
> > @@ -276,7 +276,7 @@ struct _WacomDeviceRec
> > int oldThrottle; /* previous throttle value */
> > int oldButtons; /* previous buttons state */
> > int oldProximity; /* previous proximity */
> > - int oldHwProx; /* previous hardware proximity */
> > + int oldCursorHwProx; /* previous cursor hardware proximity */
> > int old_device_id; /* last in prox device id */
> > int old_serial; /* last in prox tool serial number */
> > int devReverseCount; /* Relative ReverseConvert called twice
> each movement*/
> > --
> > 1.7.4
> >
> >
> >
> ------------------------------------------------------------------------------
> > Enable your software for Intel(R) Active Management Technology to meet
> the
> > growing manageability and security demands of your customers. Businesses
> > are taking advantage of Intel(R) vPro (TM) technology - will your
> software
> > be a part of the solution? Download the Intel(R) Manageability Checker
> > today! http://p.sf.net/sfu/intel-dev2devmar
> > _______________________________________________
> > Linuxwacom-devel mailing list
> > Linuxwacom-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
> >
>
------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software
be a part of the solution? Download the Intel(R) Manageability Checker
today! http://p.sf.net/sfu/intel-dev2devmar
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
