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

Fix touch calibration matrix #9553

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

khalid151
Copy link
Contributor

Describe your PR, what does it fix/add?

The previous implementation overrides any calibration matrix that is set (like in hwdb), even when transform is not set in the config. This PR checks if a calibration matrix is set, and would ignore transforms as they'd already be applied. Otherwise, matrices set by transform are used.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

This could also be applied to tablets in setTabletConfigs but I haven't tested that.

Is it ready for merging, or does it need work?

I believe it is ready.

Rotation transform property is only applied when **no** calibration
matrix is set.
Co-authored-by: nyx <nnyyxxxx@protonmail.com>
Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if the user wants to forcefully override this?

@khalid151
Copy link
Contributor Author

Then it would be better to check if the transform is set in the config, and this means getting rid of the default transform value, only applying it when it's set.

Or having a calibration_matrix config variable that takes precedence over transform.

But I personally think that it's redundant, because the only way a device would have a non-default calibration matrix is when it is set by the user.

@khalid151
Copy link
Contributor Author

Now transform works as expected, but only when it's set.

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

Successfully merging this pull request may close these issues.

3 participants