Hi,
On Wed, Jul 8, 2015 at 1:03 AM, Peter Hutterer <peter.hutte...@who-t.net> wrote: > > +double > > +evdev_device_transform_touch_point_to_mm(struct evdev_device *device, > > + int32_t axis_value, > > + double axis_angle) > > oh dear. I just spent about 15 min trying to figure out the conversion and > wondering why you pass x/y in here etc. "point" refers to a point on the > screen. You're passing in the size of the ellipsis so please don't name > this > touch_point, it's very confusing. Name it ellipsis instead, that's how you > call it in the test suite too. > Hm yes this the unified code of what was duplicated in the previous version. Agreed using the term point when the visual extent of something meant is confusing. > > +{ > > + double x_res = device->abs.absinfo_x->resolution; > > + double y_res = device->abs.absinfo_y->resolution; > > + > > + if (x_res == y_res) > > + return axis_value / x_res; > > + > > + if (device->abs.absinfo_orientation == NULL) > > + return axis_value * 2.0 / (x_res + y_res); > > I admit, I'm a bit confused in how this should work. if we don't have > orientation, the angle should always be 0, right? > so the returned value is either /xres or /yres, depending on what you pass > in. can you explain this in more detail for me please? > This is a rather broken case - the screen uses different vertical and horizontal resolution, but we have not orientation value. So the code estimates the resolution by taking the average. I do not have access to such a touch device but android considered this case. > > + > > + return axis_value / (y_res*fabs(cos(deg2rad(axis_angle))) + > > + x_res*fabs(sin(deg2rad(axis_angle)))); > > can we please use a tmp scale variable for the fabs(cos(deg2rad())) > nesting? > (besides, please dont mix spaces with no spaces) > > also, this is doing my head in. we're using x for the touch major and y for > the touch minor, despite, in the default angle, the major being the y axis > and the minor being the x axis. Which leads to the odd case of > in the simplest case, we have an angle of 0 -> cos 1, sin 0. a touch major > is assigned to touch_point.x and the returned value is x/yres. that's > correct I think, but confusing as hell. > > I think we should assign major to y and minor to x, that would make things > a > lot less confusing. > To be honest not placing the values inside a device_coords structure is probably a safer way to avoid further confusion. What about struct ellipse { int major, minor};? > [...] > > + */ > > +double > > +libinput_event_touch_get_minor_transformed(struct libinput_event_touch > *event, > > + uint32_t width, > > + uint32_t height); > > + > > +/** > > + * @ingroup event_touch > > + * > > + * Return the current pressure value touch point normalized to the range > > + * [0,1]. If this value is not provided by the device, it is always 0.0. > > thinking about this - we will at some point have to add hovering to > libinput and that's where a pressure default of 0 is going to bite us. > I'd go with the opposite and say that if pressure doesn't exist, it's > always > 1 on touch. > I had 1.0 originally so that users do not have to care whether the value is availble or not. You convinced me that 0.0 would be better since it allows users to deal with both cases. Now for hover (as in no physical contact i guess?) - why not resolve it by having another touch event type like .. LIBINPUT_EVENT_TOUCH_HOVER to differ between these cases? Pressure would not be defined for that event type, even when the device provides pressure on physical contacts. So 0.0 to indicate a lacking capability on TOUCH_MOTION still seems ok. regards Andreas
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel