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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

[Merged by Bors] - WebGL2 support #3039

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
80175a5
fix webgl init by initializing windows before wgpu
cart May 23, 2021
d852db3
fix wgpu initialization
mrk-its Oct 7, 2021
9e6eccd
add webgl to wgpu features
mrk-its Oct 8, 2021
c2b89d3
use Rgba8UnormSrgb by default on wasm32
mrk-its Oct 7, 2021
52f9f4a
cargo fmt
mrk-its Oct 27, 2021
6716a28
only enable webgl feature on wasm targets
cart Oct 29, 2021
20a761c
clippy
cart Oct 29, 2021
dbe09fa
come on cart get it together!
cart Oct 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/bevy_internal/src/default_plugins.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ impl PluginGroup for PipelinedDefaultPlugins {
group.add(bevy_asset::AssetPlugin::default());
group.add(bevy_scene::ScenePlugin::default());

#[cfg(feature = "bevy_winit")]
group.add(bevy_winit::WinitPlugin::default());

#[cfg(feature = "bevy_render2")]
{
group.add(bevy_render2::RenderPlugin::default());
Expand All @@ -137,8 +140,5 @@ impl PluginGroup for PipelinedDefaultPlugins {
#[cfg(feature = "bevy_gltf2")]
group.add(bevy_gltf2::GltfPlugin::default());
}

#[cfg(feature = "bevy_winit")]
group.add(bevy_winit::WinitPlugin::default());
}
}
49 changes: 36 additions & 13 deletions crates/bevy_winit/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ impl Plugin for WinitPlugin {
app.init_resource::<WinitWindows>()
.set_runner(winit_runner)
.add_system_to_stage(CoreStage::PostUpdate, change_window.exclusive_system());
let mut event_loop = EventLoop::new();
handle_initial_window_events(&mut app.world, &mut event_loop);
app.insert_non_send_resource(event_loop);
}
}

Expand Down Expand Up @@ -207,21 +210,22 @@ where
}

pub fn winit_runner(app: App) {
winit_runner_with(app, EventLoop::new());
winit_runner_with(app);
}

#[cfg(any(
target_os = "linux",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "netbsd",
target_os = "openbsd"
))]
pub fn winit_runner_any_thread(app: App) {
winit_runner_with(app, EventLoop::new_any_thread());
}

pub fn winit_runner_with(mut app: App, mut event_loop: EventLoop<()>) {
// #[cfg(any(
// target_os = "linux",
// target_os = "dragonfly",
// target_os = "freebsd",
// target_os = "netbsd",
// target_os = "openbsd"
// ))]
// pub fn winit_runner_any_thread(app: App) {
// winit_runner_with(app, EventLoop::new_any_thread());
// }

pub fn winit_runner_with(mut app: App) {
let mut event_loop = app.world.remove_non_send::<EventLoop<()>>().unwrap();
let mut create_window_event_reader = ManualEventReader::<CreateWindow>::default();
let mut app_exit_event_reader = ManualEventReader::<AppExit>::default();
app.world.insert_non_send(event_loop.create_proxy());
Expand Down Expand Up @@ -525,3 +529,22 @@ fn handle_create_window_events(
});
}
}

fn handle_initial_window_events(world: &mut World, event_loop: &EventLoop<()>) {
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Member

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.

let world = world.cell();
let mut winit_windows = world.get_resource_mut::<WinitWindows>().unwrap();
let mut windows = world.get_resource_mut::<Windows>().unwrap();
let mut create_window_events = world.get_resource_mut::<Events<CreateWindow>>().unwrap();
let mut window_created_events = world.get_resource_mut::<Events<WindowCreated>>().unwrap();
for create_window_event in create_window_events.drain() {
let window = winit_windows.create_window(
event_loop,
create_window_event.id,
&create_window_event.descriptor,
);
windows.add(window);
window_created_events.send(WindowCreated {
id: create_window_event.id,
});
}
}
2 changes: 1 addition & 1 deletion pipelined/bevy_render2/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Copy link
Member

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

Copy link
Member Author

Choose a reason for hiding this comment

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

probably, taking a look

Copy link
Member

Choose a reason for hiding this comment

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

just fixed this

naga = { version = "0.7.0", features = ["glsl-in", "spv-in", "spv-out", "wgsl-in", "wgsl-out"] }
serde = { version = "1", features = ["derive"] }
bitflags = "1.2.1"
Expand Down
41 changes: 33 additions & 8 deletions pipelined/bevy_render2/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,40 @@ struct ScratchRenderWorld(World);

impl Plugin for RenderPlugin {
fn build(&self, app: &mut App) {
let (instance, device, queue) =
futures_lite::future::block_on(renderer::initialize_renderer(
wgpu::util::backend_bits_from_env().unwrap_or(Backends::PRIMARY),
&wgpu::RequestAdapterOptions {
power_preference: wgpu::PowerPreference::HighPerformance,
..Default::default()
let default_backend = if cfg!(not(target_arch = "wasm32")) {
Backends::PRIMARY
} else {
Backends::GL
};
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Member

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".

let backends = wgpu::util::backend_bits_from_env().unwrap_or(default_backend);
let instance = wgpu::Instance::new(backends);
let surface = {
let world = app.world.cell();
let windows = world.get_resource_mut::<bevy_window::Windows>().unwrap();
let raw_handle = windows.get_primary().map(|window| unsafe {
let handle = window.raw_window_handle().get_handle();
instance.create_surface(&handle)
});
raw_handle
};
let (device, queue) = futures_lite::future::block_on(renderer::initialize_renderer(
&instance,
&wgpu::RequestAdapterOptions {
power_preference: wgpu::PowerPreference::HighPerformance,
compatible_surface: surface.as_ref(),
..Default::default()
},
&wgpu::DeviceDescriptor {
features: wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES,
#[cfg(not(target_arch = "wasm32"))]
limits: wgpu::Limits::default(),
#[cfg(target_arch = "wasm32")]
limits: wgpu::Limits {
..wgpu::Limits::downlevel_webgl2_defaults()
},
&wgpu::DeviceDescriptor::default(),
));
..Default::default()
},
));
app.insert_resource(device.clone())
.insert_resource(queue.clone())
.init_resource::<ScratchRenderWorld>();
Expand Down
8 changes: 3 additions & 5 deletions pipelined/bevy_render2/src/renderer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,10 @@ pub type RenderQueue = Arc<Queue>;
pub type RenderInstance = Instance;

pub async fn initialize_renderer(
backends: Backends,
instance: &Instance,
request_adapter_options: &RequestAdapterOptions<'_>,
device_descriptor: &DeviceDescriptor<'_>,
) -> (RenderInstance, RenderDevice, RenderQueue) {
let instance = wgpu::Instance::new(backends);

) -> (RenderDevice, RenderQueue) {
let adapter = instance
.request_adapter(request_adapter_options)
.await
Expand All @@ -72,7 +70,7 @@ pub async fn initialize_renderer(
.unwrap();
let device = Arc::new(device);
let queue = Arc::new(queue);
(instance, RenderDevice::from(device), queue)
(RenderDevice::from(device), queue)
}

pub struct RenderContext {
Expand Down
2 changes: 1 addition & 1 deletion pipelined/bevy_render2/src/texture/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Copy link
Contributor

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?

Copy link
Member

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.

Copy link
Member

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?

Copy link

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.

// Bgra8UnormSrgb texture missing on some Android devices
wgpu::TextureFormat::Rgba8UnormSrgb
} else {
Expand Down