On Mon, Feb 22, 2016 at 05:58:22PM -0800, Jason Gerecke wrote: > On 02/17/2016 05:39 PM, Peter Hutterer wrote: > > A nonzero resolution on the tilt axes is units/rad so we can calculate the > > physical min/max based. Uneven min/max ranges are supported. > > > > Signed-off-by: Peter Hutterer <[email protected]> > > --- > > src/evdev-tablet.c | 28 +++++++++++++++++++++------- > > test/tablet.c | 12 ++++++++---- > > 2 files changed, 29 insertions(+), 11 deletions(-) > > > > diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c > > index 1e5c2cd..226d5bd 100644 > > --- a/src/evdev-tablet.c > > +++ b/src/evdev-tablet.c > > @@ -230,17 +230,31 @@ adjust_tilt(const struct input_absinfo *absinfo) > > double range = absinfo->maximum - absinfo->minimum; > > double value = (absinfo->value - absinfo->minimum) / range; > > const int WACOM_MAX_DEGREES = 64; > > + double max_degrees; > > + > > + /* If resolution is nonzero, it's in units/radian. But require > > + * a min/max less/greater than zero so we can assume 0 is the > > + * center */ > > + if (absinfo->resolution != 0 && > > + absinfo->maximum > 0 && > > + absinfo->minimum < 0) { > > + int range_max = absinfo->value >= 0 ? absinfo->maximum > > + : -absinfo->minimum; > > + > > + max_degrees = 180.0/M_PI * range_max/absinfo->resolution; > > + } else { > > + /* Wacom supports physical [-64, 64] degrees, so map to that by > > + * default. If other tablets have a different physical range or > > + * nonzero physical offsets, they need extra treatment > > + * here. > > + */ > > + max_degrees = WACOM_MAX_DEGREES; > > + } > > > > /* Map to the (-1, 1) range */ > > value = (value * 2) - 1; > > > > - /* Wacom supports physical [-64, 64] degrees, so map to that by > > - * default. If other tablets have a different physical range or > > - * nonzero physical offsets, they need extra treatment > > - * here. > > - */ > > - > > - return value * WACOM_MAX_DEGREES; > > + return value * max_degrees; > > } > > This looks correct, if a bit difficult to follow because of the range > normalization (specifically the subtle need for "range_max"). You might > consider having the normalization only occur in the 'else' case so that > the 'if' can be written more straightforwardly: > > > double value; > const int WACOM_MAX_DEGREES = 64; > > if ( /* can use resolution */ ) { > value = 180.0/M_PI * absinfo->value/absinfo->resolution; > } else { > /* normalize and scale to +-64 degrees */ > } > > return value; > > > That aside, everything looks fine. > > Reviewed-by: Jason Gerecke <[email protected]>
amended as requested, thanks! Cheers, Peter > > > > > static inline int32_t > > diff --git a/test/tablet.c b/test/tablet.c > > index c5dc892..ad6ac45 100644 > > --- a/test/tablet.c > > +++ b/test/tablet.c > > @@ -3298,7 +3298,8 @@ START_TEST(tilt_x) > > ck_assert_double_ge(tx, -52); > > > > ty = libinput_event_tablet_tool_get_tilt_y(tev); > > - ck_assert_double_eq(ty, -64); > > + ck_assert_double_ge(ty, -65); > > + ck_assert_double_lt(ty, -63); > > > > libinput_event_destroy(event); > > > > @@ -3320,7 +3321,8 @@ START_TEST(tilt_x) > > ck_assert_double_le(tx, expected_tx + 2); > > > > ty = libinput_event_tablet_tool_get_tilt_y(tev); > > - ck_assert_double_eq(ty, -64); > > + ck_assert_double_ge(ty, -65); > > + ck_assert_double_lt(ty, -63); > > > > libinput_event_destroy(event); > > > > @@ -3365,7 +3367,8 @@ START_TEST(tilt_y) > > ck_assert_double_ge(ty, -52); > > > > tx = libinput_event_tablet_tool_get_tilt_x(tev); > > - ck_assert_double_eq(tx, -64); > > + ck_assert_double_ge(tx, -65); > > + ck_assert_double_lt(tx, -63); > > > > libinput_event_destroy(event); > > > > @@ -3387,7 +3390,8 @@ START_TEST(tilt_y) > > ck_assert_double_le(ty, expected_ty + 2); > > > > tx = libinput_event_tablet_tool_get_tilt_x(tev); > > - ck_assert_double_eq(tx, -64); > > + ck_assert_double_ge(tx, -65); > > + ck_assert_double_lt(tx, -63); > > > > libinput_event_destroy(event); > > > > _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
