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/input_queue: Replace InputQueueError with std::io::Error and add missing docs #292

Merged
merged 1 commit into from
Jun 12, 2022

Conversation

MarijnS95
Copy link
Member

These AInputQueue functions all return standard errno.h error codes which can and should be communicated back to the user instead of appearing as some opaque magic struct.

In addition import the NDK documentation for all these functions and make the scope of unsafe blocks smaller.

@MarijnS95 MarijnS95 added blocking a release Prevents a new release difficulty: easy Likely easier than most tasks here status: needs review A maintainer must review this code labels Jun 11, 2022
@MarijnS95 MarijnS95 force-pushed the ndk-input-queue-error-and-docs branch from 6f9a5f0 to 145c328 Compare June 11, 2022 11:05
ndk/src/input_queue.rs Outdated Show resolved Hide resolved
ndk/src/input_queue.rs Show resolved Hide resolved
@MarijnS95 MarijnS95 force-pushed the ndk-input-queue-error-and-docs branch 4 times, most recently from f2d3d6b to 36b2ccf Compare June 11, 2022 21:53
…add missing docs

These `AInputQueue` functions all return [standard `errno.h` error
codes] which can and should be communicated back to the user instead of
appearing as some opaque magic struct.

In addition import the NDK documentation for all these functions and
make the scope of `unsafe` blocks smaller.

[standard `errno.h` error codes]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputQueue.cpp;l=112;drc=7b3193e1e86ab5c0fcb784cb8616864045dd2515
@MarijnS95 MarijnS95 force-pushed the ndk-input-queue-error-and-docs branch from 36b2ccf to e765038 Compare June 11, 2022 21:54
@MarijnS95 MarijnS95 requested a review from msiglreith June 11, 2022 22:56
@MarijnS95 MarijnS95 merged commit 51f5fb0 into master Jun 12, 2022
@MarijnS95 MarijnS95 deleted the ndk-input-queue-error-and-docs branch June 12, 2022 14:10
MarijnS95 added a commit that referenced this pull request Jun 12, 2022
After having blindly retained error handling in [#292], it turns out the
underlying [`InputQueue::hasEvents()` implementation] doesn't ever
return an error nor does the documentation state that this is possible.

As such, mimic what we're already doing in `fn pre_dispatch()` and panic
if the value doesn't represent a `bool`.

[`InputQueue::hasEvents()` implementation]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputQueue.cpp;l=88-91;drc=5497dd365e9135805636a40267cb98e2e2b78ee6
MarijnS95 added a commit that referenced this pull request Jun 12, 2022
After having blindly retained error handling in [#292], it turns out the
underlying [`InputQueue::hasEvents()` implementation] doesn't ever
return an error nor does the documentation state that this is possible.

As such, mimic what we're already doing in `fn pre_dispatch()` and panic
if the value doesn't represent a `bool`.

[`InputQueue::hasEvents()` implementation]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputQueue.cpp;l=88-91;drc=5497dd365e9135805636a40267cb98e2e2b78ee6
MarijnS95 added a commit that referenced this pull request Jun 12, 2022
…helper conversion function

These `AHardwareBuffer` functions all return [standard `errno.h` error
codes] which can be communicated back to the user in ergonomic form
instead of appearing as a newtyped struct holding a "meaningless" `i32`.

Perform the same conversion to just `NativeWindow::set_buffers_geometry`
which also returns `errno.h` codes: this change is small enough to not
warrant _yet another_ PR, as we've already changed the exact same for
`InputQueueError` in [#292].
(Note that this is intentionally omitted from the changelog since
 `set_buffers_geometry()` is only making its debut in the upcoming
 release, this change isn't breaking).

This pattern is now prevalent across the codebase (`hardware_buffer`,
`surface_texture`, and `native_window` all use it), warranting the new
`status_to_io_result()` helper function to be introduced in an also-new
`utils.rs` module.

[standard `errno.h` error codes]: https://cs.android.com/android/platform/superproject/+/master:frameworks/native/libs/nativewindow/AHardwareBuffer.cpp;drc=4120991d92a0265ce480672f704612556328057e
[#292]: #292
MarijnS95 added a commit that referenced this pull request Jul 5, 2022
After having blindly retained error handling in [#292], it turns out the
underlying [`InputQueue::hasEvents()` implementation] doesn't ever
return an error nor does the documentation state that this is possible.

As such, mimic what we're already doing in `fn pre_dispatch()` and panic
if the value doesn't represent a `bool`.

[`InputQueue::hasEvents()` implementation]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputQueue.cpp;l=88-91;drc=5497dd365e9135805636a40267cb98e2e2b78ee6
[#292]: #292
MarijnS95 added a commit that referenced this pull request Jul 5, 2022
After having blindly retained error handling in [#292], it turns out the
underlying [`InputQueue::hasEvents()` implementation] doesn't ever
return an error nor does the documentation state that this is possible.

As such, mimic what we're already doing in `fn pre_dispatch()` and panic
if the value doesn't represent a `bool`.

[`InputQueue::hasEvents()` implementation]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputQueue.cpp;l=88-91;drc=5497dd365e9135805636a40267cb98e2e2b78ee6
[#292]: #292
MarijnS95 added a commit that referenced this pull request Jul 5, 2022
…helper conversion function

These `AHardwareBuffer` functions all return [standard `errno.h` error
codes] which can be communicated back to the user in ergonomic form
instead of appearing as a newtyped struct holding a "meaningless" `i32`.

Perform the same conversion to just `NativeWindow::set_buffers_geometry`
which also returns `errno.h` codes: this change is small enough to not
warrant _yet another_ PR, as we've already changed the exact same for
`InputQueueError` in [#292].
(Note that this is intentionally omitted from the changelog since
 `set_buffers_geometry()` is only making its debut in the upcoming
 release, this change isn't breaking).

This pattern is now prevalent across the codebase (`hardware_buffer`,
`surface_texture`, and `native_window` all use it), warranting the new
`status_to_io_result()` helper function to be introduced in an also-new
`utils.rs` module.

[standard `errno.h` error codes]: https://cs.android.com/android/platform/superproject/+/master:frameworks/native/libs/nativewindow/AHardwareBuffer.cpp;drc=4120991d92a0265ce480672f704612556328057e
[#292]: #292
MarijnS95 added a commit that referenced this pull request Jul 5, 2022
…helper conversion function (#295)

These `AHardwareBuffer` functions all return [standard `errno.h` error
codes] which can be communicated back to the user in ergonomic form
instead of appearing as a newtyped struct holding a "meaningless" `i32`.

Perform the same conversion to just `NativeWindow::set_buffers_geometry`
which also returns `errno.h` codes: this change is small enough to not
warrant _yet another_ PR, as we've already changed the exact same for
`InputQueueError` in [#292].
(Note that this is intentionally omitted from the changelog since
 `set_buffers_geometry()` is only making its debut in the upcoming
 release, this change isn't breaking).

This pattern is now prevalent across the codebase (`hardware_buffer`,
`surface_texture`, and `native_window` all use it), warranting the new
`status_to_io_result()` helper function to be introduced in an also-new
`utils.rs` module.

[standard `errno.h` error codes]: https://cs.android.com/android/platform/superproject/+/master:frameworks/native/libs/nativewindow/AHardwareBuffer.cpp;drc=4120991d92a0265ce480672f704612556328057e
[#292]: #292
rib pushed a commit to rust-mobile/ndk-glue that referenced this pull request Dec 6, 2022
After having blindly retained error handling in [#292], it turns out the
underlying [`InputQueue::hasEvents()` implementation] doesn't ever
return an error nor does the documentation state that this is possible.

As such, mimic what we're already doing in `fn pre_dispatch()` and panic
if the value doesn't represent a `bool`.

[`InputQueue::hasEvents()` implementation]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputQueue.cpp;l=88-91;drc=5497dd365e9135805636a40267cb98e2e2b78ee6
[#292]: rust-mobile/ndk#292
rib pushed a commit to rust-mobile/ndk-context that referenced this pull request Dec 7, 2022
After having blindly retained error handling in [#292], it turns out the
underlying [`InputQueue::hasEvents()` implementation] doesn't ever
return an error nor does the documentation state that this is possible.

As such, mimic what we're already doing in `fn pre_dispatch()` and panic
if the value doesn't represent a `bool`.

[`InputQueue::hasEvents()` implementation]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputQueue.cpp;l=88-91;drc=5497dd365e9135805636a40267cb98e2e2b78ee6
[#292]: rust-mobile/ndk#292
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking a release Prevents a new release difficulty: easy Likely easier than most tasks here status: needs review A maintainer must review this code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants