Thank you for your cleanup work, Jason. The first two patches look fine.
This one definitely needs some testing to make sure it achieves what you
planned for.

Assuming your testing of this patch was successfully, the whole set is;

Reviewed-by: Ping Cheng <ping.ch...@wacom.com>

With one question below.

On Fri, Dec 8, 2017 at 1:55 PM, Jason Gerecke <killert...@gmail.com> wrote:

> Coordinate averaging is useful for producing smooth strokes, but this
> averaging can cause button events to be sent at the wrong location.
> In particular, when making a series of quick strokes with an AES pen,
> the actual location the pen contacts the display can be some distance
> away from the first (low-quality) position report. This can result in
> "hook" artifacts if it is still influencing the averaged pointer
> position when the pen goes down.
>
> To prevent these artifacts from appearing, we reset the averaging filter
> whenever a button is pressed. This ensures that button events are sent
> at the pen's actual location, plus or minus some (much smaller) noise.
> Resetting the filter should not significantly impact stroke quality since
> buttons are rarely pressed mid-stroke.
>
> https://sourceforge.net/p/linuxwacom/bugs/338/
>
> Signed-off-by: Jason Gerecke <jason.gere...@wacom.com>
> ---
>  src/wcmCommon.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index 900544b..e515078 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1241,6 +1241,15 @@ static void commonDispatchDevice(InputInfoPtr
> pInfo,
>                 if (!priv->oldState.proximity)
>                         wcmResetSampleCounter(pChannel);
>
> +               /* Reset filter whenever a button is pressed to
>

Shouldn't we do it for button release too? Some actions may be trigged by
button release instead.

+                * ensure the clicks are sent at the pen's actual
> +                * position. This prevents "hooks" from appearing
> +                * at the beginning of series of rapidly-drawn or
> +                * highly-averaged strokes.
> +                */
> +               if ((priv->oldState.buttons ^ filtered.buttons) &
> filtered.buttons)
>

So, is the second filtered.buttons necessary above?

Cheers,
Ping

+                       wcmResetSampleCounter(pChannel);
> +
>                 wcmFilterCoord(common,pChannel,&filtered);
>         }
>
> --
> 2.15.1
>
>
------------------------------------------------------------------------------
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

Reply via email to