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

Add Android example #1417

Merged
merged 3 commits into from Oct 4, 2022
Merged

Add Android example #1417

merged 3 commits into from Oct 4, 2022

Conversation

MarijnS95
Copy link
Member

As mentioned in #1398/#1411 we should have an example that showcases how to deal with the Android activity lifecycle. Ultimately this turns into a helper function on ContextWrapper that has access to the internal surface for recreation, as did the broken implementation prior to #1411. At the same time I hope to replace new_windowed() with an implementation soon™ that takes a RawWindowHandle, making the code more generic, winit-agnostic, and allows users to use Android Surfaces for specific views in their Java/Kotlin/Flutter apps without needing a fullscreen NativeActivity.

This PR is based on top of #1412 so that my editor doesn't go bonkers on all the lint warnings/errors :)

glutin/src/api/egl/mod.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@kchibisov
Copy link
Member

This would need update to the latest Api.

@MarijnS95
Copy link
Member Author

@kchibisov Sure, I've just returned from holiday and am slowly getting back into things, here's hoping we can use this Android example to finally vet the API to be compatible/sensible in this regard.

Pieced together something that compiles at least, but running into thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: Error { raw_code: None, raw_os_message: None, kind: BadConfig }', glutin_examples/examples/android.rs:54:73, will throw some more time at this when it's light outside again.

@kchibisov
Copy link
Member

kchibisov commented Sep 6, 2022

Pieced together something that compiles at least, but running into thread '' panicked at 'called Result::unwrap() on an Err value: Error { raw_code: None, raw_os_message: None, kind: BadConfig }', glutin_examples/examples/android.rs:54:73, will throw some more time at this when it's light outside again.

You should ask for GLES I guess in the template builder. In general we should likely remove that requirement...

In a config builder try .with_api(Api::GLES2).

@MarijnS95
Copy link
Member Author

@kchibisov Right, yes, I also had to add .with_context_api(glutin::context::ContextApi::Gles(None)) (not sure why we have two places for this, I'm not too familiar with the GL(ES) startup/setup), it's rendering (clearing) a teal background now.

@kchibisov
Copy link
Member

kchibisov commented Sep 6, 2022

@MarijnS95 could we use the same example as for desktop, but for android as well? I'll look into adjusting api wrt gles picking in that regard.

@MarijnS95
Copy link
Member Author

@kchibisov That'd be nice, I copy-pasted the example from yours so it's almost identical, #[cfg_attr(target_os = "android", ndk_glue::main(backtrace = "on"))] already makes this a generic fn main() and Resumed is emitted on every platform now so we might get away with a single file. crate-type = ["cdylib"] will throw a spanner in the works, though.

@kchibisov
Copy link
Member

kchibisov commented Sep 6, 2022

Yeah, if we can have a single example for all platforms it could be better? But I'm not entirely sure? Maybe having a separate example for android works as well...

@MarijnS95
Copy link
Member Author

@kchibisov I think we can, we already have the glutin_example crate-library provide support; the more code we move in there the smaller / less overlap remains in the desktop-runnable binary and Android-runnable library (apk).

@kchibisov
Copy link
Member

You should rebase and fix the CI. Be aware that formatting is on nightly.

@MarijnS95
Copy link
Member Author

I'm aware, and was just waiting for the dependent PR to be merged before reevaluating where to take this.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

I'd suggest you to create context out side of run. We should advertise the best we can here. The problem is symbol loading, I guess? I wonder if we can make context current, but without any surface? Make add Api for that or something to help here.

glutin_examples/examples/android.rs Outdated Show resolved Hide resolved
glutin_examples/examples/android.rs Outdated Show resolved Hide resolved
glutin_examples/examples/android.rs Outdated Show resolved Hide resolved
glutin_examples/examples/android.rs Outdated Show resolved Hide resolved
@kchibisov
Copy link
Member

I wonder if we can make context current, but without any surface? Make add Api for that or something to help here.

Ah, we can, but it's not guaranteed to work. The issue I was trying to solve is gl functions loading, since I was confused why we need context to be current for that. And it turned out that WGL needs that to perform ICD loading. In general we don't need that if we use RawWindowHandle but we do support rendering without passing RawWindowHandle at all. This seems very strange to be fair, and I think that we should force RawWindowHandle with WGL.

Though, we can provide a method to get proc address from not current context on supported platforms.

Do you have any idea how to approach the symbol loading here and WGL situation?

@MarijnS95
Copy link
Member Author

I'd suggest you to create context out side of run. We should advertise the best we can here. The problem is symbol loading, I guess? I wonder if we can make context current, but without any surface? Make add Api for that or something to help here.

It seems you figured out how to clean this up on the glutin API side in #1460 now, I'll rebase and see where we end up.

@kchibisov
Copy link
Member

Now you should be able to reuse your context properly.

@MarijnS95
Copy link
Member Author

@kchibisov Having a little trouble with that, make_current() wants to move the NotCurrent context that I create outside of the run() loop, but it's not Copy nor Clone and I can only make it current inside Resumed where a Window is created.

I can create an enum that keeps either the PossiblyCurrent or NotCurrent context, and then just make it current once we have a Window?

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Could you not recreate the renderer on Resume? There's no need for that at all and it just slows you down. You also don't need to drop it.

@MarijnS95 MarijnS95 closed this Sep 20, 2022
@MarijnS95
Copy link
Member Author

@kchibisov It'd be nice if you actually read my questions before shooting this down immediately.

@kchibisov
Copy link
Member

I mean, I've read it. Nothing stops you from using hand rolled Lazy. I can send a patch if you struggle.

@kchibisov kchibisov reopened this Sep 20, 2022
@MarijnS95
Copy link
Member Author

I mean, I've read it.

Just asking what your design decision was here, and why the selected comment is incomplete.

Nothing stops you from using hand rolled Lazy.

Yawn.

I can send a patch if you struggle.

Pls halp I can't Rust /s

@kchibisov
Copy link
Member

You may base it on your recently merged PR now.

@MarijnS95 MarijnS95 force-pushed the android-example branch 3 times, most recently from d088baf to e2c3cc3 Compare September 21, 2022 09:22
@MarijnS95
Copy link
Member Author

@kchibisov Done, can you recheck that this still works on WGL, especially since WGL needs a window before creating a display according to your comments?

Comment on lines -90 to -91
// Make it current and load symbols.
let gl_context = gl_context.make_current(&gl_window.surface).unwrap();
Copy link
Member Author

Choose a reason for hiding this comment

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

@kchibisov If I understood things correctly make_current() now doesn't load any functions anymore, that's done by Renderer::new() right? Or does it load a limited set of functions?

@MarijnS95 MarijnS95 mentioned this pull request Sep 22, 2022
Make the shared event loop suitable for creating and destroying windows
in accordance with `winit`s `Resumed` and `Suspended` events.
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

This should be good to go once CI passes. I've updated the handling of X11 to be more robust given that it was proven that it's not really working.

Not sure if I want to update internal handling of X11 config picking, but might alter it to not consider raw-window-handle at all.

@kchibisov
Copy link
Member

Thanks.

@kchibisov kchibisov deleted the android-example branch October 4, 2022 07:44
@MarijnS95
Copy link
Member Author

@kchibisov Thank you for patching up the X11 situation and getting this in!

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.

None yet

3 participants