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

android: Use event identifier instead of userdata pointer #1826

Merged
merged 1 commit into from
Jan 13, 2021

Conversation

MarijnS95
Copy link
Member

ndk-glue currently sets both the ident field and user-data pointer to 0 or 1 for the event pipe and input queue respectively, to tell these sources apart. While it works to reinterpret this data pointer as integer identifier it shouldn't be abused for that, in particular when one may wish to provide extra information with an event in the future; then the data field is used as pointer (or abused as abstract value) for that.


These constants (0 and 1) are internal to ndk-glue and not publicly documented anywhere (afaik). I believe they should at least be exposed as constants, or perhaps abstracted behind a poll function internal to ndk-glue. Consequently we might want to warn API users when they try use one of these "reserved idents" (but only on the thread looper set up by ndk-glues init() function)?

CC @dvc94ch @msiglreith

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

ndk-glue currently sets both the `ident` field and user-data pointer to
`0` or `1` for the event pipe and input queue respectively, to tell
these sources apart. While it works to reinterpret this `data` pointer
as integer identifier it shouldn't be abused for that, in particular
when one may wish to provide extra information with an event in the
future; then the `data` field is used as pointer (or abused as abstract
value) for that.
Copy link
Member

@dvc94ch dvc94ch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for all your work on android

@dvc94ch dvc94ch merged commit 05fe983 into rust-windowing:master Jan 13, 2021
@MarijnS95 MarijnS95 deleted the poll-use-identifier branch January 13, 2021 22:13
MarijnS95 added a commit to MarijnS95/winit that referenced this pull request Jan 30, 2021
Following the changes in [1] this bumps ndk and ndk-glue to 0.3 and uses
the new constants. The minor version has been bumped to prevent
applications from running an older winit (without rust-windowing#1826) with a newer
ndk/ndk-glue that does not pass this `ident` through the `data` pointer
anymore.

[1]: rust-mobile/ndk#112
MarijnS95 added a commit that referenced this pull request Jan 30, 2021
…1847)

Following the changes in [1] this bumps ndk and ndk-glue to 0.3 and uses
the new constants. The minor version has been bumped to prevent
applications from running an older winit (without #1826) with a newer
ndk/ndk-glue that does not pass this `ident` through the `data` pointer
anymore.

[1]: rust-mobile/ndk#112
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants