-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Merged by Bors] - WebGL2 support #3039
Conversation
@@ -525,3 +529,22 @@ fn handle_create_window_events( | |||
}); | |||
} | |||
} | |||
|
|||
fn handle_initial_window_events(world: &mut World, event_loop: &EventLoop<()>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you do this in the winit_runner_with and keep the EventLoop argument of winit_runner_with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cart ? (it is your code :) But it seems handle_initial_window_events
needs to be called as is, because we need to have a window when renderer_plugin is initialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup sadly its an order of operations thing. We need to init these windows before the RenderPlugin is setup, not before the app starts running.
@@ -42,7 +42,7 @@ pub trait BevyDefault { | |||
|
|||
impl BevyDefault for wgpu::TextureFormat { | |||
fn bevy_default() -> Self { | |||
if cfg!(target_os = "android") { | |||
if cfg!(target_os = "android") || cfg!(target_arch = "wasm32") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use rgba everywhere? Are there any advantages to bgra on other platforms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember hearing that a lot of hardware only supports (or prefers) bgra swap chains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wgpu used to use bgra as a default in their examples, but it looks like at some point they switched to rgba. @kvark is there any reason to default to bgra at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point we can default to RGBA. Originally, we weren't sure if RGBA is really supported everywhere for presentation, as weird as it sounds.
Backends::PRIMARY | ||
} else { | ||
Backends::GL | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wgpu be changed to default to webgl on wasm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, it would be great to prefer webgpu over webgl if available (probably). I'll try to figure out how to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think its worth blocking on this, but I agree that its a "nice to have".
pipelined/bevy_render2/Cargo.toml
Outdated
@@ -29,7 +29,7 @@ bevy_utils = { path = "../../crates/bevy_utils", version = "0.5.0" } | |||
image = { version = "0.23.12", default-features = false } | |||
|
|||
# misc | |||
wgpu = { version = "0.11.0", features = ["spirv"] } | |||
wgpu = { version = "0.11.0", features = ["spirv", "webgl"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this feature be enabled only in wasm? That's possible now with edition 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably, taking a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just fixed this
Just pushed a change to only enable the wgpu webgl feature on wasm. I think this is good to go. Some follow ups to do:
|
bors r+ |
# Objective Make possible to use wgpu gles backend on in the browser (wasm32 + WebGL2). ## Solution It is built on top of old @cart patch initializing windows before wgpu. Also: - initializes wgpu with `Backends::GL` and proper `wgpu::Limits` on wasm32 - changes default texture format to `wgpu::TextureFormat::Rgba8UnormSrgb` Co-authored-by: Mariusz Kryński <[email protected]>
Also I can confirm that bevymark_pipelined does work on wasm/webgl2 if i disable the following features: bevy_audio, bevy_dynamic_plugin, bevy_wgpu, bevy_pbr2, render, mp3 |
Build failed: |
bors r+ |
# Objective Make possible to use wgpu gles backend on in the browser (wasm32 + WebGL2). ## Solution It is built on top of old @cart patch initializing windows before wgpu. Also: - initializes wgpu with `Backends::GL` and proper `wgpu::Limits` on wasm32 - changes default texture format to `wgpu::TextureFormat::Rgba8UnormSrgb` Co-authored-by: Mariusz Kryński <[email protected]>
Build failed: |
crates/bevy_winit/src/lib.rs
Outdated
use winit::platform::unix::EventLoopExtUnix; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the cfg before should be removed also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bawhaha well that wasn't smart of me
bors r+ |
# Objective Make possible to use wgpu gles backend on in the browser (wasm32 + WebGL2). ## Solution It is built on top of old @cart patch initializing windows before wgpu. Also: - initializes wgpu with `Backends::GL` and proper `wgpu::Limits` on wasm32 - changes default texture format to `wgpu::TextureFormat::Rgba8UnormSrgb` Co-authored-by: Mariusz Kryński <[email protected]>
Pull request successfully merged into pipelined-rendering. Build succeeded: |
This reverts commit 7d932ac.
Objective
Make possible to use wgpu gles backend on in the browser (wasm32 + WebGL2).
Solution
It is built on top of old @cart patch initializing windows before wgpu. Also:
Backends::GL
and properwgpu::Limits
on wasm32wgpu::TextureFormat::Rgba8UnormSrgb