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/event: Implement SourceClass bitflag and provide Source::class() getter #458

Merged
merged 2 commits into from
Jan 27, 2024

Conversation

MarijnS95
Copy link
Member

This enum was never pub even though it serves a slight purpose to more easily classify various event sources, which is accounted for in the bitmask-like value of AINPUT_SOURCE_XXX.

@MarijnS95 MarijnS95 requested a review from rib December 25, 2023 14:49
ndk/src/event.rs Outdated Show resolved Hide resolved
ndk/src/event.rs Outdated Show resolved Hide resolved
@MarijnS95 MarijnS95 changed the title ndk/event: Implement SourceClass enum and provide Source::class() getter ndk/event: Implement SourceClass bitflag and provide Source::class() getter Jan 18, 2024
ndk/src/event.rs Show resolved Hide resolved
@MarijnS95 MarijnS95 requested a review from rib January 19, 2024 15:13
@MarijnS95 MarijnS95 force-pushed the event-source-class branch 2 times, most recently from 80b6e1a to da5441f Compare January 19, 2024 15:20
Copy link
Contributor

@rib rib left a comment

Choose a reason for hiding this comment

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

cool, all looking good to me now!

Copy link
Contributor

@rib rib left a comment

Choose a reason for hiding this comment

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

oh, actually, maybe we shouldn't use !0 but should use AINPUT_SOURCE_CLASS_MASK ?

@MarijnS95
Copy link
Member Author

@rib good point. Since it's 0xff this could have been prevented if the type is u8. Which would you prefer, or should we do both?

Copy link
Contributor

@rib rib left a comment

Choose a reason for hiding this comment

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

Looking good except I think there's a spurious negate for the const _ value for SourceClass flags.

ndk/src/event.rs Outdated Show resolved Hide resolved
…ss()` getter

This `enum` was never `pub` even though it serves a slight purpose to
more easily classify various event sources, which is accounted for in
the bitmask-like value of `AINPUT_SOURCE_XXX`.  Even though every source
currently only appears to use a single class value, the class variants
all describe an individual bit and should be treated as a bitflags type.
… to be extended

Android may return values in `bitflags` that are not yet mapped or
recognized in the `ndk` crate.  This makes functions like `all()` and
`!` behave unexpectedly when they truncate or limit their operation to
known bits exclusively.

A more likable solution is disabling functions that are susceptible to
this, as was done in the `ash` crate.
@MarijnS95 MarijnS95 merged commit f216886 into master Jan 27, 2024
38 checks passed
@MarijnS95 MarijnS95 deleted the event-source-class branch January 27, 2024 15:43
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