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

Postponed: Experimentally render to an Android surface #99

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

maxammann
Copy link
Collaborator

Thanks @MarijnS95 for making this possible! This PR experimentally renders to a surface on android: rust-mobile/ndk#272

💻 Examples

🚨 Test instructions

✔️ PR Todo

  • Included links to related issues/PRs

@maxammann maxammann force-pushed the experiement-surface-android branch from a889831 to 7b6ca43 Compare May 13, 2022 09:53
Comment on lines +24 to +30
unsafe impl raw_window_handle::HasRawWindowHandle for AndroidNativeWindow {
fn raw_window_handle(&self) -> RawWindowHandle {
let mut handle = AndroidNdkHandle::empty();
handle.a_native_window = unsafe { self.window.ptr().as_mut() as *mut _ as *mut _ };
RawWindowHandle::AndroidNdk(handle)
}
}

Choose a reason for hiding this comment

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

Yes, let me add the bindings for this one directly on ndk::NativeWindow 🎉

Choose a reason for hiding this comment

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

rust-mobile/ndk#274 Hopefully this simplifies things a bit for you, too :)

@MarijnS95
Copy link

Whoops!

Updating git repository `[https://github.com/rust-windowing/android-ndk-rs.git`](https://github.com/rust-windowing/android-ndk-rs.git%60)
error: failed to get `ndk` as a dependency of package `maplibre-android v0.1.0 (/home/runner/work/maplibre-rs/maplibre-rs/android)`

Caused by:
  failed to load source for dependency `ndk`

Caused by:
  Unable to update https://github.com/rust-windowing/android-ndk-rs.git?branch=native-window-jni-surface

Caused by:
  failed to find branch `native-window-jni-surface`

Caused by:
  cannot locate remote-tracking branch 'origin/native-window-jni-surface'; class=Reference (4); code=NotFound (-3)

The branch disappeared because the PR - and the one adding HasRawWindowHandle - have both been merged 🎉! You should be able to use a rev from master now which won't disappear.

@maxammann
Copy link
Collaborator Author

Whoops!

Updating git repository `[https://github.com/rust-windowing/android-ndk-rs.git`](https://github.com/rust-windowing/android-ndk-rs.git%60)
error: failed to get `ndk` as a dependency of package `maplibre-android v0.1.0 (/home/runner/work/maplibre-rs/maplibre-rs/android)`

Caused by:
  failed to load source for dependency `ndk`

Caused by:
  Unable to update https://github.com/rust-windowing/android-ndk-rs.git?branch=native-window-jni-surface

Caused by:
  failed to find branch `native-window-jni-surface`

Caused by:
  cannot locate remote-tracking branch 'origin/native-window-jni-surface'; class=Reference (4); code=NotFound (-3)

The branch disappeared because the PR - and the one adding HasRawWindowHandle - have both been merged tada! You should be able to use a rev from master now which won't disappear.

Thank you! I think I'll actually wait with this until a new release is deployed :) else we can not publish to crates.io :)

@MarijnS95
Copy link

That'll take a while... :(

@MarijnS95
Copy link

Pinging back to #53, I couldn't find this PR from there :)

Comment on lines +57 to +76
libc::pipe(logpipe.as_mut_ptr());
libc::dup2(logpipe[1], libc::STDOUT_FILENO);
libc::dup2(logpipe[1], libc::STDERR_FILENO);
thread::spawn(move || {
let tag = CStr::from_bytes_with_nul(b"MapLibreStderr\0").unwrap();
let file = File::from_raw_fd(logpipe[0]);
let mut reader = BufReader::new(file);
let mut buffer = String::new();
loop {
buffer.clear();
if let Ok(len) = reader.read_line(&mut buffer) {
if len == 0 {
break;
} else if let Ok(msg) = CString::new(buffer.clone()) {
ndk_glue::android_log(Level::Info, tag, &msg);
}
}
}
});
}

Choose a reason for hiding this comment

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

CC rust-mobile/ndk#281, perhaps we should move this out of ndk_glue and make it generic :)

Perhaps some unification with the android_logger crate too.

@maxammann
Copy link
Collaborator Author

That'll take a while... :(

Everything below 1-2 months is fine I think :) this poc is enough for now. It wont be usable until input is working for android

@MarijnS95
Copy link

I don't know what the cadence for winit is going to be, which I want to synchronize the next breaking release with. We could also have a breaking release earlier if folks are actively blocking on it, but I've previously received complaints about doing lots of breaking releases in short succession (for usually minor breaking changes in isolated parts of a crate). In this specific case I'm planning ahead some breaking changes that affect both winit and ndk so I'd like to prevent doing a release before that is over (pending exactly another breaking release in short succession). In addition people like to keep their stuff up to date, and it's annoying if they see a new ndk update but then can't upgrade because that breaks or results in duplicate dependencies with winit.

But again, let me know if/when you start depending on a release and we'll see what we can do!

@maxammann
Copy link
Collaborator Author

Alright! So I think right now its no problem to depend on main as we have not even an unstable release. I'll continue to work on this with very low priority though as the changes are quite isolated.

# Conflicts:
#	maplibre-winit/src/winit/mod.rs
#	maplibre/src/window.rs
@maxammann maxammann force-pushed the experiement-surface-android branch from 91b59d0 to 5dbff6d Compare June 2, 2022 14:41
@maxammann maxammann force-pushed the main branch 3 times, most recently from 8c5d8ee to 4dbd47b Compare June 3, 2022 09:54
@maxammann maxammann changed the title Experimentally render to an Android surface Postphoned until needed:Experimentally render to an Android surface Nov 12, 2022
@maxammann maxammann changed the title Postphoned until needed:Experimentally render to an Android surface Postphoned until needed: Experimentally render to an Android surface Nov 12, 2022
@maxammann maxammann changed the title Postphoned until needed: Experimentally render to an Android surface Postponed: Experimentally render to an Android surface Nov 19, 2022
@maxammann maxammann added the pr-postponed Postponed PR, which is not needed right now but might be needed (contains valuable information) label Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-postponed Postponed PR, which is not needed right now but might be needed (contains valuable information)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants