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

adds support for fn set_suspend_callback #2297

Closed
wants to merge 1 commit into from

Conversation

lattice0
Copy link

Fix for rust-windowing/glutin#1307 but I still don´t know what to put into the implementation of set_suspend_callback. It looks like this repo had this but now does not have it anymore, and I couldn't find the implementation for it. What should I put there? At least it compiles now

  • Tested on all platforms changed
  • 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

@MarijnS95
Copy link
Member

but I still don´t know what to put into the implementation of set_suspend_callback. It looks like this repo had this but now does not have it anymore

I would not be surprised if it has simply been replaced with the Suspended and Resumed functions, which glutin should start listening for.

Having not looked at the glutin codebase yet, I presume it uses this to stop using the window surface when Android drops the NativeWindow (the Suspended event).

#2118 tried to handle different Android lifecycle events. It'd be nice to have these available at some point, but they're not mandatory for proper window management unlike the (badly named, IMO) Suspended and Resumed event. In my eyes the Resumed event should provide a RawWindowHandle to help users utilize the API in the same way across all windowing platforms, even if it's "always available" elsewhere.

@lattice0
Copy link
Author

do you think I should just drop this then?

It was meant to fix https://github.com/rust-windowing/glutin/blob/bab33a84dfb094ff65c059400bed7993434638e2/glutin/src/api/android/mod.rs#L76 but then I don't know there how could I handle the Resume and Suspend. I only have access to an https://docs.rs/winit/latest/winit/event_loop/struct.EventLoopWindowTarget.html which does not seem to have calls for that.

PS: I know nothing about windowing, just trying to do some OpenGL stuff from Flutter but it ended up requiring stuff from these libs

@lattice0 lattice0 changed the title android fix adds support for fn set_suspend_callback May 21, 2022
@MarijnS95
Copy link
Member

MarijnS95 commented May 21, 2022

do you think I should just drop this then?

Yes, because:

It was meant to fix https://github.com/rust-windowing/glutin/blob/bab33a84dfb094ff65c059400bed7993434638e2/glutin/src/api/android/mod.rs#L76 but then I don't know there how could I handle the Resume and Suspend

That's exactly what I expect in a Suspended and Resumed handler. I'd have to look at the glutin implementation to see where that'd fit in, it should be called from where glutin handles EventLoop.run().

PS: I know nothing about windowing, just trying to do some OpenGL stuff from Flutter but it ended up requiring stuff from these libs

I don't know too much about Flutter but afaik it handles the Android window itself, so you shouldn't use something like glutin and winit which also subsume unique ownership of a full window. Instead all you might need it Surface/SurfaceTexture interop which has recently been discussed around various rust-windowing repositories.

(But it's nice to finally fix glutin regardless, there have been many reports of this compiler error but the crate seems to have been abandoned from at least an Android perspective)

@lattice0
Copy link
Author

I'm using glium for opengl stuff, and gonna try to render to a SurfaceTexture. I have everything setted up, but this compiling error was preventing me from getting things done. So no windowing, but still gotta fix it.

So should I just remove the glutin call el.set_suspend_callback(Some(Box::new(move |suspended| { on the PR? I'm lost on whether things have to be dealt on that function call of elsewhere

@MarijnS95 MarijnS95 added DS - android C - needs discussion Direction must be ironed out labels May 26, 2022
@MarijnS95
Copy link
Member

MarijnS95 commented May 26, 2022

So should I just remove the glutin call el.set_suspend_callback(Some(Box::new(move |suspended| { on the PR? I'm lost on whether things have to be dealt on that function call of elsewhere

Yes. I hope you don't mind but I've started taking things from here. We'll merge rust-windowing/glutin#1398 which fixes some compiler errors, then on top figure out how to best handle the Suspended/Resumed events in glutin - which supersede this set_suspend_callback() functionality. This PR anyway doesn't provide an implementation for that function.

For the time being we recreate the entire glutin OpenGL context + surface, see the working example here: https://github.com/MarijnS95/glutin/commits/wr

@MarijnS95 MarijnS95 closed this May 26, 2022
@lattice0
Copy link
Author

No problem, thank you so much. I just want to have this stuff working, one way or another :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out DS - android
Development

Successfully merging this pull request may close these issues.

2 participants