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

ndk: Add tool_type getter for Pointer events #323

Merged
merged 4 commits into from
Sep 6, 2022
Merged

ndk: Add tool_type getter for Pointer events #323

merged 4 commits into from
Sep 6, 2022

Conversation

Atilogit
Copy link
Contributor

I'm working on a PR for winit that adds stylus support for Windows and Android.
This getter is required to differentiate between touch, mouse and stylus inputs consistently.

let tool_type = unsafe {
ffi::AMotionEvent_getToolType(self.event.as_ptr(), self.index as ffi::size_t) as u32
};
tool_type.try_into().unwrap()
Copy link
Member

Choose a reason for hiding this comment

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

It seems that in some cases we use .try_into().unwrap_or(XXX::Unknown), but I'd have to ask @mb64 why that wasn't done in all cases.

Though I think I'd rather panic than consider every yet-unknown value to be equivalent to the literal _UNKNOWN constant, as this typically means the NDK or these enums need an update (which I'd like to mitigate even further by generating these types and derives in bindgen directly, but that hasn't materialized yet and especially the event code uses anonymous enums and typedefs which can't yet be interpreted that way). For example, Source is missing Hdmi and Sensor :)

Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

Thanks, this looks okay to me!

The CI failure is not your fault, GitHub's Virtual Environments team removed an "erroneous" ndk-bundle symlink but also seems to have deemed it necessary to delete all environment variables with it - instead of pointing them to a different known-working folder 😞 - actions/runner-images#5879.

@MarijnS95
Copy link
Member

MarijnS95 commented Sep 6, 2022

@Atilogit I had to rebase and force-push your branch to make the CI re-run, all failed CI runs seem to be old enough that there's no "rerun [failed] jobs" button anymore.

EDIT: Unfortunately your signature is now lost, so you might want to redo that yourself instead :)

@Atilogit
Copy link
Contributor Author

Atilogit commented Sep 6, 2022

Thank you for getting back to this!
I don't really care about the signature so feel free to merge if you don't see any other issue with it.

@MarijnS95
Copy link
Member

It took a while for Github to roll out the fixed images, while I was on holiday.

Do you want to add a changelog entry for this?

@Atilogit
Copy link
Contributor Author

Atilogit commented Sep 6, 2022

Sure, if it's not an issue.

@MarijnS95
Copy link
Member

@Atilogit Feel free to push one or let me know if I have to add it for you before merging.

@Atilogit
Copy link
Contributor Author

Atilogit commented Sep 6, 2022

On it right now

@MarijnS95 MarijnS95 changed the title ndk: Add tool_type getter to Pointer for stylus PR ndk: Add tool_type getter for Pointer events Sep 6, 2022
ndk-sys/CHANGELOG.md Outdated Show resolved Hide resolved
ndk/CHANGELOG.md Outdated Show resolved Hide resolved
@MarijnS95 MarijnS95 merged commit 914439e into rust-mobile:master Sep 6, 2022
rib added a commit to rust-mobile/android-activity that referenced this pull request Oct 22, 2022
This maintains compatibility with the `ndk` crate's `Pointer` API

Ref: rust-mobile/ndk#323

This will also be required for enabling pen pressure support to
Winit, re: rust-windowing/winit#2396
rib added a commit to rust-mobile/android-activity that referenced this pull request Oct 22, 2022
This maintains compatibility with the `ndk` crate's `Pointer` API

Ref: rust-mobile/ndk#323

This will also be required for enabling pen pressure support to
Winit, re: rust-windowing/winit#2396
rib added a commit to rust-mobile/android-activity that referenced this pull request Oct 23, 2022
This maintains compatibility with the `ndk` crate's `Pointer` API

Ref: rust-mobile/ndk#323

This will also be required for enabling pen pressure support to
Winit, re: rust-windowing/winit#2396
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