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

egui_winit/wgpu: enable Android support #1634

Merged
merged 2 commits into from
May 22, 2022

Conversation

rib
Copy link
Contributor

@rib rib commented May 17, 2022

I've recently been looking at using egui to create an Android app that I can use to test a Bluetooth library that I'm building but I found that it would panic due to some assumptions that it's possible to access a native window for initializing render + surface state while the application is paused.

This implements the proposal in #1633 which enables graphics state to be lazily initialized on Android when the application is Resumed (at which point it has a valid native window) as well as support dropping/recreating surface state each time the application is paused/resumed on Android.

Since I'm currently using egui_winit and egui_wgpu directly this PR only focuses on enabling Android support via these APIs. The corresponding epi/glium changes adapt to the egui_wini/wgpu API changes but have some remaining assumptions about when a window is valid which means they would require some further work to add Android support.

Closes #1633.

rib added a commit to rust-mobile/android-activity that referenced this pull request May 17, 2022
The functionality found in egui_winit_platform and egui_wgpu_backend is
now available in the upstream egui-winit and egui-wgpu crates
respectively.

This has simplified the example itself and also removed the dependency
on epi.

The example is now based on egui 0.18 (was previously 0.17)

Since the upstream egui-winit/wgpu crates were generally assuming that
the graphics context and surface can be initialized immediately this
currently builds against a branch that aims to upstream changes that
remove these assumptions:

https://github.com/rib/egui/tree/android-deferred-winit-wgpu
Ref: emilk/egui#1634
@lunixbochs
Copy link
Contributor

If we're changing the wgpu constructor, can we maybe go a step further and allow the user to fully set wgpu::Backends, wgpu::RequestAdapterOptions and wgpu::DeviceDescriptor?

@rib
Copy link
Contributor Author

rib commented May 20, 2022

yeah, sounds good, it was fairly arbitrary that I just exposed the power preference and present mode

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Thanks for working on Android support!

I assume the FIXME:s in the code need fixin' before mergin'?

eframe/Cargo.toml Outdated Show resolved Hide resolved
egui-wgpu/CHANGELOG.md Outdated Show resolved Hide resolved
egui-wgpu/src/winit.rs Outdated Show resolved Hide resolved
egui-wgpu/CHANGELOG.md Outdated Show resolved Hide resolved
egui-winit/CHANGELOG.md Outdated Show resolved Hide resolved
@rib
Copy link
Contributor Author

rib commented May 21, 2022

Thanks for working on Android support!

I assume the FIXME:s in the code need fixin' before mergin'?

In this case I added the FiXMEs to code where I was maintaining status quo (where it wont currently work on Android) but I had to make integration changes to use the updated egui_wgpu and egui_winit API.

My intent was to just focus first on supporting Android better via egui_winit + egui_wgpu directly (i'm not cuurently using eframe) but I figured it was worth highlighting where the current integration details for epi / eframe aren't portable to Android.

I can try to take another pass later to also support Android via epi / eframe by addressing the FIXMEs, but would it be ok perhaps to leave them for this PR? Or perhaps just remove for cleaner aesthetics?

@rib rib force-pushed the android-deferred-winit-wgpu branch from 3c60969 to bce303f Compare May 22, 2022 00:04
@rib
Copy link
Contributor Author

rib commented May 22, 2022

If we're changing the wgpu constructor, can we maybe go a step further and allow the user to fully set wgpu::Backends, wgpu::RequestAdapterOptions and wgpu::DeviceDescriptor?

With the updated patches I've just pushed the Painter::new() now takes a wgpu::Backends mask and wgpu::DeviceDescriptor struct and but not wgpu::RequestAdapterOption. The adapter options struct has a lifetime that's normally associated with the surface that can be given, which is not expected to be available when calling Painter::new(). It still accepts the wgpu::PowerPreference so it's currently just the force_fallback_adapter adapter option that's not exposed.

@rib rib requested a review from emilk May 22, 2022 00:15
egui-wgpu/src/winit.rs Outdated Show resolved Hide resolved
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Only a CI doc failure and a merge conflict that need to be fixed

rib added 2 commits May 22, 2022 18:37
On Android in particular we can only initialize render state once we
have a native window, after a 'Resumed' lifecycle event. It's still
practical to be able to initialize an egui_winit::State early on
so this adds setters for the max_texture_side and pixels_per_point
that can be called once we have a valid Window and have initialized
a graphics context.

On Wayland, where we need to access the Display for clipboard handling
we now get the Display from the event loop instead of a window.
Enable the renderer and surface state initialization to be deferred
until we know that any winit window we created has a valid native window
and enable the surface state to be updated in case the native window
changes.

In particular these changes help with running on Android where winit
windows will only have a valid native window associated with them
between Resumed and Paused lifecycle events, and so surface creation
(and render state initialization) needs to wait until the first
Resumed event, and the surface needs to be dropped/recreated based on
Paused/Resumed events.
@rib rib force-pushed the android-deferred-winit-wgpu branch from bce303f to e75ac2b Compare May 22, 2022 17:49
@rib rib requested a review from emilk May 22, 2022 17:53
@emilk emilk merged commit a5076d4 into emilk:master May 22, 2022
rib added a commit to rust-mobile/rust-android-examples that referenced this pull request Nov 17, 2022
The functionality found in egui_winit_platform and egui_wgpu_backend is
now available in the upstream egui-winit and egui-wgpu crates
respectively.

This has simplified the example itself and also removed the dependency
on epi.

The example is now based on egui 0.18 (was previously 0.17)

Since the upstream egui-winit/wgpu crates were generally assuming that
the graphics context and surface can be initialized immediately this
currently builds against a branch that aims to upstream changes that
remove these assumptions:

https://github.com/rib/egui/tree/android-deferred-winit-wgpu
Ref: emilk/egui#1634
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android Support
3 participants