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

Re-implement PaintCallbacks With Support for WGPU #1684

Merged
merged 5 commits into from May 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,10 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui-w
* Added opt-in feature `deadlock_detection` to detect double-lock of mutexes on the same thread ([#1619](https://github.com/emilk/egui/pull/1619)).
* Added `InputState::stable_dt`: a more stable estimate for the delta-time in reactive mode ([#1625](https://github.com/emilk/egui/pull/1625)).
* You can now specify a texture filter for your textures ([#1636](https://github.com/emilk/egui/pull/1636)).
* Added support for using `PaintCallback` shapes with the WGPU backend ([#1684](https://github.com/emilk/egui/pull/1684))

### Changed
* `PaintCallback` shapes now require the whole callback to be put in an `Arc<dyn Any>` with the value being a backend-specific callback type. ([#1684](https://github.com/emilk/egui/pull/1684))

### Fixed 🐛
* Fixed `ImageButton`'s changing background padding on hover ([#1595](https://github.com/emilk/egui/pull/1595)).
Expand Down
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions eframe/src/epi.rs
Expand Up @@ -29,6 +29,11 @@ pub struct CreationContext<'s> {
/// you might want to use later from a [`egui::PaintCallback`].
#[cfg(feature = "glow")]
pub gl: Option<std::sync::Arc<glow::Context>>,

/// Can be used to manage GPU resources for custom rendering with WGPU using
/// [`egui::PaintCallback`]s.
#[cfg(feature = "wgpu")]
pub render_state: Option<egui_wgpu::RenderState>,
}

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -335,6 +340,11 @@ pub struct Frame {
#[cfg(feature = "glow")]
#[doc(hidden)]
pub gl: Option<std::sync::Arc<glow::Context>>,

/// Can be used to manage GPU resources for custom rendering with WGPU using
/// [`egui::PaintCallback`]s.
#[cfg(feature = "wgpu")]
pub render_state: Option<egui_wgpu::RenderState>,
}

impl Frame {
Expand Down
3 changes: 3 additions & 0 deletions eframe/src/lib.rs
Expand Up @@ -61,6 +61,9 @@ pub use {egui, egui::emath, egui::epaint};
#[cfg(feature = "glow")]
pub use {egui_glow, glow};

#[cfg(feature = "wgpu")]
pub use {egui_wgpu, wgpu};

mod epi;

// Re-export everything in `epi` so `eframe` users don't have to care about what `epi` is:
Expand Down
3 changes: 3 additions & 0 deletions eframe/src/native/epi_integration.rs
Expand Up @@ -188,6 +188,7 @@ impl EpiIntegration {
window: &winit::window::Window,
storage: Option<Box<dyn epi::Storage>>,
#[cfg(feature = "glow")] gl: Option<std::sync::Arc<glow::Context>>,
#[cfg(feature = "wgpu")] render_state: Option<egui_wgpu::RenderState>,
) -> Self {
let egui_ctx = egui::Context::default();

Expand All @@ -207,6 +208,8 @@ impl EpiIntegration {
storage,
#[cfg(feature = "glow")]
gl,
#[cfg(feature = "wgpu")]
render_state,
};

if prefer_dark_mode == Some(true) {
Expand Down
8 changes: 8 additions & 0 deletions eframe/src/native/run.rs
Expand Up @@ -62,6 +62,8 @@ pub fn run_glow(
gl_window.window(),
storage,
Some(gl.clone()),
#[cfg(feature = "wgpu")]
None,
);

{
Expand All @@ -76,6 +78,8 @@ pub fn run_glow(
integration_info: integration.frame.info(),
storage: integration.frame.storage(),
gl: Some(gl.clone()),
#[cfg(feature = "wgpu")]
render_state: None,
});

if app.warm_up_enabled() {
Expand Down Expand Up @@ -230,13 +234,16 @@ pub fn run_wgpu(
painter
};

let render_state = painter.get_render_state().expect("Uninitialized");
Copy link
Contributor

Choose a reason for hiding this comment

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

This always panics on Android, so this breaks the support added in #1634.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well that's not cool! I don't have an Android to test on unfortunately.

Still, I did try to build it for Android real quick. A quick look over and, it appears that there is multiple things breaking the ability to build eframe for android right now, that isn't related to this PR. It looks like #1634 might not have actually gotten eframe building for Android, just egui_wgpu and egui_winit.

Some things I noticed need to be fixed for eframe will build for Android:

  • The inclusion of the arboard crate for clipboard support needs to be made optional, because it doesn't build for android.
  • Same fix for the glutin dependency
  • It appears that the winit::event::Event::Paused event doesn't exist, but it's used in the android event loop code.

For what I can tell, it looks like the eframe crate hasn't ever actually worked for Android yet, but it might not be a big deal to make work. I'm not sure if I'll be able to get the time to try to get it working yet, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have things half working on Android. Buttons show up, but no text.

Paused is called Suspended now. glutin master works, and there's an open PR to fix arboard.

It would probably be a good idea to add cargo check --target aarch64-linux-android to CI to avoid build errors at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool! We shouldn't actually need glutin anyway if we are using the WGPU backend. I'm not sure why text wouldn't be working.

Copy link
Owner

Choose a reason for hiding this comment

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

Nice to hear some progress on this! Feel free to open a PR to add cargo check --target aarch64-linux-android to CI, and any other needed changes


let mut integration = epi_integration::EpiIntegration::new(
&event_loop,
painter.max_texture_side().unwrap_or(2048),
&window,
storage,
#[cfg(feature = "glow")]
None,
Some(render_state.clone()),
);

{
Expand All @@ -252,6 +259,7 @@ pub fn run_wgpu(
storage: integration.frame.storage(),
#[cfg(feature = "glow")]
gl: None,
render_state: Some(render_state),
});

if app.warm_up_enabled() {
Expand Down
4 changes: 4 additions & 0 deletions eframe/src/web/backend.rs
Expand Up @@ -170,6 +170,8 @@ impl AppRunner {
storage: Some(&storage),
#[cfg(feature = "glow")]
gl: Some(painter.painter.gl().clone()),
#[cfg(feature = "wgpu")]
render_state: None,
});

let frame = epi::Frame {
Expand All @@ -178,6 +180,8 @@ impl AppRunner {
storage: Some(Box::new(storage)),
#[cfg(feature = "glow")]
gl: Some(painter.gl().clone()),
#[cfg(feature = "wgpu")]
render_state: None,
};

let needs_repaint: std::sync::Arc<NeedRepaint> = Default::default();
Expand Down
1 change: 1 addition & 0 deletions egui-wgpu/Cargo.toml
Expand Up @@ -37,6 +37,7 @@ egui = { version = "0.18.1", path = "../egui", default-features = false, feature

bytemuck = "1.7"
tracing = "0.1"
type-map = "0.5.0"
wgpu = { version = "0.12", features = ["webgl"] }

# Optional:
Expand Down
3 changes: 3 additions & 0 deletions egui-wgpu/src/lib.rs
Expand Up @@ -6,7 +6,10 @@ pub use wgpu;

/// Low-level painting of [`egui`] on [`wgpu`].
pub mod renderer;
pub use renderer::CallbackFn;

/// Module for painting [`egui`] with [`wgpu`] on [`winit`].
#[cfg(feature = "winit")]
pub mod winit;
#[cfg(feature = "winit")]
pub use crate::winit::RenderState;