Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct the calibration when the axes are inverted. #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kreijack
Copy link
Contributor

I had a problem to calibrate a touch screen with the axes inverted. Looking at the code I found that when the axes inversion is detected, the {x,y}_{max,min} are swapped:

// calculate average of clicks
float x_min = (clicked.x[UL] + clicked.x[LL])/2.0;
float x_max = (clicked.x[UR] + clicked.x[LR])/2.0;
float y_min = (clicked.y[UL] + clicked.y[UR])/2.0;
float y_max = (clicked.y[LL] + clicked.y[LR])/2.0;

// Should x and y be swapped?
if (abs(clicked.x[UL] - clicked.x[UR]) < abs(clicked.y[UL] - clicked.y[UR])) {
    std::swap(x_min, y_min);
    std::swap(x_max, y_max);

However I think that when the axis are swapped the calculation of the
{x,y}_{max,min} are wrong.
Let me explain why:

During the calibration, on the screen are showed four crosses called UL, LR, LL, UR (Upper Left, Lower Right...).

If the axes are not swapped during the calibration we got the following coordinates (x,y the numbers are hypothetical):

UL -> 10,10            UR -> 90,10
LL -> 10,70            LR -> 90,70

and

x_min = (10+10)/2 = 10
x_max = (90+90)/2 = 90
y_min = (10+10/2 = 10
y_max = (70+70)/2 = 70

Instead if the axes are inverted

UL -> 10,10            UR -> 10,90
LL -> 70,10            LR -> 70,90

and

x_min = (10+70)/2 = 35
x_max = (10+70)/2 = 35                   x_min == x_max !!!!
y_min = (10+90)/2 = 45
y_max = (10+90)/2 = 45                   y_min == y_max !!!!

which are basically wrong. I think that the right solution should be to calculate the
{x,y}_{max,min} as following when an axis inversion is detected:

    x_min = (clicked.x[UL] + clicked.x[UR])/2.0;
    x_max = (clicked.x[LL] + clicked.x[LR])/2.0;
    y_min = (clicked.y[UL] + clicked.y[LL])/2.0;
    y_max = (clicked.y[UR] + clicked.y[LR])/2.0;

( Note the different array indexes )

How reproduce the bug:

  1. start xinput_calibrator --fake
  2. calibrating clicking the upper left corner, then the LOWER LEFT corner, then the UPPER RIGHT corner, then the lower right corner

RESULT: in the output message message you can see the MaxX and MinX values, the MaxY and the MinY values very near.
EXPECTED RESULT: the difference between MaxX and MinX should be comparable to the screen width.

I had a problem to calibrate a touch screen with the axes inverted. Looking at the code I found that when the axes inversion is detected, the {x,y}_{max,min} are swapped:

    // calculate average of clicks
    float x_min = (clicked.x[UL] + clicked.x[LL])/2.0;
    float x_max = (clicked.x[UR] + clicked.x[LR])/2.0;
    float y_min = (clicked.y[UL] + clicked.y[UR])/2.0;
    float y_max = (clicked.y[LL] + clicked.y[LR])/2.0;

    // Should x and y be swapped?
    if (abs(clicked.x[UL] - clicked.x[UR]) < abs(clicked.y[UL] - clicked.y[UR])) {
        std::swap(x_min, y_min);
        std::swap(x_max, y_max);

However I think that when the axis are swapped the calculation of the
{x,y}_{max,min} are wrong.
Let me explain why:

During the calibration, on the screen are showed four crosses called UL, LR, LL, UR (Upper Left, Lower Right...).

If the axes are not swapped during the calibration we got the following coordinates (x,y the numbers are hypothetical):

    UL -> 10,10            UR -> 90,10
    LL -> 10,70            LR -> 90,70

and

    x_min = (10+10)/2 = 10
    x_max = (90+90)/2 = 90
    y_min = (10+10/2 = 10
    y_max = (70+70)/2 = 70

Instead if the axes are inverted

    UL -> 10,10            UR -> 10,90
    LL -> 70,10            LR -> 70,90

and

    x_min = (10+70)/2 = 35
    x_max = (10+70)/2 = 35                   x_min == x_max
    y_min = (10+90)/2 = 45
    y_max = (10+90)/2 = 45                   y_min == y_max

which are basically wrong. I think that the right solution should be to calculate the
{x,y}_{max,min} as following when an axis inversion is detected:

        x_min = (clicked.x[UL] + clicked.x[UR])/2.0;
        x_max = (clicked.x[LL] + clicked.x[LR])/2.0;
        y_min = (clicked.y[UL] + clicked.y[LL])/2.0;
        y_max = (clicked.y[UR] + clicked.y[LR])/2.0;

( Note the different array indexes )
@ddario
Copy link

ddario commented Sep 20, 2016

Just stumbled in the same problem and after I found my solution someone pointed me to yours which looks a little different.
From what I see, if we find out axes are swapped (and "Evdev Axes Swap" is 0) we need to do
x_min = (clicked.y[UL] + clicked.y[LL])/2.0;
x_max = (clicked.y[UR] + clicked.y[LR])/2.0;
y_min = (clicked.x[UL] + clicked.x[UR])/2.0;
y_max = (clicked.x[LL] + clicked.x[LR])/2.0;
because what we think to be x is y and vice versa or am I missing something?

@kreijack
Copy link
Contributor Author

Hi,
It was an old work, so may be I missing something. However this patch worked form me, so I guess that when the conversion matrix is printed on the screen, the axes are swapped after the scaling. If this is right the scaling of 'x' should be performed against 'min_x/max_x'; the same for y. Then the values my be swapped.

You would be right is the swapping is performed before the scaling....

@ddario
Copy link

ddario commented Sep 21, 2016

Hi Goffredo,
my problem was that trying to calibrate the touchscreen without
setting the Evdev Axes Swap property to 1, the coordinates of the clicks
were swapped and as you reported x/y _min/_max calculation was faulty
(_min=_max).

As far as I can see in the code, the check

if (abs(clicked.x[UL] - clicked.x[UR]) < abs(clicked.y[UL] -
clicked.y[UR])) {
new_axis.swap_xy = !new_axis.swap_xy;
...

is made to change the swap info and this applies both if Evdev Axes Swap
is set or not if condition is met.

From what I can see, touch coordinates are print in

bool Calibrator::add_click(int x, int y)

and report coordinates accordingly to the status of the Axes Swap
property (Swap = 0 => screen X/Y = device X/Y, Swap = 1 => screen X/Y =
device Y/X).

So, assuming that we find axes have to be swapped (again, to me this
could also mean that Evdev Axes Swap was set and we find out it is
wrong), in the new calibration x and y axes will be swapped so to find
out x_min/x_max we need to use y coordinates and vice versa.

Also I have to say that code checks if Evdex Axis Inversion is specified
to take care of

val = (pEvdev->absinfo[i].maximum - val + pEvdev->absinfo[i].minimum);

but after that, it is also needed to see if x/y _max >
x/y _min because it is possible (to me reasonable) that if you rotate
screen left/right one of the new axes needs inversion.

If you want I have a patch that applies on current master and with it I
get the same calibration even if I use wrong options. This seems useful
to me because you don't need to specify any option before to start
calibration and the result will let you know if you need to swap axes or
not and also takes care of axis inversion using corrected _min/_max.

Once I have time to clean it up I'd submit it but would be great if
someone could test it and report if it works properly or not.

Daniele.

On mar, 2016-09-20 at 12:17 -0700, Goffredo Baroncelli wrote:

Hi,
It was an old work, so may be I missing something. However this patch
worked form me, so I guess that when the conversion matrix is printed
on the screen, the axes are swapped after the scaling. If this is
right the scaling of 'x' should be performed against 'min_x/max_x';
the same for y. Then the values my be swapped.

You would be right is the swapping is performed before the scaling....


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants