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