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 <[email protected]> > > --- > > 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
