Bug in CalibrationTransform

[AndreR]

Hello,

first of all I have to admit I really enjoy uGFX. I've been using STemWin before and moving to uGFX was absolutely the right decision. It's great having the source code. Which also brings me to the reason of this post. I came across this:

	static inline void CalibrationTransform(GMouseReading *pt, const GMouseCalibration *c) {
		pt->x = (coord_t) (c->ax * pt->x + c->bx * pt->y + c->cx);
		pt->y = (coord_t) (c->ay * pt->x + c->by * pt->y + c->cy);
	}

The problem here is, that pt->x is assigned an new value, which is used again in the next line. This is certainly not intended. In my extreme case, X and Y of the touch interface and the display were swapped. Leading to y being assigned to x and both values used again to calculate y. Thereby, Y was too large and has always been clipped.

I suggest the following:

	static inline void CalibrationTransform(GMouseReading *pt, const GMouseCalibration *c) {
		coord_t x = (coord_t) (c->ax * pt->x + c->bx * pt->y + c->cx);
		coord_t y = (coord_t) (c->ay * pt->x + c->by * pt->y + c->cy);
		pt->x = x;
		pt->y = y;
	}

Cheers,

Andre

[Joel Bodenmann]

Hello Andre and welcome to the community!

Thank you for your positive feedback. We are happy to hear that you like uGFX.

Regarding the calibration issue: This is indeed a bug. You Are completely right, the two calculations need to be independent from each other.

We just pushed a fix as per your suggestion: https://bitbucket.org/Tectu/ugfx/commit ... d7eaa3eb6a

Thank you for your contribution!

~ Tectu

[fixer]

the bug fix is valid ( good catch ) but the second extra variable and assignment in the fix are unnecessary, it seems.

static inline void CalibrationTransform(GMouseReading *pt, const GMouseCalibration *c) {
      coord_t tmp = (coord_t) (c->ax * pt->x + c->bx * pt->y + c->cx);
      pt->y = (coord_t) (c->ay * pt->x + c->by * pt->y + c->cy);
      pt->x = tmp;
 }

I assume the aim is to keep the code as tight and small as possible.

[inmarket]

Yes and we will tidy that up shortly.

It is low on the priority list though because the other code works and the compiler optimiser will likely sort it out anyway.

[Joel Bodenmann]

I'll get that done in a few minutes/hours.

This way it will still make it into the new 2.4 release.

Thank you for your contribution!

~ Tectu

[inmarket]

Thanks Tectu.