lovyan03 / LovyanGFX

SPI LCD graphics library for ESP32 (ESP-IDF/ArduinoESP32) / ESP8266 (ArduinoESP8266) / SAMD51(Seeed ArduinoSAMD51)
Other
1.03k stars 189 forks source link

Adjust default touch calibration for Core2 #469

Closed toyoshim closed 4 months ago

toyoshim commented 6 months ago

p->touch(t) call invokes Panel_Device::touchCalibrate and results in setting up the affine matrix to map the touch geometry into the panel one. This is not preferable as the touch geometry contains the virtual screen panel area outside the panel area.

This patch changes the order to call setCalibrateAffine with dedicated parameters for Core2 to overwrite the default unexpected matrix.

tobozo commented 6 months ago

hi, thanks for your contribution :+1:

I'm not sure I understand the changes (didn't observe any anomaly before) how can I reproduce the issue this PR addresses?

[edit] changed the target branch

toyoshim commented 6 months ago

Hi, I will prepare an example project with that you can reproduce the issue. I'll be back here once it's ready.

toyoshim commented 6 months ago

https://github.com/toyoshim/LovyanGFXTest/blob/main/src/main.cc Here is an ESPIDF project and test code for PlatformIO/Code. Please check my comment on main.cc and console logs shown on running it and touching the screen.

tobozo commented 6 months ago

after doing some research it appears changing the calibration sequence order will impact existing projects relying on the current behaviour

  // Current getTouch() never returns a point that is recognized as in the
  // virtual screen button area.

this is by design, getTouch() and getTouchRaw() are supposed to behave differently, the former is constrained to the lcd area while the latter is constrained to the touch area

supporting the touch zone outside the lcd area with M5Core2 can be done as follows:

    int TOUCH_MAX_POINTS = 2;
    lgfx::touch_point_t touch_raw[TOUCH_MAX_POINTS];
    int count = tft.getTouchRaw(touch_raw, TOUCH_MAX_POINTS);
    while (--count >= 0) {
      if( touch_raw[count].y > tft.height() ) {
        // (raw.x, raw.y) touch point is in the M5Core2 buttons area
        Serial.printf("[%3d:%-3d]\n", touch_raw[count].x, touch_raw[count].y );
      }
    }
toyoshim commented 6 months ago

getTouch() returns adjusted points per the calibrated or restoed affine matrix, and getTouchRaw() returns the original point. So, I understand they could return different points. But the problem exists in the default calibration logic for Core2.

https://twitter.com/lovyan03/status/1721701020118695948 Here is a thread lovyan and I discussed this, and he expects if we touch outside the Panel, getTouch() returns a point outside the Panel rather than adjusted point that is scaled to map on the Panel, so that we can also handle the virtual screen buttons through the getTouch() API. This behavior is aligned with the behavior after users do a manual calibration.

Also, sorry. The thread linked above was in Japanese. I can translate it if machine translation doesn't work well for the thread.

tobozo commented 6 months ago

interesting, somehow I believed that getTouch returned a constrained value rather than a scaled value, so this means that any UI code relying on the assumption that touch height == display height is wrong ?

toyoshim commented 6 months ago

yeah, actual behavior is returning a scaled value rather than saturated/constrained value, i.e. touch.y * panel.height / touch.height. So, if I draw pixels on touching points, they are drawn at slightly shifted point in the y direction.

toyoshim commented 6 months ago

Hi, is there anything I can do here to move the discussion forward? or shall we close as work-as-intended? In such a case, I can have a workaround in my application end, as I just need to reinitialize the calibration setting after the library's initialization.

tobozo commented 6 months ago

let's keep this PR opens until @lovyan03 gives the final word

toyoshim commented 6 months ago

ok, sounds good. thanks!

github-actions[bot] commented 5 months ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

tobozo commented 5 months ago

bump

lovyan03 commented 4 months ago

Sorry for the long wait, and thank you for submitting this pull request...!