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

Adjust default touch calibration for Core2 #469

Merged
merged 2 commits into from
Dec 27, 2023

Conversation

toyoshim
Copy link
Contributor

@toyoshim toyoshim commented Nov 9, 2023

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.

lovyan03 and others added 2 commits September 8, 2023 09:06
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
Copy link
Collaborator

tobozo commented Nov 11, 2023

hi, thanks for your contribution 👍

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

@tobozo tobozo changed the base branch from master to develop November 11, 2023 10:03
@toyoshim
Copy link
Contributor Author

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

@toyoshim
Copy link
Contributor Author

toyoshim commented Nov 12, 2023

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
Copy link
Collaborator

tobozo commented Nov 13, 2023

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
Copy link
Contributor Author

toyoshim commented Nov 13, 2023

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
Copy link
Collaborator

tobozo commented Nov 13, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Collaborator

tobozo commented Nov 18, 2023

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

@toyoshim
Copy link
Contributor Author

ok, sounds good.
thanks!

Copy link

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.

@github-actions github-actions bot added the stale Inactive issues label Dec 18, 2023
@tobozo
Copy link
Collaborator

tobozo commented Dec 18, 2023

bump

@github-actions github-actions bot removed the stale Inactive issues label Dec 18, 2023
@lovyan03 lovyan03 merged commit 529594f into lovyan03:develop Dec 27, 2023
89 of 98 checks passed
@lovyan03
Copy link
Owner

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

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.

3 participants