From f969c62f7bfaf1932fbb533dfd87467aeddf75cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Sat, 11 Jun 2022 20:10:13 +0000 Subject: [PATCH] Fix wasm examples (#4967) # Objective Fix #4958 There was 4 issues: - this is not true in WASM and on macOS: https://github.com/bevyengine/bevy/blob/f28b92120920f387020f3b3e858f0e7039b9c07e/examples/3d/split_screen.rs#L90 - ~~I made sure the system was running at least once~~ - I'm sending the event on window creation - in webgl, setting a viewport has impacts on other render passes - only in webgl and when there is a custom viewport, I added a render pass without a custom viewport - shaderdef NO_ARRAY_TEXTURES_SUPPORT was not used by the 2d pipeline - webgl feature was used but not declared in bevy_sprite, I added it to the Cargo.toml - shaderdef NO_STORAGE_BUFFERS_SUPPORT was not used by the 2d pipeline - I added it based on the BufferBindingType The last commit changes the two last fixes to add the shaderdefs in the shader cache directly instead of needing to do it in each pipeline Co-authored-by: Carter Anderson --- crates/bevy_core_pipeline/Cargo.toml | 1 + .../src/core_2d/main_pass_2d_node.rs | 78 ++++++++++++------- .../src/core_3d/main_pass_3d_node.rs | 20 +++++ crates/bevy_internal/Cargo.toml | 2 +- crates/bevy_pbr/src/render/light.rs | 12 --- crates/bevy_pbr/src/render/mesh.rs | 15 ---- .../src/render_resource/pipeline_cache.rs | 24 +++++- crates/bevy_sprite/src/mesh2d/mesh.rs | 3 - crates/bevy_winit/src/lib.rs | 11 +++ 9 files changed, 107 insertions(+), 59 deletions(-) diff --git a/crates/bevy_core_pipeline/Cargo.toml b/crates/bevy_core_pipeline/Cargo.toml index 50abf04a60988..89602f5048756 100644 --- a/crates/bevy_core_pipeline/Cargo.toml +++ b/crates/bevy_core_pipeline/Cargo.toml @@ -14,6 +14,7 @@ keywords = ["bevy"] [features] trace = [] +webgl = [] [dependencies] # bevy diff --git a/crates/bevy_core_pipeline/src/core_2d/main_pass_2d_node.rs b/crates/bevy_core_pipeline/src/core_2d/main_pass_2d_node.rs index b0d65c0acc30d..626b022d46ede 100644 --- a/crates/bevy_core_pipeline/src/core_2d/main_pass_2d_node.rs +++ b/crates/bevy_core_pipeline/src/core_2d/main_pass_2d_node.rs @@ -11,6 +11,8 @@ use bevy_render::{ renderer::RenderContext, view::{ExtractedView, ViewTarget}, }; +#[cfg(feature = "trace")] +use bevy_utils::tracing::info_span; pub struct MainPass2dNode { query: QueryState< @@ -57,37 +59,61 @@ impl Node for MainPass2dNode { // no target return Ok(()); }; + { + #[cfg(feature = "trace")] + let _main_pass_2d = info_span!("main_pass_2d").entered(); + let pass_descriptor = RenderPassDescriptor { + label: Some("main_pass_2d"), + color_attachments: &[target.get_color_attachment(Operations { + load: match camera_2d.clear_color { + ClearColorConfig::Default => { + LoadOp::Clear(world.resource::().0.into()) + } + ClearColorConfig::Custom(color) => LoadOp::Clear(color.into()), + ClearColorConfig::None => LoadOp::Load, + }, + store: true, + })], + depth_stencil_attachment: None, + }; - let pass_descriptor = RenderPassDescriptor { - label: Some("main_pass_2d"), - color_attachments: &[target.get_color_attachment(Operations { - load: match camera_2d.clear_color { - ClearColorConfig::Default => { - LoadOp::Clear(world.resource::().0.into()) - } - ClearColorConfig::Custom(color) => LoadOp::Clear(color.into()), - ClearColorConfig::None => LoadOp::Load, - }, - store: true, - })], - depth_stencil_attachment: None, - }; - - let draw_functions = world.resource::>(); + let draw_functions = world.resource::>(); - let render_pass = render_context - .command_encoder - .begin_render_pass(&pass_descriptor); + let render_pass = render_context + .command_encoder + .begin_render_pass(&pass_descriptor); - let mut draw_functions = draw_functions.write(); - let mut tracked_pass = TrackedRenderPass::new(render_pass); - if let Some(viewport) = camera.viewport.as_ref() { - tracked_pass.set_camera_viewport(viewport); + let mut draw_functions = draw_functions.write(); + let mut tracked_pass = TrackedRenderPass::new(render_pass); + if let Some(viewport) = camera.viewport.as_ref() { + tracked_pass.set_camera_viewport(viewport); + } + for item in &transparent_phase.items { + let draw_function = draw_functions.get_mut(item.draw_function).unwrap(); + draw_function.draw(world, &mut tracked_pass, view_entity, item); + } } - for item in &transparent_phase.items { - let draw_function = draw_functions.get_mut(item.draw_function).unwrap(); - draw_function.draw(world, &mut tracked_pass, view_entity, item); + + // WebGL2 quirk: if ending with a render pass with a custom viewport, the viewport isn't + // reset for the next render pass so add an empty render pass without a custom viewport + #[cfg(feature = "webgl")] + if camera.viewport.is_some() { + #[cfg(feature = "trace")] + let _reset_viewport_pass_2d = info_span!("reset_viewport_pass_2d").entered(); + let pass_descriptor = RenderPassDescriptor { + label: Some("reset_viewport_pass_2d"), + color_attachments: &[target.get_color_attachment(Operations { + load: LoadOp::Load, + store: true, + })], + depth_stencil_attachment: None, + }; + + render_context + .command_encoder + .begin_render_pass(&pass_descriptor); } + Ok(()) } } diff --git a/crates/bevy_core_pipeline/src/core_3d/main_pass_3d_node.rs b/crates/bevy_core_pipeline/src/core_3d/main_pass_3d_node.rs index 1521e230a9a28..ec9aa7a6ff884 100644 --- a/crates/bevy_core_pipeline/src/core_3d/main_pass_3d_node.rs +++ b/crates/bevy_core_pipeline/src/core_3d/main_pass_3d_node.rs @@ -194,6 +194,26 @@ impl Node for MainPass3dNode { } } + // WebGL2 quirk: if ending with a render pass with a custom viewport, the viewport isn't + // reset for the next render pass so add an empty render pass without a custom viewport + #[cfg(feature = "webgl")] + if camera.viewport.is_some() { + #[cfg(feature = "trace")] + let _reset_viewport_pass_3d = info_span!("reset_viewport_pass_3d").entered(); + let pass_descriptor = RenderPassDescriptor { + label: Some("reset_viewport_pass_3d"), + color_attachments: &[target.get_color_attachment(Operations { + load: LoadOp::Load, + store: true, + })], + depth_stencil_attachment: None, + }; + + render_context + .command_encoder + .begin_render_pass(&pass_descriptor); + } + Ok(()) } } diff --git a/crates/bevy_internal/Cargo.toml b/crates/bevy_internal/Cargo.toml index 9635929399504..5868ebc494f71 100644 --- a/crates/bevy_internal/Cargo.toml +++ b/crates/bevy_internal/Cargo.toml @@ -55,7 +55,7 @@ x11 = ["bevy_winit/x11"] subpixel_glyph_atlas = ["bevy_text/subpixel_glyph_atlas"] # Optimise for WebGL2 -webgl = ["bevy_pbr/webgl", "bevy_render/webgl"] +webgl = ["bevy_core_pipeline/webgl", "bevy_pbr/webgl", "bevy_render/webgl"] # enable systems that allow for automated testing on CI bevy_ci_testing = ["bevy_app/bevy_ci_testing", "bevy_render/ci_limits"] diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 1862ebda3e265..63cf180e0e810 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -212,7 +212,6 @@ pub struct ShadowPipeline { pub skinned_mesh_layout: BindGroupLayout, pub point_light_sampler: Sampler, pub directional_light_sampler: Sampler, - pub clustered_forward_buffer_binding_type: BufferBindingType, } // TODO: this pattern for initializing the shaders / pipeline isn't ideal. this should be handled by the asset system @@ -221,9 +220,6 @@ impl FromWorld for ShadowPipeline { let world = world.cell(); let render_device = world.resource::(); - let clustered_forward_buffer_binding_type = render_device - .get_supported_read_only_binding_type(CLUSTERED_FORWARD_STORAGE_BUFFER_COUNT); - let view_layout = render_device.create_bind_group_layout(&BindGroupLayoutDescriptor { entries: &[ // View @@ -268,7 +264,6 @@ impl FromWorld for ShadowPipeline { compare: Some(CompareFunction::GreaterEqual), ..Default::default() }), - clustered_forward_buffer_binding_type, } } } @@ -330,13 +325,6 @@ impl SpecializedMeshPipeline for ShadowPipeline { bind_group_layout.push(self.mesh_layout.clone()); } - if !matches!( - self.clustered_forward_buffer_binding_type, - BufferBindingType::Storage { .. } - ) { - shader_defs.push(String::from("NO_STORAGE_BUFFERS_SUPPORT")); - } - let vertex_buffer_layout = layout.get_layout(&vertex_attributes)?; Ok(RenderPipelineDescriptor { diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 449e711100002..705357c034927 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/crates/bevy_pbr/src/render/mesh.rs @@ -582,18 +582,6 @@ impl SpecializedMeshPipeline for MeshPipeline { vertex_attributes.push(Mesh::ATTRIBUTE_COLOR.at_shader_location(4)); } - // TODO: consider exposing this in shaders in a more generally useful way, such as: - // # if AVAILABLE_STORAGE_BUFFER_BINDINGS == 3 - // /* use storage buffers here */ - // # elif - // /* use uniforms here */ - if !matches!( - self.clustered_forward_buffer_binding_type, - BufferBindingType::Storage { .. } - ) { - shader_defs.push(String::from("NO_STORAGE_BUFFERS_SUPPORT")); - } - let mut bind_group_layout = vec![self.view_layout.clone()]; if layout.contains(Mesh::ATTRIBUTE_JOINT_INDEX) && layout.contains(Mesh::ATTRIBUTE_JOINT_WEIGHT) @@ -624,9 +612,6 @@ impl SpecializedMeshPipeline for MeshPipeline { depth_write_enabled = true; } - #[cfg(feature = "webgl")] - shader_defs.push(String::from("NO_ARRAY_TEXTURES_SUPPORT")); - Ok(RenderPipelineDescriptor { vertex: VertexState { shader: MESH_SHADER_HANDLE.typed::(), diff --git a/crates/bevy_render/src/render_resource/pipeline_cache.rs b/crates/bevy_render/src/render_resource/pipeline_cache.rs index 204a94745bbeb..71021fbde9cca 100644 --- a/crates/bevy_render/src/render_resource/pipeline_cache.rs +++ b/crates/bevy_render/src/render_resource/pipeline_cache.rs @@ -15,7 +15,10 @@ use bevy_ecs::system::{Res, ResMut}; use bevy_utils::{default, tracing::error, Entry, HashMap, HashSet}; use std::{hash::Hash, iter::FusedIterator, mem, ops::Deref, sync::Arc}; use thiserror::Error; -use wgpu::{PipelineLayoutDescriptor, ShaderModule, VertexBufferLayout as RawVertexBufferLayout}; +use wgpu::{ + BufferBindingType, PipelineLayoutDescriptor, ShaderModule, + VertexBufferLayout as RawVertexBufferLayout, +}; enum PipelineDescriptor { RenderPipelineDescriptor(Box), @@ -117,9 +120,26 @@ impl ShaderCache { let module = match data.processed_shaders.entry(shader_defs.to_vec()) { Entry::Occupied(entry) => entry.into_mut(), Entry::Vacant(entry) => { + let mut shader_defs = shader_defs.to_vec(); + #[cfg(feature = "webgl")] + shader_defs.push(String::from("NO_ARRAY_TEXTURES_SUPPORT")); + + // TODO: 3 is the value from CLUSTERED_FORWARD_STORAGE_BUFFER_COUNT declared in bevy_pbr + // consider exposing this in shaders in a more generally useful way, such as: + // # if AVAILABLE_STORAGE_BUFFER_BINDINGS == 3 + // /* use storage buffers here */ + // # elif + // /* use uniforms here */ + if !matches!( + render_device.get_supported_read_only_binding_type(3), + BufferBindingType::Storage { .. } + ) { + shader_defs.push(String::from("NO_STORAGE_BUFFERS_SUPPORT")); + } + let processed = self.processor.process( shader, - shader_defs, + &shader_defs, &self.shaders, &self.import_path_shaders, )?; diff --git a/crates/bevy_sprite/src/mesh2d/mesh.rs b/crates/bevy_sprite/src/mesh2d/mesh.rs index 6dc1ce9d0d6b1..6d87d757a0423 100644 --- a/crates/bevy_sprite/src/mesh2d/mesh.rs +++ b/crates/bevy_sprite/src/mesh2d/mesh.rs @@ -323,9 +323,6 @@ impl SpecializedMeshPipeline for Mesh2dPipeline { vertex_attributes.push(Mesh::ATTRIBUTE_COLOR.at_shader_location(4)); } - #[cfg(feature = "webgl")] - shader_defs.push(String::from("NO_ARRAY_TEXTURES_SUPPORT")); - let vertex_buffer_layout = layout.get_layout(&vertex_attributes)?; Ok(RenderPipelineDescriptor { diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 40534dd8098eb..88e3f81126bea 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -637,12 +637,23 @@ fn handle_create_window_events( let mut windows = world.resource_mut::(); let create_window_events = world.resource::>(); let mut window_created_events = world.resource_mut::>(); + #[cfg(not(any(target_os = "windows", target_feature = "x11")))] + let mut window_resized_events = world.resource_mut::>(); for create_window_event in create_window_event_reader.iter(&create_window_events) { let window = winit_windows.create_window( event_loop, create_window_event.id, &create_window_event.descriptor, ); + // This event is already sent on windows, x11, and xwayland. + // TODO: we aren't yet sure about native wayland, so we might be able to exclude it, + // but sending a duplicate event isn't problematic, as windows already does this. + #[cfg(not(any(target_os = "windows", target_feature = "x11")))] + window_resized_events.send(WindowResized { + id: create_window_event.id, + width: window.width(), + height: window.height(), + }); windows.add(window); window_created_events.send(WindowCreated { id: create_window_event.id,