[PATCH synaptics 2/2] Force SLOTSTATE_EMPTY on DeviceOff
Peter Hutterer
peter.hutterer at who-t.net
Mon Apr 30 18:19:02 PDT 2012
On Mon, Apr 30, 2012 at 10:45:30AM -0700, Chase Douglas wrote:
> On 04/29/2012 08:21 PM, Peter Hutterer wrote:
> > SLOTSTATE_OPEN_EMPTY on resume leads to erroneously detected touches.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > src/eventcomm.c | 2 +-
> > src/synaptics.c | 6 +++---
> > src/synproto.c | 6 +++---
> > src/synproto.h | 2 +-
> > 4 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index 741f988..4ef8ad6 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -662,7 +662,7 @@ EventReadHwState(InputInfoPtr pInfo,
> > SynapticsParameters *para = &priv->synpara;
> > struct eventcomm_proto_data *proto_data = priv->proto_data;
> >
> > - SynapticsResetTouchHwState(hw);
> > + SynapticsResetTouchHwState(hw, FALSE);
> >
> > /* Reset cumulative values if buttons were not previously pressed */
> > if (!hw->left && !hw->right && !hw->middle)
> > diff --git a/src/synaptics.c b/src/synaptics.c
> > index 935650d..6dc8004 100644
> > --- a/src/synaptics.c
> > +++ b/src/synaptics.c
> > @@ -1619,7 +1619,7 @@ timerFunc(OsTimerPtr timer, CARD32 now, pointer arg)
> >
> > priv->hwState->millis += now - priv->timer_time;
> > SynapticsCopyHwState(hw, priv->hwState);
> > - SynapticsResetTouchHwState(hw);
> > + SynapticsResetTouchHwState(hw, FALSE);
> > delay = HandleState(pInfo, hw, hw->millis, TRUE);
> >
> > priv->timer_time = now;
> > @@ -1659,7 +1659,7 @@ ReadInput(InputInfoPtr pInfo)
> > int delay = 0;
> > Bool newDelay = FALSE;
> >
> > - SynapticsResetTouchHwState(hw);
> > + SynapticsResetTouchHwState(hw, FALSE);
> >
> > while (SynapticsGetHwState(pInfo, priv, hw)) {
> > /* Semi-mt device touch slots do not track touches. When there is a
> > @@ -3017,7 +3017,7 @@ UpdateTouchState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> > }
> > }
> >
> > - SynapticsResetTouchHwState(hw);
> > + SynapticsResetTouchHwState(hw, FALSE);
> > #endif
> > }
> >
> > diff --git a/src/synproto.c b/src/synproto.c
> > index cf54c4d..e7534e2 100644
> > --- a/src/synproto.c
> > +++ b/src/synproto.c
> > @@ -153,11 +153,11 @@ SynapticsResetHwState(struct SynapticsHwState *hw)
> > hw->middle = 0;
> > memset(hw->multi, 0, sizeof(hw->multi));
> >
> > - SynapticsResetTouchHwState(hw);
> > + SynapticsResetTouchHwState(hw, TRUE);
> > }
> >
> > void
> > -SynapticsResetTouchHwState(struct SynapticsHwState *hw)
> > +SynapticsResetTouchHwState(struct SynapticsHwState *hw, Bool force_empty)
> > {
> > #ifdef HAVE_MULTITOUCH
> > int i;
> > @@ -175,7 +175,7 @@ SynapticsResetTouchHwState(struct SynapticsHwState *hw)
> > case SLOTSTATE_OPEN:
> > case SLOTSTATE_OPEN_EMPTY:
> > case SLOTSTATE_UPDATE:
> > - hw->slot_state[i] = SLOTSTATE_OPEN_EMPTY;
> > + hw->slot_state[i] = force_empty ? SLOTSTATE_EMPTY : SLOTSTATE_OPEN_EMPTY;
> > break;
> >
> > default:
> > diff --git a/src/synproto.h b/src/synproto.h
> > index 7f80bcd..413579d 100644
> > --- a/src/synproto.h
> > +++ b/src/synproto.h
> > @@ -120,7 +120,7 @@ extern void SynapticsHwStateFree(struct SynapticsHwState **hw);
> > extern void SynapticsCopyHwState(struct SynapticsHwState *dst,
> > const struct SynapticsHwState *src);
> > extern void SynapticsResetHwState(struct SynapticsHwState *hw);
> > -extern void SynapticsResetTouchHwState(struct SynapticsHwState *hw);
> > +extern void SynapticsResetTouchHwState(struct SynapticsHwState *hw, Bool force_empty);
> >
> > extern Bool SynapticsIsSoftButtonAreasValid(int *values);
> >
>
> This looks functionally correct, but I don't love the extra force_empty
> parameter. There's only one case where we want to force empty slots.
> What if instead we make this a one-line change in SynapticsResetHwState
> after the call to SynapticsResetTouchHwState:
>
> /* Force all touches to the empty, closed state */
> memset(hw->slot_state, 0, hw->num_mt_mask * sizeof(enum SynapticsSlotState);
I'm not a fan of this. If you reset, you shouldn't need code outside the
caller to re-set some arrays again. We already have a reset_hw_state() that
does something else again. (eventually) having one function that resets the
struct is a good thing, even if the type of reset is controlled by some
parameters.
Cheers,
Peter
More information about the xorg-devel
mailing list