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: add request_redraw based on ndk_glue #1822

Closed
wants to merge 1 commit into from

Conversation

semtexzv
Copy link
Contributor

@semtexzv semtexzv commented Jan 9, 2021

  • 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

Fixes #1723, just picked up the code from there, seems to work fine

@msiglreith msiglreith added DS - android C - waiting on maintainer A maintainer must review this code labels Jan 13, 2021
@msiglreith msiglreith self-requested a review January 13, 2021 19:00
@MarijnS95
Copy link
Member

While this might work it is not all that elegant to go through the unsafe, native onNativeWindowRedrawNeeded callback that is really only supposed to be called by Android, but happens to be publicly available to users of ndk-glue.

Instead we might want to make the fn wake(Event) function public to push the event safely in through there (this is where the above native call ends up). Then however I don't think we want to make that function public, nor is this going to help distinguish where the event came from (may we ever need that).

Two options remain:

  1. Use looper.wake() and somehow finagle the "redraw requested' state somewhere, either in an atomic bool as suggested by @dvc94ch, maybe a more generic atomic integer/queue that can keep multiple events may we need it in the future, or stuff it into the existing user_queue that is already read when the event loop receives EventSource::User (perhaps by wrapping its items in another enum);
  2. Create a pipe (see ndk-glue code), attach that to the looper on a new ident, add a new EventSource that is returned from fn poll() when an event is retrieved on that pipe, then read that event from the pipe and set redraw = true.

Let's discuss with the maintainers here to find the best solution before deciding anything 😄. CC @msiglreith and @francesca64

@msiglreith
Copy link
Member

I would prefer the 1st option suggested by @MarijnS95

Use looper.wake() and somehow finagle the "redraw requested' state somewhere, either in an atomic bool as suggested by @dvc94ch, maybe a more generic atomic integer/queue that can keep multiple events may we need it in the future, or stuff it into the existing user_queue that is already read when the event loop receives EventSource::User (perhaps by wrapping its items in another enum);

Wrapping the current user event in an enum and a RedrawRequest would probably be straightforward as the parts are already in place. Immediately emitting the redraw request after the MainEventsCleared call seems more tricky, but could be a followup imo.

@msiglreith
Copy link
Member

Thanks, #1953 landed which covers the functionality from this PR

@msiglreith msiglreith closed this Jun 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - waiting on maintainer A maintainer must review this code DS - android
Development

Successfully merging this pull request may close these issues.

Android redraw requests aren't implemented
3 participants