From 6f2a5cb862d747c9068bf83996c6c1a9f9d3b901 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sat, 21 Oct 2023 16:39:22 +0100 Subject: [PATCH 01/11] Bind group entries (#9694) # Objective Simplify bind group creation code. alternative to (and based on) #9476 ## Solution - Add a `BindGroupEntries` struct that can transparently be used where `&[BindGroupEntry<'b>]` is required in BindGroupDescriptors. Allows constructing the descriptor's entries as: ```rust render_device.create_bind_group( "my_bind_group", &my_layout, &BindGroupEntries::with_indexes(( (2, &my_sampler), (3, my_uniform), )), ); ``` instead of ```rust render_device.create_bind_group( "my_bind_group", &my_layout, &[ BindGroupEntry { binding: 2, resource: BindingResource::Sampler(&my_sampler), }, BindGroupEntry { binding: 3, resource: my_uniform, }, ], ); ``` or ```rust render_device.create_bind_group( "my_bind_group", &my_layout, &BindGroupEntries::sequential((&my_sampler, my_uniform)), ); ``` instead of ```rust render_device.create_bind_group( "my_bind_group", &my_layout, &[ BindGroupEntry { binding: 0, resource: BindingResource::Sampler(&my_sampler), }, BindGroupEntry { binding: 1, resource: my_uniform, }, ], ); ``` the structs has no user facing macros, is tuple-type-based so stack allocated, and has no noticeable impact on compile time. - Also adds a `DynamicBindGroupEntries` struct with a similar api that uses a `Vec` under the hood and allows extending the entries. - Modifies `RenderDevice::create_bind_group` to take separate arguments `label`, `layout` and `entries` instead of a `BindGroupDescriptor` struct. The struct can't be stored due to the internal references, and with only 3 members arguably does not add enough context to justify itself. - Modify the codebase to use the new api and the `BindGroupEntries` / `DynamicBindGroupEntries` structs where appropriate (whenever the entries slice contains more than 1 member). ## Migration Guide - Calls to `RenderDevice::create_bind_group({BindGroupDescriptor { label, layout, entries })` must be amended to `RenderDevice::create_bind_group(label, layout, entries)`. - If `label`s have been specified as `"bind_group_name".into()`, they need to change to just `"bind_group_name"`. `Some("bind_group_name")` and `None` will still work, but `Some("bind_group_name")` can optionally be simplified to just `"bind_group_name"`. --------- Co-authored-by: IceSentry --- crates/bevy_core_pipeline/src/bloom/mod.rs | 88 ++---- .../src/contrast_adaptive_sharpening/node.rs | 36 +-- .../src/deferred/copy_lighting_id.rs | 22 +- crates/bevy_core_pipeline/src/fxaa/node.rs | 27 +- .../bevy_core_pipeline/src/msaa_writeback.rs | 23 +- crates/bevy_core_pipeline/src/skybox/mod.rs | 41 +-- crates/bevy_core_pipeline/src/taa/mod.rs | 65 ++-- .../bevy_core_pipeline/src/tonemapping/mod.rs | 14 +- .../src/tonemapping/node.rs | 48 +-- .../bevy_core_pipeline/src/upscaling/node.rs | 27 +- crates/bevy_gizmos/src/lib.rs | 15 +- crates/bevy_pbr/src/deferred/mod.rs | 15 +- crates/bevy_pbr/src/environment_map/mod.rs | 22 +- crates/bevy_pbr/src/prepass/mod.rs | 50 +--- .../bevy_pbr/src/prepass/prepass_bindings.rs | 57 +--- crates/bevy_pbr/src/render/mesh_bindings.rs | 43 ++- .../bevy_pbr/src/render/mesh_view_bindings.rs | 140 +++------ crates/bevy_pbr/src/ssao/mod.rs | 223 ++++---------- .../src/render_resource/bind_group.rs | 11 +- .../src/render_resource/bind_group_entries.rs | 282 ++++++++++++++++++ crates/bevy_render/src/render_resource/mod.rs | 2 + .../src/render_resource/uniform_buffer.rs | 19 ++ .../bevy_render/src/renderer/render_device.rs | 17 +- crates/bevy_render/src/view/window/mod.rs | 17 +- crates/bevy_sprite/src/mesh2d/mesh.rs | 32 +- crates/bevy_sprite/src/render/mod.rs | 39 +-- crates/bevy_ui/src/render/mod.rs | 41 +-- crates/bevy_utils/macros/src/lib.rs | 35 +++ .../shader/compute_shader_game_of_life.rs | 13 +- examples/shader/post_processing.rs | 49 ++- examples/shader/texture_binding_array.rs | 24 +- 31 files changed, 704 insertions(+), 833 deletions(-) create mode 100644 crates/bevy_render/src/render_resource/bind_group_entries.rs diff --git a/crates/bevy_core_pipeline/src/bloom/mod.rs b/crates/bevy_core_pipeline/src/bloom/mod.rs index 9f8d0aec2946f..c1299794df570 100644 --- a/crates/bevy_core_pipeline/src/bloom/mod.rs +++ b/crates/bevy_core_pipeline/src/bloom/mod.rs @@ -170,30 +170,16 @@ impl ViewNode for BloomNode { // First downsample pass { - let downsampling_first_bind_group = - render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: Some("bloom_downsampling_first_bind_group"), - layout: &downsampling_pipeline_res.bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - // Read from main texture directly - resource: BindingResource::TextureView( - view_target.main_texture_view(), - ), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&bind_groups.sampler), - }, - BindGroupEntry { - binding: 2, - resource: uniforms.clone(), - }, - ], - }); + let downsampling_first_bind_group = render_context.render_device().create_bind_group( + "bloom_downsampling_first_bind_group", + &downsampling_pipeline_res.bind_group_layout, + &BindGroupEntries::sequential(( + // Read from main texture directly + view_target.main_texture_view(), + &bind_groups.sampler, + uniforms.clone(), + )), + ); let view = &bloom_texture.view(0); let mut downsampling_first_pass = @@ -416,46 +402,28 @@ fn prepare_bloom_bind_groups( let mut downsampling_bind_groups = Vec::with_capacity(bind_group_count); for mip in 1..bloom_texture.mip_count { - downsampling_bind_groups.push(render_device.create_bind_group(&BindGroupDescriptor { - label: Some("bloom_downsampling_bind_group"), - layout: &downsampling_pipeline.bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(&bloom_texture.view(mip - 1)), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(sampler), - }, - BindGroupEntry { - binding: 2, - resource: uniforms.binding().unwrap(), - }, - ], - })); + downsampling_bind_groups.push(render_device.create_bind_group( + "bloom_downsampling_bind_group", + &downsampling_pipeline.bind_group_layout, + &BindGroupEntries::sequential(( + &bloom_texture.view(mip - 1), + sampler, + uniforms.binding().unwrap(), + )), + )); } let mut upsampling_bind_groups = Vec::with_capacity(bind_group_count); for mip in (0..bloom_texture.mip_count).rev() { - upsampling_bind_groups.push(render_device.create_bind_group(&BindGroupDescriptor { - label: Some("bloom_upsampling_bind_group"), - layout: &upsampling_pipeline.bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(&bloom_texture.view(mip)), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(sampler), - }, - BindGroupEntry { - binding: 2, - resource: uniforms.binding().unwrap(), - }, - ], - })); + upsampling_bind_groups.push(render_device.create_bind_group( + "bloom_upsampling_bind_group", + &upsampling_pipeline.bind_group_layout, + &BindGroupEntries::sequential(( + &bloom_texture.view(mip), + sampler, + uniforms.binding().unwrap(), + )), + )); } commands.entity(entity).insert(BloomBindGroups { diff --git a/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/node.rs b/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/node.rs index 0dd3f086ee27c..5bb8b87ebc58b 100644 --- a/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/node.rs +++ b/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/node.rs @@ -7,8 +7,8 @@ use bevy_render::{ extract_component::{ComponentUniforms, DynamicUniformIndex}, render_graph::{Node, NodeRunError, RenderGraphContext}, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindingResource, BufferId, Operations, - PipelineCache, RenderPassColorAttachment, RenderPassDescriptor, TextureViewId, + BindGroup, BindGroupEntries, BufferId, Operations, PipelineCache, + RenderPassColorAttachment, RenderPassDescriptor, TextureViewId, }, renderer::RenderContext, view::{ExtractedView, ViewTarget}, @@ -77,29 +77,15 @@ impl Node for CASNode { bind_group } cached_bind_group => { - let bind_group = - render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: Some("cas_bind_group"), - layout: &sharpening_pipeline.texture_bind_group, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(view_target.source), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler( - &sharpening_pipeline.sampler, - ), - }, - BindGroupEntry { - binding: 2, - resource: uniforms, - }, - ], - }); + let bind_group = render_context.render_device().create_bind_group( + "cas_bind_group", + &sharpening_pipeline.texture_bind_group, + &BindGroupEntries::sequential(( + view_target.source, + &sharpening_pipeline.sampler, + uniforms, + )), + ); let (_, _, bind_group) = cached_bind_group.insert((uniforms_id, source.id(), bind_group)); diff --git a/crates/bevy_core_pipeline/src/deferred/copy_lighting_id.rs b/crates/bevy_core_pipeline/src/deferred/copy_lighting_id.rs index 5609896045ec8..c60306286900a 100644 --- a/crates/bevy_core_pipeline/src/deferred/copy_lighting_id.rs +++ b/crates/bevy_core_pipeline/src/deferred/copy_lighting_id.rs @@ -18,10 +18,7 @@ use bevy_render::{ use bevy_ecs::query::QueryItem; use bevy_render::{ render_graph::{NodeRunError, RenderGraphContext, ViewNode}, - render_resource::{ - BindGroupDescriptor, BindGroupEntry, BindingResource, Operations, PipelineCache, - RenderPassDescriptor, - }, + render_resource::{Operations, PipelineCache, RenderPassDescriptor}, renderer::RenderContext, }; @@ -94,18 +91,11 @@ impl ViewNode for CopyDeferredLightingIdNode { return Ok(()); }; - let bind_group = render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: Some("copy_deferred_lighting_id_bind_group"), - layout: ©_deferred_lighting_id_pipeline.layout, - entries: &[BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView( - &deferred_lighting_pass_id_texture.default_view, - ), - }], - }); + let bind_group = render_context.render_device().create_bind_group( + "copy_deferred_lighting_id_bind_group", + ©_deferred_lighting_id_pipeline.layout, + &BindGroupEntries::single(&deferred_lighting_pass_id_texture.default_view), + ); let mut render_pass = render_context.begin_tracked_render_pass(RenderPassDescriptor { label: Some("copy_deferred_lighting_id_pass"), diff --git a/crates/bevy_core_pipeline/src/fxaa/node.rs b/crates/bevy_core_pipeline/src/fxaa/node.rs index 2daaf0584d981..7eaf4dce268ad 100644 --- a/crates/bevy_core_pipeline/src/fxaa/node.rs +++ b/crates/bevy_core_pipeline/src/fxaa/node.rs @@ -6,9 +6,8 @@ use bevy_ecs::query::QueryItem; use bevy_render::{ render_graph::{NodeRunError, RenderGraphContext, ViewNode}, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindingResource, FilterMode, Operations, - PipelineCache, RenderPassColorAttachment, RenderPassDescriptor, SamplerDescriptor, - TextureViewId, + BindGroup, BindGroupEntries, FilterMode, Operations, PipelineCache, + RenderPassColorAttachment, RenderPassDescriptor, SamplerDescriptor, TextureViewId, }, renderer::RenderContext, view::ViewTarget, @@ -61,23 +60,11 @@ impl ViewNode for FxaaNode { ..default() }); - let bind_group = - render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: None, - layout: &fxaa_pipeline.texture_bind_group, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(source), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&sampler), - }, - ], - }); + let bind_group = render_context.render_device().create_bind_group( + None, + &fxaa_pipeline.texture_bind_group, + &BindGroupEntries::sequential((source, &sampler)), + ); let (_, bind_group) = cached_bind_group.insert((source.id(), bind_group)); bind_group diff --git a/crates/bevy_core_pipeline/src/msaa_writeback.rs b/crates/bevy_core_pipeline/src/msaa_writeback.rs index 0646d4ce67ffb..d80bc0fce7bc9 100644 --- a/crates/bevy_core_pipeline/src/msaa_writeback.rs +++ b/crates/bevy_core_pipeline/src/msaa_writeback.rs @@ -8,6 +8,7 @@ use bevy_ecs::prelude::*; use bevy_render::{ camera::ExtractedCamera, render_graph::{Node, NodeRunError, RenderGraphApp, RenderGraphContext}, + render_resource::BindGroupEntries, renderer::RenderContext, view::{Msaa, ViewTarget}, Render, RenderSet, @@ -90,23 +91,11 @@ impl Node for MsaaWritebackNode { depth_stencil_attachment: None, }; - let bind_group = - render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: None, - layout: &blit_pipeline.texture_bind_group, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(post_process.source), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&blit_pipeline.sampler), - }, - ], - }); + let bind_group = render_context.render_device().create_bind_group( + None, + &blit_pipeline.texture_bind_group, + &BindGroupEntries::sequential((post_process.source, &blit_pipeline.sampler)), + ); let mut render_pass = render_context .command_encoder() diff --git a/crates/bevy_core_pipeline/src/skybox/mod.rs b/crates/bevy_core_pipeline/src/skybox/mod.rs index efc133651930e..11caa03afd8aa 100644 --- a/crates/bevy_core_pipeline/src/skybox/mod.rs +++ b/crates/bevy_core_pipeline/src/skybox/mod.rs @@ -10,13 +10,13 @@ use bevy_render::{ extract_component::{ExtractComponent, ExtractComponentPlugin}, render_asset::RenderAssets, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindGroupLayout, BindGroupLayoutDescriptor, - BindGroupLayoutEntry, BindingResource, BindingType, BufferBindingType, - CachedRenderPipelineId, ColorTargetState, ColorWrites, CompareFunction, DepthBiasState, - DepthStencilState, FragmentState, MultisampleState, PipelineCache, PrimitiveState, - RenderPipelineDescriptor, SamplerBindingType, Shader, ShaderStages, ShaderType, - SpecializedRenderPipeline, SpecializedRenderPipelines, StencilFaceState, StencilState, - TextureFormat, TextureSampleType, TextureViewDimension, VertexState, + BindGroup, BindGroupEntries, BindGroupLayout, BindGroupLayoutDescriptor, + BindGroupLayoutEntry, BindingType, BufferBindingType, CachedRenderPipelineId, + ColorTargetState, ColorWrites, CompareFunction, DepthBiasState, DepthStencilState, + FragmentState, MultisampleState, PipelineCache, PrimitiveState, RenderPipelineDescriptor, + SamplerBindingType, Shader, ShaderStages, ShaderType, SpecializedRenderPipeline, + SpecializedRenderPipelines, StencilFaceState, StencilState, TextureFormat, + TextureSampleType, TextureViewDimension, VertexState, }, renderer::RenderDevice, texture::{BevyDefault, Image}, @@ -224,24 +224,15 @@ fn prepare_skybox_bind_groups( if let (Some(skybox), Some(view_uniforms)) = (images.get(&skybox.0), view_uniforms.uniforms.binding()) { - let bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: Some("skybox_bind_group"), - layout: &pipeline.bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(&skybox.texture_view), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&skybox.sampler), - }, - BindGroupEntry { - binding: 2, - resource: view_uniforms, - }, - ], - }); + let bind_group = render_device.create_bind_group( + "skybox_bind_group", + &pipeline.bind_group_layout, + &BindGroupEntries::sequential(( + &skybox.texture_view, + &skybox.sampler, + view_uniforms, + )), + ); commands.entity(entity).insert(SkyboxBindGroup(bind_group)); } diff --git a/crates/bevy_core_pipeline/src/taa/mod.rs b/crates/bevy_core_pipeline/src/taa/mod.rs index e61dccc5d4bf5..28926a1b8ed0d 100644 --- a/crates/bevy_core_pipeline/src/taa/mod.rs +++ b/crates/bevy_core_pipeline/src/taa/mod.rs @@ -21,13 +21,13 @@ use bevy_render::{ prelude::{Camera, Projection}, render_graph::{NodeRunError, RenderGraphApp, RenderGraphContext, ViewNode, ViewNodeRunner}, render_resource::{ - BindGroupDescriptor, BindGroupEntry, BindGroupLayout, BindGroupLayoutDescriptor, - BindGroupLayoutEntry, BindingResource, BindingType, CachedRenderPipelineId, - ColorTargetState, ColorWrites, Extent3d, FilterMode, FragmentState, MultisampleState, - Operations, PipelineCache, PrimitiveState, RenderPassColorAttachment, RenderPassDescriptor, - RenderPipelineDescriptor, Sampler, SamplerBindingType, SamplerDescriptor, Shader, - ShaderStages, SpecializedRenderPipeline, SpecializedRenderPipelines, TextureDescriptor, - TextureDimension, TextureFormat, TextureSampleType, TextureUsages, TextureViewDimension, + BindGroupEntries, BindGroupLayout, BindGroupLayoutDescriptor, BindGroupLayoutEntry, + BindingType, CachedRenderPipelineId, ColorTargetState, ColorWrites, Extent3d, FilterMode, + FragmentState, MultisampleState, Operations, PipelineCache, PrimitiveState, + RenderPassColorAttachment, RenderPassDescriptor, RenderPipelineDescriptor, Sampler, + SamplerBindingType, SamplerDescriptor, Shader, ShaderStages, SpecializedRenderPipeline, + SpecializedRenderPipelines, TextureDescriptor, TextureDimension, TextureFormat, + TextureSampleType, TextureUsages, TextureViewDimension, }, renderer::{RenderContext, RenderDevice}, texture::{BevyDefault, CachedTexture, TextureCache}, @@ -197,45 +197,18 @@ impl ViewNode for TAANode { }; let view_target = view_target.post_process_write(); - let taa_bind_group = - render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: Some("taa_bind_group"), - layout: &pipelines.taa_bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(view_target.source), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::TextureView( - &taa_history_textures.read.default_view, - ), - }, - BindGroupEntry { - binding: 2, - resource: BindingResource::TextureView( - &prepass_motion_vectors_texture.default_view, - ), - }, - BindGroupEntry { - binding: 3, - resource: BindingResource::TextureView( - &prepass_depth_texture.default_view, - ), - }, - BindGroupEntry { - binding: 4, - resource: BindingResource::Sampler(&pipelines.nearest_sampler), - }, - BindGroupEntry { - binding: 5, - resource: BindingResource::Sampler(&pipelines.linear_sampler), - }, - ], - }); + let taa_bind_group = render_context.render_device().create_bind_group( + "taa_bind_group", + &pipelines.taa_bind_group_layout, + &BindGroupEntries::sequential(( + view_target.source, + &taa_history_textures.read.default_view, + &prepass_motion_vectors_texture.default_view, + &prepass_depth_texture.default_view, + &pipelines.nearest_sampler, + &pipelines.linear_sampler, + )), + ); { let mut taa_pass = render_context.begin_tracked_render_pass(RenderPassDescriptor { diff --git a/crates/bevy_core_pipeline/src/tonemapping/mod.rs b/crates/bevy_core_pipeline/src/tonemapping/mod.rs index 2af1d48b6eed0..6aada19c458e5 100644 --- a/crates/bevy_core_pipeline/src/tonemapping/mod.rs +++ b/crates/bevy_core_pipeline/src/tonemapping/mod.rs @@ -306,8 +306,7 @@ pub fn get_lut_bindings<'a>( images: &'a RenderAssets, tonemapping_luts: &'a TonemappingLuts, tonemapping: &Tonemapping, - bindings: [u32; 2], -) -> [BindGroupEntry<'a>; 2] { +) -> (&'a TextureView, &'a Sampler) { let image = match tonemapping { // AgX lut texture used when tonemapping doesn't need a texture since it's very small (32x32x32) Tonemapping::None @@ -320,16 +319,7 @@ pub fn get_lut_bindings<'a>( Tonemapping::BlenderFilmic => &tonemapping_luts.blender_filmic, }; let lut_image = images.get(image).unwrap(); - [ - BindGroupEntry { - binding: bindings[0], - resource: BindingResource::TextureView(&lut_image.texture_view), - }, - BindGroupEntry { - binding: bindings[1], - resource: BindingResource::Sampler(&lut_image.sampler), - }, - ] + (&lut_image.texture_view, &lut_image.sampler) } pub fn get_lut_bind_group_layout_entries(bindings: [u32; 2]) -> [BindGroupLayoutEntry; 2] { diff --git a/crates/bevy_core_pipeline/src/tonemapping/node.rs b/crates/bevy_core_pipeline/src/tonemapping/node.rs index e3da4aa03e417..1d1c95d970850 100644 --- a/crates/bevy_core_pipeline/src/tonemapping/node.rs +++ b/crates/bevy_core_pipeline/src/tonemapping/node.rs @@ -7,9 +7,8 @@ use bevy_render::{ render_asset::RenderAssets, render_graph::{NodeRunError, RenderGraphContext, ViewNode}, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindingResource, BufferId, LoadOp, - Operations, PipelineCache, RenderPassColorAttachment, RenderPassDescriptor, - SamplerDescriptor, TextureViewId, + BindGroup, BindGroupEntries, BufferId, LoadOp, Operations, PipelineCache, + RenderPassColorAttachment, RenderPassDescriptor, SamplerDescriptor, TextureViewId, }, renderer::RenderContext, texture::Image, @@ -88,36 +87,19 @@ impl ViewNode for TonemappingNode { let tonemapping_luts = world.resource::(); - let mut entries = vec![ - BindGroupEntry { - binding: 0, - resource: view_uniforms.binding().unwrap(), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::TextureView(source), - }, - BindGroupEntry { - binding: 2, - resource: BindingResource::Sampler(&sampler), - }, - ]; - - entries.extend(get_lut_bindings( - gpu_images, - tonemapping_luts, - tonemapping, - [3, 4], - )); - - let bind_group = - render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: None, - layout: &tonemapping_pipeline.texture_bind_group, - entries: &entries, - }); + let lut_bindings = get_lut_bindings(gpu_images, tonemapping_luts, tonemapping); + + let bind_group = render_context.render_device().create_bind_group( + None, + &tonemapping_pipeline.texture_bind_group, + &BindGroupEntries::sequential(( + view_uniforms, + source, + &sampler, + lut_bindings.0, + lut_bindings.1, + )), + ); let (_, _, bind_group) = cached_bind_group.insert((view_uniforms_id, source.id(), bind_group)); diff --git a/crates/bevy_core_pipeline/src/upscaling/node.rs b/crates/bevy_core_pipeline/src/upscaling/node.rs index 76ff1d195c998..536b2b9437515 100644 --- a/crates/bevy_core_pipeline/src/upscaling/node.rs +++ b/crates/bevy_core_pipeline/src/upscaling/node.rs @@ -4,9 +4,8 @@ use bevy_render::{ camera::{CameraOutputMode, ExtractedCamera}, render_graph::{NodeRunError, RenderGraphContext, ViewNode}, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindingResource, LoadOp, Operations, - PipelineCache, RenderPassColorAttachment, RenderPassDescriptor, SamplerDescriptor, - TextureViewId, + BindGroup, BindGroupEntries, LoadOp, Operations, PipelineCache, RenderPassColorAttachment, + RenderPassDescriptor, SamplerDescriptor, TextureViewId, }, renderer::RenderContext, view::ViewTarget, @@ -57,23 +56,11 @@ impl ViewNode for UpscalingNode { .render_device() .create_sampler(&SamplerDescriptor::default()); - let bind_group = - render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: None, - layout: &blit_pipeline.texture_bind_group, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(upscaled_texture), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&sampler), - }, - ], - }); + let bind_group = render_context.render_device().create_bind_group( + None, + &blit_pipeline.texture_bind_group, + &BindGroupEntries::sequential((upscaled_texture, &sampler)), + ); let (_, bind_group) = cached_bind_group.insert((upscaled_texture.id(), bind_group)); bind_group diff --git a/crates/bevy_gizmos/src/lib.rs b/crates/bevy_gizmos/src/lib.rs index 2efaa9c914607..446605b43e4f8 100644 --- a/crates/bevy_gizmos/src/lib.rs +++ b/crates/bevy_gizmos/src/lib.rs @@ -52,7 +52,7 @@ use bevy_render::{ render_asset::{PrepareAssetError, RenderAsset, RenderAssetPlugin, RenderAssets}, render_phase::{PhaseItem, RenderCommand, RenderCommandResult, TrackedRenderPass}, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindGroupLayout, BindGroupLayoutDescriptor, + BindGroup, BindGroupEntries, BindGroupLayout, BindGroupLayoutDescriptor, BindGroupLayoutEntry, BindingType, Buffer, BufferBindingType, BufferInitDescriptor, BufferUsages, Shader, ShaderStages, ShaderType, VertexAttribute, VertexBufferLayout, VertexFormat, VertexStepMode, @@ -422,14 +422,11 @@ fn prepare_line_gizmo_bind_group( ) { if let Some(binding) = line_gizmo_uniforms.uniforms().binding() { commands.insert_resource(LineGizmoUniformBindgroup { - bindgroup: render_device.create_bind_group(&BindGroupDescriptor { - entries: &[BindGroupEntry { - binding: 0, - resource: binding, - }], - label: Some("LineGizmoUniform bindgroup"), - layout: &line_gizmo_uniform_layout.layout, - }), + bindgroup: render_device.create_bind_group( + "LineGizmoUniform bindgroup", + &line_gizmo_uniform_layout.layout, + &BindGroupEntries::single(binding), + ), }); } } diff --git a/crates/bevy_pbr/src/deferred/mod.rs b/crates/bevy_pbr/src/deferred/mod.rs index b8b61d15573da..f679f638d42ac 100644 --- a/crates/bevy_pbr/src/deferred/mod.rs +++ b/crates/bevy_pbr/src/deferred/mod.rs @@ -191,16 +191,11 @@ impl ViewNode for DeferredOpaquePass3dPbrLightingNode { return Ok(()); }; - let bind_group_1 = render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: Some("deferred_lighting_layout_group_1"), - layout: &deferred_lighting_layout.bind_group_layout_1, - entries: &[BindGroupEntry { - binding: 0, - resource: deferred_lighting_pass_id_binding.clone(), - }], - }); + let bind_group_1 = render_context.render_device().create_bind_group( + "deferred_lighting_layout_group_1", + &deferred_lighting_layout.bind_group_layout_1, + &BindGroupEntries::single(deferred_lighting_pass_id_binding), + ); let mut render_pass = render_context.begin_tracked_render_pass(RenderPassDescriptor { label: Some("deferred_lighting_pass"), diff --git a/crates/bevy_pbr/src/environment_map/mod.rs b/crates/bevy_pbr/src/environment_map/mod.rs index f46f0309f0cac..8cc74a212a396 100644 --- a/crates/bevy_pbr/src/environment_map/mod.rs +++ b/crates/bevy_pbr/src/environment_map/mod.rs @@ -7,8 +7,8 @@ use bevy_render::{ extract_component::{ExtractComponent, ExtractComponentPlugin}, render_asset::RenderAssets, render_resource::{ - BindGroupEntry, BindGroupLayoutEntry, BindingResource, BindingType, SamplerBindingType, - Shader, ShaderStages, TextureSampleType, TextureViewDimension, + BindGroupLayoutEntry, BindingType, Sampler, SamplerBindingType, Shader, ShaderStages, + TextureSampleType, TextureView, TextureViewDimension, }, texture::{FallbackImageCubemap, Image}, }; @@ -65,8 +65,7 @@ pub fn get_bindings<'a>( environment_map_light: Option<&EnvironmentMapLight>, images: &'a RenderAssets, fallback_image_cubemap: &'a FallbackImageCubemap, - bindings: [u32; 3], -) -> [BindGroupEntry<'a>; 3] { +) -> (&'a TextureView, &'a TextureView, &'a Sampler) { let (diffuse_map, specular_map) = match ( environment_map_light.and_then(|env_map| images.get(&env_map.diffuse_map)), environment_map_light.and_then(|env_map| images.get(&env_map.specular_map)), @@ -80,20 +79,7 @@ pub fn get_bindings<'a>( ), }; - [ - BindGroupEntry { - binding: bindings[0], - resource: BindingResource::TextureView(diffuse_map), - }, - BindGroupEntry { - binding: bindings[1], - resource: BindingResource::TextureView(specular_map), - }, - BindGroupEntry { - binding: bindings[2], - resource: BindingResource::Sampler(&fallback_image_cubemap.sampler), - }, - ] + (diffuse_map, specular_map, &fallback_image_cubemap.sampler) } pub fn get_bind_group_layout_entries(bindings: [u32; 3]) -> [BindGroupLayoutEntry; 3] { diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 6c5b4559d045c..54155ce385a0e 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -35,7 +35,7 @@ use bevy_render::{ RenderPhase, SetItemPipeline, TrackedRenderPass, }, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindGroupLayout, BindGroupLayoutDescriptor, + BindGroup, BindGroupEntries, BindGroupLayout, BindGroupLayoutDescriptor, BindGroupLayoutEntry, BindingType, BufferBindingType, ColorTargetState, ColorWrites, CompareFunction, DepthBiasState, DepthStencilState, DynamicUniformBuffer, FragmentState, FrontFace, MultisampleState, PipelineCache, PolygonMode, PrimitiveState, PushConstantRange, @@ -713,42 +713,22 @@ pub fn prepare_prepass_view_bind_group( view_uniforms.uniforms.binding(), globals_buffer.buffer.binding(), ) { - prepass_view_bind_group.no_motion_vectors = - Some(render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: view_binding.clone(), - }, - BindGroupEntry { - binding: 1, - resource: globals_binding.clone(), - }, - ], - label: Some("prepass_view_no_motion_vectors_bind_group"), - layout: &prepass_pipeline.view_layout_no_motion_vectors, - })); + prepass_view_bind_group.no_motion_vectors = Some(render_device.create_bind_group( + "prepass_view_no_motion_vectors_bind_group", + &prepass_pipeline.view_layout_no_motion_vectors, + &BindGroupEntries::sequential((view_binding.clone(), globals_binding.clone())), + )); if let Some(previous_view_proj_binding) = previous_view_proj_uniforms.uniforms.binding() { - prepass_view_bind_group.motion_vectors = - Some(render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: view_binding, - }, - BindGroupEntry { - binding: 1, - resource: globals_binding, - }, - BindGroupEntry { - binding: 2, - resource: previous_view_proj_binding, - }, - ], - label: Some("prepass_view_motion_vectors_bind_group"), - layout: &prepass_pipeline.view_layout_motion_vectors, - })); + prepass_view_bind_group.motion_vectors = Some(render_device.create_bind_group( + "prepass_view_motion_vectors_bind_group", + &prepass_pipeline.view_layout_motion_vectors, + &BindGroupEntries::sequential(( + view_binding, + globals_binding, + previous_view_proj_binding, + )), + )); } } } diff --git a/crates/bevy_pbr/src/prepass/prepass_bindings.rs b/crates/bevy_pbr/src/prepass/prepass_bindings.rs index acbf80ceabde1..b72ddd1e318cd 100644 --- a/crates/bevy_pbr/src/prepass/prepass_bindings.rs +++ b/crates/bevy_pbr/src/prepass/prepass_bindings.rs @@ -1,7 +1,7 @@ use bevy_core_pipeline::prepass::ViewPrepassTextures; use bevy_render::render_resource::{ - BindGroupEntry, BindGroupLayoutEntry, BindingResource, BindingType, ShaderStages, - TextureAspect, TextureSampleType, TextureView, TextureViewDescriptor, TextureViewDimension, + BindGroupLayoutEntry, BindingType, ShaderStages, TextureAspect, TextureSampleType, TextureView, + TextureViewDescriptor, TextureViewDimension, }; use bevy_utils::default; use smallvec::SmallVec; @@ -83,51 +83,7 @@ pub fn get_bind_group_layout_entries( result } -// Needed so the texture views can live long enough. -pub struct PrepassBindingsSet { - depth_view: Option, - normal_view: Option, - motion_vectors_view: Option, - deferred_view: Option, -} - -impl PrepassBindingsSet { - pub fn get_entries(&self, bindings: [u32; 4]) -> SmallVec<[BindGroupEntry; 4]> { - let mut result = SmallVec::<[BindGroupEntry; 4]>::new(); - - if let Some(ref depth_view) = self.depth_view { - result.push(BindGroupEntry { - binding: bindings[0], - resource: BindingResource::TextureView(depth_view), - }); - } - - if let Some(ref normal_view) = self.normal_view { - result.push(BindGroupEntry { - binding: bindings[1], - resource: BindingResource::TextureView(normal_view), - }); - } - - if let Some(ref motion_vectors_view) = self.motion_vectors_view { - result.push(BindGroupEntry { - binding: bindings[2], - resource: BindingResource::TextureView(motion_vectors_view), - }); - } - - if let Some(ref deferred_view) = self.deferred_view { - result.push(BindGroupEntry { - binding: bindings[3], - resource: BindingResource::TextureView(deferred_view), - }); - } - - result - } -} - -pub fn get_bindings(prepass_textures: Option<&ViewPrepassTextures>) -> PrepassBindingsSet { +pub fn get_bindings(prepass_textures: Option<&ViewPrepassTextures>) -> [Option; 4] { let depth_desc = TextureViewDescriptor { label: Some("prepass_depth"), aspect: TextureAspect::DepthOnly, @@ -149,10 +105,5 @@ pub fn get_bindings(prepass_textures: Option<&ViewPrepassTextures>) -> PrepassBi .and_then(|x| x.deferred.as_ref()) .map(|texture| texture.default_view.clone()); - PrepassBindingsSet { - depth_view, - normal_view, - motion_vectors_view, - deferred_view, - } + [depth_view, normal_view, motion_vectors_view, deferred_view] } diff --git a/crates/bevy_pbr/src/render/mesh_bindings.rs b/crates/bevy_pbr/src/render/mesh_bindings.rs index dcc01e1aa4c8b..bf45fd12b8c83 100644 --- a/crates/bevy_pbr/src/render/mesh_bindings.rs +++ b/crates/bevy_pbr/src/render/mesh_bindings.rs @@ -4,8 +4,7 @@ use bevy_math::Mat4; use bevy_render::{ mesh::morph::MAX_MORPH_WEIGHTS, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupLayout, BindGroupLayoutDescriptor, - BindingResource, Buffer, TextureView, + BindGroup, BindGroupLayout, BindGroupLayoutDescriptor, BindingResource, Buffer, TextureView, }, renderer::RenderDevice, }; @@ -179,11 +178,11 @@ impl MeshLayouts { // ---------- BindGroup methods ---------- pub fn model_only(&self, render_device: &RenderDevice, model: &BindingResource) -> BindGroup { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[entry::model(0, model.clone())], - layout: &self.model_only, - label: Some("model_only_mesh_bind_group"), - }) + render_device.create_bind_group( + "model_only_mesh_bind_group", + &self.model_only, + &[entry::model(0, model.clone())], + ) } pub fn skinned( &self, @@ -191,11 +190,11 @@ impl MeshLayouts { model: &BindingResource, skin: &Buffer, ) -> BindGroup { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[entry::model(0, model.clone()), entry::skinning(1, skin)], - layout: &self.skinned, - label: Some("skinned_mesh_bind_group"), - }) + render_device.create_bind_group( + "skinned_mesh_bind_group", + &self.skinned, + &[entry::model(0, model.clone()), entry::skinning(1, skin)], + ) } pub fn morphed( &self, @@ -204,15 +203,15 @@ impl MeshLayouts { weights: &Buffer, targets: &TextureView, ) -> BindGroup { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ + render_device.create_bind_group( + "morphed_mesh_bind_group", + &self.morphed, + &[ entry::model(0, model.clone()), entry::weights(2, weights), entry::targets(3, targets), ], - layout: &self.morphed, - label: Some("morphed_mesh_bind_group"), - }) + ) } pub fn morphed_skinned( &self, @@ -222,15 +221,15 @@ impl MeshLayouts { weights: &Buffer, targets: &TextureView, ) -> BindGroup { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ + render_device.create_bind_group( + "morphed_skinned_mesh_bind_group", + &self.morphed_skinned, + &[ entry::model(0, model.clone()), entry::skinning(1, skin), entry::weights(2, weights), entry::targets(3, targets), ], - layout: &self.morphed_skinned, - label: Some("morphed_skinned_mesh_bind_group"), - }) + ) } } diff --git a/crates/bevy_pbr/src/render/mesh_view_bindings.rs b/crates/bevy_pbr/src/render/mesh_view_bindings.rs index 8ed474769525f..ba66561c6d785 100644 --- a/crates/bevy_pbr/src/render/mesh_view_bindings.rs +++ b/crates/bevy_pbr/src/render/mesh_view_bindings.rs @@ -15,9 +15,9 @@ use bevy_render::{ globals::{GlobalsBuffer, GlobalsUniform}, render_asset::RenderAssets, render_resource::{ - BindGroup, BindGroupDescriptor, BindGroupEntry, BindGroupLayout, BindGroupLayoutDescriptor, - BindGroupLayoutEntry, BindingResource, BindingType, BufferBindingType, SamplerBindingType, - ShaderStages, ShaderType, TextureFormat, TextureSampleType, TextureViewDimension, + BindGroup, BindGroupLayout, BindGroupLayoutDescriptor, BindGroupLayoutEntry, BindingType, + BufferBindingType, DynamicBindGroupEntries, SamplerBindingType, ShaderStages, ShaderType, + TextureFormat, TextureSampleType, TextureViewDimension, }, renderer::RenderDevice, texture::{BevyDefault, FallbackImageCubemap, FallbackImageMsaa, Image}, @@ -383,8 +383,8 @@ pub fn prepare_mesh_view_bind_groups( ) { for ( entity, - view_shadow_bindings, - view_cluster_bindings, + shadow_bindings, + cluster_bindings, ssao_textures, prepass_textures, environment_map, @@ -395,108 +395,58 @@ pub fn prepare_mesh_view_bind_groups( .image_for_samplecount(1, TextureFormat::bevy_default()) .texture_view .clone(); + let ssao_view = ssao_textures + .map(|t| &t.screen_space_ambient_occlusion_texture.default_view) + .unwrap_or(&fallback_ssao); let layout = &mesh_pipeline.get_view_layout( MeshPipelineViewLayoutKey::from(*msaa) | MeshPipelineViewLayoutKey::from(prepass_textures), ); - let mut entries = vec![ - BindGroupEntry { - binding: 0, - resource: view_binding.clone(), - }, - BindGroupEntry { - binding: 1, - resource: light_binding.clone(), - }, - BindGroupEntry { - binding: 2, - resource: BindingResource::TextureView( - &view_shadow_bindings.point_light_depth_texture_view, - ), - }, - BindGroupEntry { - binding: 3, - resource: BindingResource::Sampler(&shadow_samplers.point_light_sampler), - }, - BindGroupEntry { - binding: 4, - resource: BindingResource::TextureView( - &view_shadow_bindings.directional_light_depth_texture_view, - ), - }, - BindGroupEntry { - binding: 5, - resource: BindingResource::Sampler(&shadow_samplers.directional_light_sampler), - }, - BindGroupEntry { - binding: 6, - resource: point_light_binding.clone(), - }, - BindGroupEntry { - binding: 7, - resource: view_cluster_bindings.light_index_lists_binding().unwrap(), - }, - BindGroupEntry { - binding: 8, - resource: view_cluster_bindings.offsets_and_counts_binding().unwrap(), - }, - BindGroupEntry { - binding: 9, - resource: globals.clone(), - }, - BindGroupEntry { - binding: 10, - resource: fog_binding.clone(), - }, - BindGroupEntry { - binding: 11, - resource: BindingResource::TextureView( - ssao_textures - .map(|t| &t.screen_space_ambient_occlusion_texture.default_view) - .unwrap_or(&fallback_ssao), - ), - }, - ]; - - let env_map = environment_map::get_bindings( - environment_map, - &images, - &fallback_cubemap, - [12, 13, 14], - ); - entries.extend_from_slice(&env_map); - - let tonemapping_luts = - get_lut_bindings(&images, &tonemapping_luts, tonemapping, [15, 16]); - entries.extend_from_slice(&tonemapping_luts); - - let label = Some("mesh_view_bind_group"); + let mut entries = DynamicBindGroupEntries::new_with_indices(( + (0, view_binding.clone()), + (1, light_binding.clone()), + (2, &shadow_bindings.point_light_depth_texture_view), + (3, &shadow_samplers.point_light_sampler), + (4, &shadow_bindings.directional_light_depth_texture_view), + (5, &shadow_samplers.directional_light_sampler), + (6, point_light_binding.clone()), + (7, cluster_bindings.light_index_lists_binding().unwrap()), + (8, cluster_bindings.offsets_and_counts_binding().unwrap()), + (9, globals.clone()), + (10, fog_binding.clone()), + (11, ssao_view), + )); + + let env_map_bindings = + environment_map::get_bindings(environment_map, &images, &fallback_cubemap); + entries = entries.extend_with_indices(( + (12, env_map_bindings.0), + (13, env_map_bindings.1), + (14, env_map_bindings.2), + )); + + let lut_bindings = get_lut_bindings(&images, &tonemapping_luts, tonemapping); + entries = entries.extend_with_indices(((15, lut_bindings.0), (16, lut_bindings.1))); // When using WebGL, we can't have a depth texture with multisampling - let prepass_bindings = if cfg!(any(not(feature = "webgl"), not(target_arch = "wasm32"))) - || (cfg!(all(feature = "webgl", target_arch = "wasm32")) && msaa.samples() == 1) + let prepass_bindings; + if cfg!(any(not(feature = "webgl"), not(target_arch = "wasm32"))) || msaa.samples() == 1 { - Some(prepass::get_bindings(prepass_textures)) - } else { - None - }; - - // This if statement is here to make the borrow checker happy. - // Ideally we could just have `entries.extend_from_slice(&prepass_bindings.get_entries([17, 18, 19, 20]));` - // in the existing if statement above, but that either doesn't allow `prepass_bindings` to live long enough, - // as its used when creating the bind group at the end of the function, or causes a `cannot move out of` error. - if let Some(prepass_bindings) = &prepass_bindings { - entries.extend_from_slice(&prepass_bindings.get_entries([17, 18, 19, 20])); + prepass_bindings = prepass::get_bindings(prepass_textures); + for (binding, index) in prepass_bindings + .iter() + .map(Option::as_ref) + .zip([17, 18, 19, 20]) + .flat_map(|(b, i)| b.map(|b| (b, i))) + { + entries = entries.extend_with_indices(((index, binding),)); + } } commands.entity(entity).insert(MeshViewBindGroup { - value: render_device.create_bind_group(&BindGroupDescriptor { - entries: &entries, - label, - layout, - }), + value: render_device.create_bind_group("mesh_view_bind_group", layout, &entries), }); } } diff --git a/crates/bevy_pbr/src/ssao/mod.rs b/crates/bevy_pbr/src/ssao/mod.rs index eaabea3772654..59fb57cfac3c6 100644 --- a/crates/bevy_pbr/src/ssao/mod.rs +++ b/crates/bevy_pbr/src/ssao/mod.rs @@ -21,12 +21,11 @@ use bevy_render::{ prelude::Camera, render_graph::{NodeRunError, RenderGraphApp, RenderGraphContext, ViewNode, ViewNodeRunner}, render_resource::{ - AddressMode, BindGroup, BindGroupDescriptor, BindGroupEntry, BindGroupLayout, - BindGroupLayoutDescriptor, BindGroupLayoutEntry, BindingResource, BindingType, - BufferBindingType, CachedComputePipelineId, ComputePassDescriptor, - ComputePipelineDescriptor, Extent3d, FilterMode, PipelineCache, Sampler, - SamplerBindingType, SamplerDescriptor, Shader, ShaderDefVal, ShaderStages, ShaderType, - SpecializedComputePipeline, SpecializedComputePipelines, StorageTextureAccess, + AddressMode, BindGroup, BindGroupEntries, BindGroupLayout, BindGroupLayoutDescriptor, + BindGroupLayoutEntry, BindingType, BufferBindingType, CachedComputePipelineId, + ComputePassDescriptor, ComputePipelineDescriptor, Extent3d, FilterMode, PipelineCache, + Sampler, SamplerBindingType, SamplerDescriptor, Shader, ShaderDefVal, ShaderStages, + ShaderType, SpecializedComputePipeline, SpecializedComputePipelines, StorageTextureAccess, TextureDescriptor, TextureDimension, TextureFormat, TextureSampleType, TextureUsages, TextureView, TextureViewDescriptor, TextureViewDimension, }, @@ -776,171 +775,63 @@ fn prepare_ssao_bind_groups( }; for (entity, ssao_textures, prepass_textures) in &views { - let common_bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: Some("ssao_common_bind_group"), - layout: &pipelines.common_bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::Sampler(&pipelines.point_clamp_sampler), - }, - BindGroupEntry { - binding: 1, - resource: view_uniforms.clone(), - }, - ], - }); + let common_bind_group = render_device.create_bind_group( + "ssao_common_bind_group", + &pipelines.common_bind_group_layout, + &BindGroupEntries::sequential((&pipelines.point_clamp_sampler, view_uniforms.clone())), + ); - let preprocess_depth_mip_view_descriptor = TextureViewDescriptor { - format: Some(TextureFormat::R16Float), - dimension: Some(TextureViewDimension::D2), - mip_level_count: Some(1), - ..default() + let create_depth_view = |mip_level| { + ssao_textures + .preprocessed_depth_texture + .texture + .create_view(&TextureViewDescriptor { + label: Some("ssao_preprocessed_depth_texture_mip_view"), + base_mip_level: mip_level, + format: Some(TextureFormat::R16Float), + dimension: Some(TextureViewDimension::D2), + mip_level_count: Some(1), + ..default() + }) }; - let preprocess_depth_bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: Some("ssao_preprocess_depth_bind_group"), - layout: &pipelines.preprocess_depth_bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView( - &prepass_textures.depth.as_ref().unwrap().default_view, - ), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::TextureView( - &ssao_textures - .preprocessed_depth_texture - .texture - .create_view(&TextureViewDescriptor { - label: Some("ssao_preprocessed_depth_texture_mip_view_0"), - base_mip_level: 0, - ..preprocess_depth_mip_view_descriptor - }), - ), - }, - BindGroupEntry { - binding: 2, - resource: BindingResource::TextureView( - &ssao_textures - .preprocessed_depth_texture - .texture - .create_view(&TextureViewDescriptor { - label: Some("ssao_preprocessed_depth_texture_mip_view_1"), - base_mip_level: 1, - ..preprocess_depth_mip_view_descriptor - }), - ), - }, - BindGroupEntry { - binding: 3, - resource: BindingResource::TextureView( - &ssao_textures - .preprocessed_depth_texture - .texture - .create_view(&TextureViewDescriptor { - label: Some("ssao_preprocessed_depth_texture_mip_view_2"), - base_mip_level: 2, - ..preprocess_depth_mip_view_descriptor - }), - ), - }, - BindGroupEntry { - binding: 4, - resource: BindingResource::TextureView( - &ssao_textures - .preprocessed_depth_texture - .texture - .create_view(&TextureViewDescriptor { - label: Some("ssao_preprocessed_depth_texture_mip_view_3"), - base_mip_level: 3, - ..preprocess_depth_mip_view_descriptor - }), - ), - }, - BindGroupEntry { - binding: 5, - resource: BindingResource::TextureView( - &ssao_textures - .preprocessed_depth_texture - .texture - .create_view(&TextureViewDescriptor { - label: Some("ssao_preprocessed_depth_texture_mip_view_4"), - base_mip_level: 4, - ..preprocess_depth_mip_view_descriptor - }), - ), - }, - ], - }); + let preprocess_depth_bind_group = render_device.create_bind_group( + "ssao_preprocess_depth_bind_group", + &pipelines.preprocess_depth_bind_group_layout, + &BindGroupEntries::sequential(( + &prepass_textures.depth.as_ref().unwrap().default_view, + &create_depth_view(0), + &create_depth_view(1), + &create_depth_view(2), + &create_depth_view(3), + &create_depth_view(4), + )), + ); - let gtao_bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: Some("ssao_gtao_bind_group"), - layout: &pipelines.gtao_bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView( - &ssao_textures.preprocessed_depth_texture.default_view, - ), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::TextureView( - &prepass_textures.normal.as_ref().unwrap().default_view, - ), - }, - BindGroupEntry { - binding: 2, - resource: BindingResource::TextureView(&pipelines.hilbert_index_lut), - }, - BindGroupEntry { - binding: 3, - resource: BindingResource::TextureView( - &ssao_textures.ssao_noisy_texture.default_view, - ), - }, - BindGroupEntry { - binding: 4, - resource: BindingResource::TextureView( - &ssao_textures.depth_differences_texture.default_view, - ), - }, - BindGroupEntry { - binding: 5, - resource: globals_uniforms.clone(), - }, - ], - }); + let gtao_bind_group = render_device.create_bind_group( + "ssao_gtao_bind_group", + &pipelines.gtao_bind_group_layout, + &BindGroupEntries::sequential(( + &ssao_textures.preprocessed_depth_texture.default_view, + &prepass_textures.normal.as_ref().unwrap().default_view, + &pipelines.hilbert_index_lut, + &ssao_textures.ssao_noisy_texture.default_view, + &ssao_textures.depth_differences_texture.default_view, + globals_uniforms.clone(), + )), + ); - let spatial_denoise_bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: Some("ssao_spatial_denoise_bind_group"), - layout: &pipelines.spatial_denoise_bind_group_layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView( - &ssao_textures.ssao_noisy_texture.default_view, - ), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::TextureView( - &ssao_textures.depth_differences_texture.default_view, - ), - }, - BindGroupEntry { - binding: 2, - resource: BindingResource::TextureView( - &ssao_textures - .screen_space_ambient_occlusion_texture - .default_view, - ), - }, - ], - }); + let spatial_denoise_bind_group = render_device.create_bind_group( + "ssao_spatial_denoise_bind_group", + &pipelines.spatial_denoise_bind_group_layout, + &BindGroupEntries::sequential(( + &ssao_textures.ssao_noisy_texture.default_view, + &ssao_textures.depth_differences_texture.default_view, + &ssao_textures + .screen_space_ambient_occlusion_texture + .default_view, + )), + ); commands.entity(entity).insert(SsaoBindGroups { common_bind_group, diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index c4de9cb1b6089..8ee876b9c5208 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -9,10 +9,7 @@ use crate::{ pub use bevy_render_macros::AsBindGroup; use encase::ShaderType; use std::ops::Deref; -use wgpu::{ - BindGroupDescriptor, BindGroupEntry, BindGroupLayoutDescriptor, BindGroupLayoutEntry, - BindingResource, -}; +use wgpu::{BindGroupEntry, BindGroupLayoutDescriptor, BindGroupLayoutEntry, BindingResource}; define_atomic_id!(BindGroupId); render_resource_wrapper!(ErasedBindGroup, wgpu::BindGroup); @@ -289,11 +286,7 @@ pub trait AsBindGroup { }) .collect::>(); - let bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: Self::label(), - layout, - entries: &entries, - }); + let bind_group = render_device.create_bind_group(Self::label(), layout, &entries); Ok(PreparedBindGroup { bindings, diff --git a/crates/bevy_render/src/render_resource/bind_group_entries.rs b/crates/bevy_render/src/render_resource/bind_group_entries.rs new file mode 100644 index 0000000000000..09336eeb0a093 --- /dev/null +++ b/crates/bevy_render/src/render_resource/bind_group_entries.rs @@ -0,0 +1,282 @@ +use bevy_utils::all_tuples_with_size; +use wgpu::{BindGroupEntry, BindingResource}; + +use super::{Sampler, TextureView}; + +/// Helper for constructing bindgroups. +/// +/// Allows constructing the descriptor's entries as: +/// ```ignore +/// render_device.create_bind_group( +/// "my_bind_group", +/// &my_layout, +/// &BindGroupEntries::with_indices(( +/// (2, &my_sampler), +/// (3, my_uniform), +/// )), +/// ); +/// ``` +/// +/// instead of +/// +/// ```ignore +/// render_device.create_bind_group( +/// "my_bind_group", +/// &my_layout, +/// &[ +/// BindGroupEntry { +/// binding: 2, +/// resource: BindingResource::Sampler(&my_sampler), +/// }, +/// BindGroupEntry { +/// binding: 3, +/// resource: my_uniform, +/// }, +/// ], +/// ); +/// ``` +/// +/// or +/// +/// ```ignore +/// render_device.create_bind_group( +/// "my_bind_group", +/// &my_layout, +/// &BindGroupEntries::sequential(( +/// &my_sampler, +/// my_uniform, +/// )), +/// ); +/// ``` +/// +/// instead of +/// +/// ```ignore +/// render_device.create_bind_group( +/// "my_bind_group", +/// &my_layout, +/// &[ +/// BindGroupEntry { +/// binding: 0, +/// resource: BindingResource::Sampler(&my_sampler), +/// }, +/// BindGroupEntry { +/// binding: 1, +/// resource: my_uniform, +/// }, +/// ], +/// ); +/// ``` +/// +/// or +/// +/// ```ignore +/// render_device.create_bind_group( +/// "my_bind_group", +/// &my_layout, +/// &BindGroupEntries::single(my_uniform), +/// ); +/// ``` +/// +/// instead of +/// +/// ```ignore +/// render_device.create_bind_group( +/// "my_bind_group", +/// &my_layout, +/// &[ +/// BindGroupEntry { +/// binding: 0, +/// resource: my_uniform, +/// }, +/// ], +/// ); +/// ``` + +pub struct BindGroupEntries<'b, const N: usize = 1> { + entries: [BindGroupEntry<'b>; N], +} + +impl<'b, const N: usize> BindGroupEntries<'b, N> { + #[inline] + pub fn sequential(resources: impl IntoBindingArray<'b, N>) -> Self { + let mut i = 0; + Self { + entries: resources.into_array().map(|resource| { + let binding = i; + i += 1; + BindGroupEntry { binding, resource } + }), + } + } + + #[inline] + pub fn with_indices(indexed_resources: impl IntoIndexedBindingArray<'b, N>) -> Self { + Self { + entries: indexed_resources + .into_array() + .map(|(binding, resource)| BindGroupEntry { binding, resource }), + } + } +} + +impl<'b> BindGroupEntries<'b, 1> { + pub fn single(resource: impl IntoBinding<'b>) -> [BindGroupEntry<'b>; 1] { + [BindGroupEntry { + binding: 0, + resource: resource.into_binding(), + }] + } +} + +impl<'b, const N: usize> std::ops::Deref for BindGroupEntries<'b, N> { + type Target = [BindGroupEntry<'b>]; + + fn deref(&self) -> &[BindGroupEntry<'b>] { + &self.entries + } +} + +pub trait IntoBinding<'a> { + fn into_binding(self) -> BindingResource<'a>; +} + +impl<'a> IntoBinding<'a> for &'a TextureView { + #[inline] + fn into_binding(self) -> BindingResource<'a> { + BindingResource::TextureView(self) + } +} + +impl<'a> IntoBinding<'a> for &'a [&'a wgpu::TextureView] { + #[inline] + fn into_binding(self) -> BindingResource<'a> { + BindingResource::TextureViewArray(self) + } +} + +impl<'a> IntoBinding<'a> for &'a Sampler { + #[inline] + fn into_binding(self) -> BindingResource<'a> { + BindingResource::Sampler(self) + } +} + +impl<'a> IntoBinding<'a> for BindingResource<'a> { + #[inline] + fn into_binding(self) -> BindingResource<'a> { + self + } +} + +impl<'a> IntoBinding<'a> for wgpu::BufferBinding<'a> { + #[inline] + fn into_binding(self) -> BindingResource<'a> { + BindingResource::Buffer(self) + } +} + +pub trait IntoBindingArray<'b, const N: usize> { + fn into_array(self) -> [BindingResource<'b>; N]; +} + +macro_rules! impl_to_binding_slice { + ($N: expr, $(($T: ident, $I: ident)),*) => { + impl<'b, $($T: IntoBinding<'b>),*> IntoBindingArray<'b, $N> for ($($T,)*) { + #[inline] + fn into_array(self) -> [BindingResource<'b>; $N] { + let ($($I,)*) = self; + [$($I.into_binding(), )*] + } + } + } +} + +all_tuples_with_size!(impl_to_binding_slice, 1, 32, T, s); + +pub trait IntoIndexedBindingArray<'b, const N: usize> { + fn into_array(self) -> [(u32, BindingResource<'b>); N]; +} + +macro_rules! impl_to_indexed_binding_slice { + ($N: expr, $(($T: ident, $S: ident, $I: ident)),*) => { + impl<'b, $($T: IntoBinding<'b>),*> IntoIndexedBindingArray<'b, $N> for ($((u32, $T),)*) { + #[inline] + fn into_array(self) -> [(u32, BindingResource<'b>); $N] { + let ($(($S, $I),)*) = self; + [$(($S, $I.into_binding())), *] + } + } + } +} + +all_tuples_with_size!(impl_to_indexed_binding_slice, 1, 32, T, n, s); + +pub struct DynamicBindGroupEntries<'b> { + entries: Vec>, +} + +impl<'b> DynamicBindGroupEntries<'b> { + pub fn sequential(entries: impl IntoBindingArray<'b, N>) -> Self { + Self { + entries: entries + .into_array() + .into_iter() + .enumerate() + .map(|(ix, resource)| BindGroupEntry { + binding: ix as u32, + resource, + }) + .collect(), + } + } + + pub fn extend_sequential( + mut self, + entries: impl IntoBindingArray<'b, N>, + ) -> Self { + let start = self.entries.last().unwrap().binding + 1; + self.entries.extend( + entries + .into_array() + .into_iter() + .enumerate() + .map(|(ix, resource)| BindGroupEntry { + binding: start + ix as u32, + resource, + }), + ); + self + } + + pub fn new_with_indices(entries: impl IntoIndexedBindingArray<'b, N>) -> Self { + Self { + entries: entries + .into_array() + .into_iter() + .map(|(binding, resource)| BindGroupEntry { binding, resource }) + .collect(), + } + } + + pub fn extend_with_indices( + mut self, + entries: impl IntoIndexedBindingArray<'b, N>, + ) -> Self { + self.entries.extend( + entries + .into_array() + .into_iter() + .map(|(binding, resource)| BindGroupEntry { binding, resource }), + ); + self + } +} + +impl<'b> std::ops::Deref for DynamicBindGroupEntries<'b> { + type Target = [BindGroupEntry<'b>]; + + fn deref(&self) -> &[BindGroupEntry<'b>] { + &self.entries + } +} diff --git a/crates/bevy_render/src/render_resource/mod.rs b/crates/bevy_render/src/render_resource/mod.rs index f16f5f1269929..b7d245b0bdbc9 100644 --- a/crates/bevy_render/src/render_resource/mod.rs +++ b/crates/bevy_render/src/render_resource/mod.rs @@ -1,5 +1,6 @@ mod batched_uniform_buffer; mod bind_group; +mod bind_group_entries; mod bind_group_layout; mod buffer; mod buffer_vec; @@ -14,6 +15,7 @@ mod texture; mod uniform_buffer; pub use bind_group::*; +pub use bind_group_entries::*; pub use bind_group_layout::*; pub use buffer::*; pub use buffer_vec::*; diff --git a/crates/bevy_render/src/render_resource/uniform_buffer.rs b/crates/bevy_render/src/render_resource/uniform_buffer.rs index 3ecd692ba2880..95196568ff20a 100644 --- a/crates/bevy_render/src/render_resource/uniform_buffer.rs +++ b/crates/bevy_render/src/render_resource/uniform_buffer.rs @@ -13,6 +13,8 @@ use wgpu::{ util::BufferInitDescriptor, BindingResource, BufferBinding, BufferDescriptor, BufferUsages, }; +use super::IntoBinding; + /// Stores data to be transferred to the GPU and made accessible to shaders as a uniform buffer. /// /// Uniform buffers are available to shaders on a read-only basis. Uniform buffers are commonly used to make available to shaders @@ -139,6 +141,16 @@ impl UniformBuffer { } } +impl<'a, T: ShaderType + WriteInto> IntoBinding<'a> for &'a UniformBuffer { + #[inline] + fn into_binding(self) -> BindingResource<'a> { + self.buffer() + .expect("Failed to get buffer") + .as_entire_buffer_binding() + .into_binding() + } +} + /// Stores data to be transferred to the GPU and made accessible to shaders as a dynamic uniform buffer. /// /// Dynamic uniform buffers are available to shaders on a read-only basis. Dynamic uniform buffers are commonly used to make @@ -367,3 +379,10 @@ impl<'a> BufferMut for QueueWriteBufferViewWrapper<'a> { self.buffer_view.write(offset, val); } } + +impl<'a, T: ShaderType + WriteInto> IntoBinding<'a> for &'a DynamicUniformBuffer { + #[inline] + fn into_binding(self) -> BindingResource<'a> { + self.binding().unwrap() + } +} diff --git a/crates/bevy_render/src/renderer/render_device.rs b/crates/bevy_render/src/renderer/render_device.rs index 8a177e94774cb..6a126df8aa41e 100644 --- a/crates/bevy_render/src/renderer/render_device.rs +++ b/crates/bevy_render/src/renderer/render_device.rs @@ -3,7 +3,9 @@ use crate::render_resource::{ RenderPipeline, Sampler, Texture, }; use bevy_ecs::system::Resource; -use wgpu::{util::DeviceExt, BufferAsyncError, BufferBindingType}; +use wgpu::{ + util::DeviceExt, BindGroupDescriptor, BindGroupEntry, BufferAsyncError, BufferBindingType, +}; use super::RenderQueue; @@ -82,8 +84,17 @@ impl RenderDevice { /// Creates a new [`BindGroup`](wgpu::BindGroup). #[inline] - pub fn create_bind_group(&self, desc: &wgpu::BindGroupDescriptor) -> BindGroup { - let wgpu_bind_group = self.device.create_bind_group(desc); + pub fn create_bind_group<'a>( + &self, + label: impl Into>, + layout: &'a BindGroupLayout, + entries: &'a [BindGroupEntry<'a>], + ) -> BindGroup { + let wgpu_bind_group = self.device.create_bind_group(&BindGroupDescriptor { + label: label.into(), + layout, + entries, + }); BindGroup::from(wgpu_bind_group) } diff --git a/crates/bevy_render/src/view/window/mod.rs b/crates/bevy_render/src/view/window/mod.rs index 37729f3a253ed..31ac4fca2cee3 100644 --- a/crates/bevy_render/src/view/window/mod.rs +++ b/crates/bevy_render/src/view/window/mod.rs @@ -1,5 +1,7 @@ use crate::{ - render_resource::{PipelineCache, SpecializedRenderPipelines, SurfaceTexture, TextureView}, + render_resource::{ + BindGroupEntries, PipelineCache, SpecializedRenderPipelines, SurfaceTexture, TextureView, + }, renderer::{RenderAdapter, RenderDevice, RenderInstance}, texture::TextureFormatPixelInfo, Extract, ExtractSchedule, Render, RenderApp, RenderSet, @@ -413,14 +415,11 @@ pub fn prepare_windows( usage: BufferUsages::MAP_READ | BufferUsages::COPY_DST, mapped_at_creation: false, }); - let bind_group = render_device.create_bind_group(&wgpu::BindGroupDescriptor { - label: Some("screenshot-to-screen-bind-group"), - layout: &screenshot_pipeline.bind_group_layout, - entries: &[wgpu::BindGroupEntry { - binding: 0, - resource: wgpu::BindingResource::TextureView(&texture_view), - }], - }); + let bind_group = render_device.create_bind_group( + "screenshot-to-screen-bind-group", + &screenshot_pipeline.bind_group_layout, + &BindGroupEntries::single(&texture_view), + ); let pipeline_id = pipelines.specialize( &pipeline_cache, &screenshot_pipeline, diff --git a/crates/bevy_sprite/src/mesh2d/mesh.rs b/crates/bevy_sprite/src/mesh2d/mesh.rs index 5f62000bc441e..df5e4f0594011 100644 --- a/crates/bevy_sprite/src/mesh2d/mesh.rs +++ b/crates/bevy_sprite/src/mesh2d/mesh.rs @@ -596,14 +596,11 @@ pub fn prepare_mesh2d_bind_group( ) { if let Some(binding) = mesh2d_uniforms.binding() { commands.insert_resource(Mesh2dBindGroup { - value: render_device.create_bind_group(&BindGroupDescriptor { - entries: &[BindGroupEntry { - binding: 0, - resource: binding, - }], - label: Some("mesh2d_bind_group"), - layout: &mesh2d_pipeline.mesh_layout, - }), + value: render_device.create_bind_group( + "mesh2d_bind_group", + &mesh2d_pipeline.mesh_layout, + &BindGroupEntries::single(binding), + ), }); } } @@ -626,20 +623,11 @@ pub fn prepare_mesh2d_view_bind_groups( globals_buffer.buffer.binding(), ) { for entity in &views { - let view_bind_group = render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: view_binding.clone(), - }, - BindGroupEntry { - binding: 1, - resource: globals.clone(), - }, - ], - label: Some("mesh2d_view_bind_group"), - layout: &mesh2d_pipeline.view_layout, - }); + let view_bind_group = render_device.create_bind_group( + "mesh2d_view_bind_group", + &mesh2d_pipeline.view_layout, + &BindGroupEntries::sequential((view_binding.clone(), globals.clone())), + ); commands.entity(entity).insert(Mesh2dViewBindGroup { value: view_bind_group, diff --git a/crates/bevy_sprite/src/render/mod.rs b/crates/bevy_sprite/src/render/mod.rs index c70d33286ccf8..bebdbad231393 100644 --- a/crates/bevy_sprite/src/render/mod.rs +++ b/crates/bevy_sprite/src/render/mod.rs @@ -21,7 +21,7 @@ use bevy_render::{ DrawFunctions, PhaseItem, RenderCommand, RenderCommandResult, RenderPhase, SetItemPipeline, TrackedRenderPass, }, - render_resource::*, + render_resource::{BindGroupEntries, *}, renderer::{RenderDevice, RenderQueue}, texture::{ BevyDefault, DefaultImageSampler, GpuImage, Image, ImageSampler, TextureFormatPixelInfo, @@ -623,14 +623,11 @@ pub fn prepare_sprites( // Clear the sprite instances sprite_meta.sprite_instance_buffer.clear(); - sprite_meta.view_bind_group = Some(render_device.create_bind_group(&BindGroupDescriptor { - entries: &[BindGroupEntry { - binding: 0, - resource: view_binding, - }], - label: Some("sprite_view_bind_group"), - layout: &sprite_pipeline.view_layout, - })); + sprite_meta.view_bind_group = Some(render_device.create_bind_group( + "sprite_view_bind_group", + &sprite_pipeline.view_layout, + &BindGroupEntries::single(view_binding), + )); // Index buffer indices let mut index = 0; @@ -667,22 +664,14 @@ pub fn prepare_sprites( .values .entry(batch_image_handle) .or_insert_with(|| { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView( - &gpu_image.texture_view, - ), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&gpu_image.sampler), - }, - ], - label: Some("sprite_material_bind_group"), - layout: &sprite_pipeline.material_layout, - }) + render_device.create_bind_group( + "sprite_material_bind_group", + &sprite_pipeline.material_layout, + &BindGroupEntries::sequential(( + &gpu_image.texture_view, + &gpu_image.sampler, + )), + ) }); } diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index c852122e6ac76..a7a52c628319e 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -5,7 +5,7 @@ use bevy_core_pipeline::{core_2d::Camera2d, core_3d::Camera3d}; use bevy_hierarchy::Parent; use bevy_render::render_phase::PhaseItem; use bevy_render::view::ViewVisibility; -use bevy_render::{ExtractSchedule, Render}; +use bevy_render::{render_resource::BindGroupEntries, ExtractSchedule, Render}; use bevy_window::{PrimaryWindow, Window}; pub use pipeline::*; pub use render_pass::*; @@ -812,14 +812,11 @@ pub fn prepare_uinodes( let mut batches: Vec<(Entity, UiBatch)> = Vec::with_capacity(*previous_len); ui_meta.vertices.clear(); - ui_meta.view_bind_group = Some(render_device.create_bind_group(&BindGroupDescriptor { - entries: &[BindGroupEntry { - binding: 0, - resource: view_binding, - }], - label: Some("ui_view_bind_group"), - layout: &ui_pipeline.view_layout, - })); + ui_meta.view_bind_group = Some(render_device.create_bind_group( + "ui_view_bind_group", + &ui_pipeline.view_layout, + &BindGroupEntries::single(view_binding), + )); // Vertex buffer index let mut index = 0; @@ -851,24 +848,14 @@ pub fn prepare_uinodes( .values .entry(batch_image_handle) .or_insert_with(|| { - render_device.create_bind_group(&BindGroupDescriptor { - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView( - &gpu_image.texture_view, - ), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler( - &gpu_image.sampler, - ), - }, - ], - label: Some("ui_material_bind_group"), - layout: &ui_pipeline.image_layout, - }) + render_device.create_bind_group( + "ui_material_bind_group", + &ui_pipeline.image_layout, + &BindGroupEntries::sequential(( + &gpu_image.texture_view, + &gpu_image.sampler, + )), + ) }); existing_batch = batches.last_mut(); diff --git a/crates/bevy_utils/macros/src/lib.rs b/crates/bevy_utils/macros/src/lib.rs index 26fb9839919bd..4189b432ac605 100644 --- a/crates/bevy_utils/macros/src/lib.rs +++ b/crates/bevy_utils/macros/src/lib.rs @@ -136,3 +136,38 @@ pub fn all_tuples(input: TokenStream) -> TokenStream { )* }) } + +#[proc_macro] +pub fn all_tuples_with_size(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as AllTuples); + let len = 1 + input.end - input.start; + let mut ident_tuples = Vec::with_capacity(len); + for i in 0..=len { + let idents = input + .idents + .iter() + .map(|ident| format_ident!("{}{}", ident, i)); + if input.idents.len() < 2 { + ident_tuples.push(quote! { + #(#idents)* + }); + } else { + ident_tuples.push(quote! { + (#(#idents),*) + }); + } + } + + let macro_ident = &input.macro_ident; + let invocations = (input.start..=input.end).map(|i| { + let ident_tuples = &ident_tuples[..i]; + quote! { + #macro_ident!(#i, #(#ident_tuples),*); + } + }); + TokenStream::from(quote! { + #( + #invocations + )* + }) +} diff --git a/examples/shader/compute_shader_game_of_life.rs b/examples/shader/compute_shader_game_of_life.rs index 10c368771474b..2f8a269e592b8 100644 --- a/examples/shader/compute_shader_game_of_life.rs +++ b/examples/shader/compute_shader_game_of_life.rs @@ -107,14 +107,11 @@ fn prepare_bind_group( render_device: Res, ) { let view = gpu_images.get(&game_of_life_image.0).unwrap(); - let bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: None, - layout: &pipeline.texture_bind_group_layout, - entries: &[BindGroupEntry { - binding: 0, - resource: BindingResource::TextureView(&view.texture_view), - }], - }); + let bind_group = render_device.create_bind_group( + None, + &pipeline.texture_bind_group_layout, + &BindGroupEntries::single(&view.texture_view), + ); commands.insert_resource(GameOfLifeImageBindGroup(bind_group)); } diff --git a/examples/shader/post_processing.rs b/examples/shader/post_processing.rs index 94d3096e6a7c4..66d26cffbc271 100644 --- a/examples/shader/post_processing.rs +++ b/examples/shader/post_processing.rs @@ -20,12 +20,12 @@ use bevy::{ NodeRunError, RenderGraphApp, RenderGraphContext, ViewNode, ViewNodeRunner, }, render_resource::{ - BindGroupDescriptor, BindGroupEntry, BindGroupLayout, BindGroupLayoutDescriptor, - BindGroupLayoutEntry, BindingResource, BindingType, CachedRenderPipelineId, - ColorTargetState, ColorWrites, FragmentState, MultisampleState, Operations, - PipelineCache, PrimitiveState, RenderPassColorAttachment, RenderPassDescriptor, - RenderPipelineDescriptor, Sampler, SamplerBindingType, SamplerDescriptor, ShaderStages, - ShaderType, TextureFormat, TextureSampleType, TextureViewDimension, + BindGroupEntries, BindGroupLayout, BindGroupLayoutDescriptor, BindGroupLayoutEntry, + BindingType, CachedRenderPipelineId, ColorTargetState, ColorWrites, FragmentState, + MultisampleState, Operations, PipelineCache, PrimitiveState, RenderPassColorAttachment, + RenderPassDescriptor, RenderPipelineDescriptor, Sampler, SamplerBindingType, + SamplerDescriptor, ShaderStages, ShaderType, TextureFormat, TextureSampleType, + TextureViewDimension, }, renderer::{RenderContext, RenderDevice}, texture::BevyDefault, @@ -176,30 +176,19 @@ impl ViewNode for PostProcessNode { // The reason it doesn't work is because each post_process_write will alternate the source/destination. // The only way to have the correct source/destination for the bind_group // is to make sure you get it during the node execution. - let bind_group = render_context - .render_device() - .create_bind_group(&BindGroupDescriptor { - label: Some("post_process_bind_group"), - layout: &post_process_pipeline.layout, - // It's important for this to match the BindGroupLayout defined in the PostProcessPipeline - entries: &[ - BindGroupEntry { - binding: 0, - // Make sure to use the source view - resource: BindingResource::TextureView(post_process.source), - }, - BindGroupEntry { - binding: 1, - // Use the sampler created for the pipeline - resource: BindingResource::Sampler(&post_process_pipeline.sampler), - }, - BindGroupEntry { - binding: 2, - // Set the settings binding - resource: settings_binding.clone(), - }, - ], - }); + let bind_group = render_context.render_device().create_bind_group( + "post_process_bind_group", + &post_process_pipeline.layout, + // It's important for this to match the BindGroupLayout defined in the PostProcessPipeline + &BindGroupEntries::sequential(( + // Make sure to use the source view + post_process.source, + // Use the sampler created for the pipeline + &post_process_pipeline.sampler, + // Set the settings binding + settings_binding.clone(), + )), + ); // Begin the render pass let mut render_pass = render_context.begin_tracked_render_pass(RenderPassDescriptor { diff --git a/examples/shader/texture_binding_array.rs b/examples/shader/texture_binding_array.rs index bcce8ccac7a8e..82bcf34acc6ab 100644 --- a/examples/shader/texture_binding_array.rs +++ b/examples/shader/texture_binding_array.rs @@ -5,11 +5,8 @@ use bevy::{ prelude::*, reflect::TypePath, render::{ - render_asset::RenderAssets, - render_resource::{AsBindGroupError, PreparedBindGroup, *}, - renderer::RenderDevice, - texture::FallbackImage, - RenderApp, + render_asset::RenderAssets, render_resource::*, renderer::RenderDevice, + texture::FallbackImage, RenderApp, }, }; use std::{num::NonZeroU32, process::exit}; @@ -119,20 +116,11 @@ impl AsBindGroup for BindlessMaterial { textures[id] = &*image.texture_view; } - let bind_group = render_device.create_bind_group(&BindGroupDescriptor { - label: "bindless_material_bind_group".into(), + let bind_group = render_device.create_bind_group( + "bindless_material_bind_group", layout, - entries: &[ - BindGroupEntry { - binding: 0, - resource: BindingResource::TextureViewArray(&textures[..]), - }, - BindGroupEntry { - binding: 1, - resource: BindingResource::Sampler(&fallback_image.sampler), - }, - ], - }); + &BindGroupEntries::sequential((&textures[..], &fallback_image.sampler)), + ); Ok(PreparedBindGroup { bindings: vec![], From 38e0a8010e57b872b7ab33fdf38d347db183907c Mon Sep 17 00:00:00 2001 From: Rob Parrett Date: Sat, 21 Oct 2023 10:38:15 -0700 Subject: [PATCH 02/11] Tidy up UI node docs (#10189) # Objective While reviewing #10187 I noticed some other mistakes in the UI node docs. ## Solution I did a quick proofreading pass and fixed a few things. And of course, the typo from that other PR. ## Notes I occasionally insert a period to make a section of doc self-consistent but didn't go one way or the other on all periods in the file. --------- Co-authored-by: Noah --- crates/bevy_ui/src/ui_node.rs | 182 ++++++++++++++++++---------------- 1 file changed, 98 insertions(+), 84 deletions(-) diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index 3a87765fea840..81769141d4c17 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -14,29 +14,34 @@ use thiserror::Error; #[derive(Component, Debug, Copy, Clone, Reflect)] #[reflect(Component, Default)] pub struct Node { - /// The size of the node as width and height in logical pixels - /// automatically calculated by [`super::layout::ui_layout_system`] + /// The size of the node as width and height in logical pixels. + /// + /// Automatically calculated by [`super::layout::ui_layout_system`]. pub(crate) calculated_size: Vec2, - /// The width of this node's outline - /// If this value is `Auto`, negative or `0.` then no outline will be rendered - /// automatically calculated by [`super::layout::resolve_outlines_system`] + /// The width of this node's outline. + /// If this value is `Auto`, negative or `0.` then no outline will be rendered. + /// + /// Automatically calculated by [`super::layout::resolve_outlines_system`]. pub(crate) outline_width: f32, - // The amount of space between the outline and the edge of the node + /// The amount of space between the outline and the edge of the node. pub(crate) outline_offset: f32, - /// The unrounded size of the node as width and height in logical pixels - /// automatically calculated by [`super::layout::ui_layout_system`] + /// The unrounded size of the node as width and height in logical pixels. + /// + /// Automatically calculated by [`super::layout::ui_layout_system`]. pub(crate) unrounded_size: Vec2, } impl Node { - /// The calculated node size as width and height in logical pixels - /// automatically calculated by [`super::layout::ui_layout_system`] + /// The calculated node size as width and height in logical pixels. + /// + /// Automatically calculated by [`super::layout::ui_layout_system`]. pub const fn size(&self) -> Vec2 { self.calculated_size } - /// The calculated node size as width and height in logical pixels before rounding - /// automatically calculated by [`super::layout::ui_layout_system`] + /// The calculated node size as width and height in logical pixels before rounding. + /// + /// Automatically calculated by [`super::layout::ui_layout_system`]. pub const fn unrounded_size(&self) -> Vec2 { self.unrounded_size } @@ -102,7 +107,8 @@ impl Default for Node { /// Describes the style of a UI container node /// -/// Node's can be laid out using either Flexbox or CSS Grid Layout.
+/// Nodes can be laid out using either Flexbox or CSS Grid Layout. +/// /// See below for general learning resources and for documentation on the individual style properties. /// /// ### Flexbox @@ -128,7 +134,7 @@ pub struct Style { /// pub display: Display, - /// Whether a node should be laid out in-flow with, or independently of it's siblings: + /// Whether a node should be laid out in-flow with, or independently of its siblings: /// - [`PositionType::Relative`]: Layout this node in-flow with other nodes using the usual (flexbox/grid) layout algorithm. /// - [`PositionType::Absolute`]: Layout this node on top and independently of other nodes. /// @@ -140,9 +146,10 @@ pub struct Style { /// pub overflow: Overflow, - /// Defines the text direction. For example English is written LTR (left-to-right) while Arabic is written RTL (right-to-left). + /// Defines the text direction. For example, English is written LTR (left-to-right) while Arabic is written RTL (right-to-left). + /// + /// Note: the corresponding CSS property also affects box layout order, but this isn't yet implemented in Bevy. /// - /// Note: the corresponding CSS property also affects box layout order, but this isn't yet implemented in bevy. /// pub direction: Direction, @@ -184,12 +191,12 @@ pub struct Style { /// pub height: Val, - /// The minimum width of the node. `min_width` is used if it is greater than either `width` and/or `max_width`. + /// The minimum width of the node. `min_width` is used if it is greater than `width` and/or `max_width`. /// /// pub min_width: Val, - /// The minimum height of the node. `min_height` is used if it is greater than either `height` and/or `max_height`. + /// The minimum height of the node. `min_height` is used if it is greater than `height` and/or `max_height`. /// /// pub min_height: Val, @@ -226,7 +233,7 @@ pub struct Style { pub justify_items: JustifyItems, /// - For Flexbox items, controls cross-axis alignment of the item. - /// - For CSS Grid items, controls block (vertical) axis alignment of a grid item within it's grid area. + /// - For CSS Grid items, controls block (vertical) axis alignment of a grid item within its grid area. /// /// If set to `Auto`, alignment is inherited from the value of [`AlignItems`] set on the parent node. /// @@ -234,11 +241,11 @@ pub struct Style { pub align_self: AlignSelf, /// - For Flexbox items, this property has no effect. See `justify_content` for main-axis alignment of flex items. - /// - For CSS Grid items, controls inline (horizontal) axis alignment of a grid item within it's grid area. + /// - For CSS Grid items, controls inline (horizontal) axis alignment of a grid item within its grid area. /// /// If set to `Auto`, alignment is inherited from the value of [`JustifyItems`] set on the parent node. /// - /// + /// pub justify_self: JustifySelf, /// - For Flexbox containers, controls alignment of lines if flex_wrap is set to [`FlexWrap::Wrap`] and there are multiple lines of items. @@ -270,7 +277,7 @@ pub struct Style { /// ..Default::default() /// }; /// ``` - /// A node with this style and a parent with dimensions of 100px by 300px, will have calculated margins of 10px on both left and right edges, and 15px on both top and bottom edges. + /// A node with this style and a parent with dimensions of 100px by 300px will have calculated margins of 10px on both left and right edges, and 15px on both top and bottom edges. /// /// pub margin: UiRect, @@ -292,7 +299,7 @@ pub struct Style { /// ..Default::default() /// }; /// ``` - /// A node with this style and a parent with dimensions of 300px by 100px, will have calculated padding of 3px on the left, 6px on the right, 9px on the top and 12px on the bottom. + /// A node with this style and a parent with dimensions of 300px by 100px will have calculated padding of 3px on the left, 6px on the right, 9px on the top and 12px on the bottom. /// /// pub padding: UiRect, @@ -306,12 +313,12 @@ pub struct Style { /// pub border: UiRect, - /// Whether a Flexbox container should be a row or a column. This property has no effect of Grid nodes. + /// Whether a Flexbox container should be a row or a column. This property has no effect on Grid nodes. /// /// pub flex_direction: FlexDirection, - /// Whether a Flexbox container should wrap it's contents onto multiple line wrap if they overflow. This property has no effect of Grid nodes. + /// Whether a Flexbox container should wrap its contents onto multiple lines if they overflow. This property has no effect on Grid nodes. /// /// pub flex_wrap: FlexWrap, @@ -328,27 +335,27 @@ pub struct Style { /// The initial length of a flexbox in the main axis, before flex growing/shrinking properties are applied. /// - /// `flex_basis` overrides `size` on the main axis if both are set, but it obeys the bounds defined by `min_size` and `max_size`. + /// `flex_basis` overrides `size` on the main axis if both are set, but it obeys the bounds defined by `min_size` and `max_size`. /// /// pub flex_basis: Val, - /// The size of the gutters between items in a vertical flexbox layout or between rows in a grid layout + /// The size of the gutters between items in a vertical flexbox layout or between rows in a grid layout. /// /// Note: Values of `Val::Auto` are not valid and are treated as zero. /// /// pub row_gap: Val, - /// The size of the gutters between items in a horizontal flexbox layout or between column in a grid layout + /// The size of the gutters between items in a horizontal flexbox layout or between column in a grid layout. /// /// Note: Values of `Val::Auto` are not valid and are treated as zero. /// /// pub column_gap: Val, - /// Controls whether automatically placed grid items are placed row-wise or column-wise. And whether the sparse or dense packing algorithm is used. - /// Only affect Grid layouts + /// Controls whether automatically placed grid items are placed row-wise or column-wise as well as whether the sparse or dense packing algorithm is used. + /// Only affects Grid layouts. /// /// pub grid_auto_flow: GridAutoFlow, @@ -373,7 +380,7 @@ pub struct Style { /// Defines the size of implicitly created columns. Columns are created implicitly when grid items are given explicit placements that are out of bounds /// of the columns explicitly created using `grid_template_columns`. /// - /// + /// pub grid_auto_columns: Vec, /// The row in which a grid item starts and how many rows it spans. @@ -440,7 +447,7 @@ impl Default for Style { #[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] #[reflect(PartialEq, Serialize, Deserialize)] pub enum AlignItems { - /// The items are packed in their default position as if no alignment was applied + /// The items are packed in their default position as if no alignment was applied. Default, /// Items are packed towards the start of the axis. Start, @@ -474,7 +481,7 @@ impl Default for AlignItems { #[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] #[reflect(PartialEq, Serialize, Deserialize)] pub enum JustifyItems { - /// The items are packed in their default position as if no alignment was applied + /// The items are packed in their default position as if no alignment was applied. Default, /// Items are packed towards the start of the axis. Start, @@ -568,7 +575,7 @@ impl Default for JustifySelf { #[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] #[reflect(PartialEq, Serialize, Deserialize)] pub enum AlignContent { - /// The items are packed in their default position as if no alignment was applied + /// The items are packed in their default position as if no alignment was applied. Default, /// Each line moves towards the start of the cross axis. Start, @@ -582,13 +589,13 @@ pub enum AlignContent { Center, /// Each line will stretch to fill the remaining space. Stretch, - /// Each line fills the space it needs, putting the remaining space, if any - /// inbetween the lines. + /// Each line fills the space it needs, putting the remaining space, if any, + /// between the lines. SpaceBetween, - /// The gap between the first and last items is exactly THE SAME as the gap between items. + /// The gap between the first and last items is exactly the same as the gap between items. /// The gaps are distributed evenly. SpaceEvenly, - /// Each line fills the space it needs, putting the remaining space, if any + /// Each line fills the space it needs, putting the remaining space, if any, /// around the lines. SpaceAround, } @@ -607,7 +614,7 @@ impl Default for AlignContent { #[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] #[reflect(PartialEq, Serialize, Deserialize)] pub enum JustifyContent { - /// The items are packed in their default position as if no alignment was applied + /// The items are packed in their default position as if no alignment was applied. Default, /// Items are packed toward the start of the axis. Start, @@ -637,9 +644,9 @@ impl Default for JustifyContent { } } -/// Defines the text direction +/// Defines the text direction. /// -/// For example English is written LTR (left-to-right) while Arabic is written RTL (right-to-left). +/// For example, English is written LTR (left-to-right) while Arabic is written RTL (right-to-left). #[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] #[reflect(PartialEq, Serialize, Deserialize)] pub enum Direction { @@ -661,7 +668,7 @@ impl Default for Direction { } } -/// Whether to use a Flexbox layout model. +/// Defines the layout model used by this node. /// /// Part of the [`Style`] component. #[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] @@ -803,9 +810,7 @@ impl Default for OverflowAxis { pub enum PositionType { /// Relative to all other nodes with the [`PositionType::Relative`] value. Relative, - /// Independent of all other nodes. - /// - /// As usual, the `Style.position` field of this node is specified relative to its parent node. + /// Independent of all other nodes, but relative to its parent node. Absolute, } @@ -841,17 +846,18 @@ impl Default for FlexWrap { } } -/// Controls whether grid items are placed row-wise or column-wise. And whether the sparse or dense packing algorithm is used. +/// Controls whether grid items are placed row-wise or column-wise as well as whether the sparse or dense packing algorithm is used. /// -/// The "dense" packing algorithm attempts to fill in holes earlier in the grid, if smaller items come up later. This may cause items to appear out-of-order, when doing so would fill in holes left by larger items. +/// The "dense" packing algorithm attempts to fill in holes earlier in the grid, if smaller items come up later. +/// This may cause items to appear out-of-order when doing so would fill in holes left by larger items. /// -/// Defaults to [`GridAutoFlow::Row`] +/// Defaults to [`GridAutoFlow::Row`]. /// /// #[derive(Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Reflect)] #[reflect(PartialEq, Serialize, Deserialize)] pub enum GridAutoFlow { - /// Items are placed by filling each row in turn, adding new rows as necessary + /// Items are placed by filling each row in turn, adding new rows as necessary. Row, /// Items are placed by filling each column in turn, adding new columns as necessary. Column, @@ -904,7 +910,8 @@ pub enum MaxTrackSizingFunction { /// Track maximum size should be automatically sized Auto, /// The dimension as a fraction of the total available grid space (`fr` units in CSS) - /// Specified value is the numerator of the fraction. Denominator is the sum of all fractions specified in that grid dimension + /// Specified value is the numerator of the fraction. Denominator is the sum of all fractions specified in that grid dimension. + /// /// Spec: Fraction(f32), } @@ -944,7 +951,7 @@ impl GridTrack { /// Create a grid track with an `fr` size. /// Note that this will give the track a content-based minimum size. - /// Usually you are best off using `GridTrack::flex` instead which uses a zero minimum size + /// Usually you are best off using `GridTrack::flex` instead which uses a zero minimum size. pub fn fr>(value: f32) -> T { Self { min_sizing_function: MinTrackSizingFunction::Auto, @@ -953,7 +960,7 @@ impl GridTrack { .into() } - /// Create a grid track with an `minmax(0, Nfr)` size. + /// Create a grid track with a `minmax(0, Nfr)` size. pub fn flex>(value: f32) -> T { Self { min_sizing_function: MinTrackSizingFunction::Px(0.0), @@ -962,7 +969,7 @@ impl GridTrack { .into() } - /// Create a grid track which is automatically sized to fit it's contents, and then + /// Create a grid track which is automatically sized to fit its contents. pub fn auto>() -> T { Self { min_sizing_function: MinTrackSizingFunction::Auto, @@ -971,7 +978,7 @@ impl GridTrack { .into() } - /// Create a grid track which is automatically sized to fit it's contents when sized at their "min-content" sizes + /// Create a grid track which is automatically sized to fit its contents when sized at their "min-content" sizes pub fn min_content>() -> T { Self { min_sizing_function: MinTrackSizingFunction::MinContent, @@ -980,7 +987,7 @@ impl GridTrack { .into() } - /// Create a grid track which is automatically sized to fit it's contents when sized at their "max-content" sizes + /// Create a grid track which is automatically sized to fit its contents when sized at their "max-content" sizes pub fn max_content>() -> T { Self { min_sizing_function: MinTrackSizingFunction::MaxContent, @@ -1070,14 +1077,14 @@ impl From for GridTrackRepetition { /// /// The repetition parameter can either be: /// - The integer `1`, in which case the track is non-repeated. -/// - a `u16` count to repeat the track N times -/// - A `GridTrackRepetition::AutoFit` or `GridTrackRepetition::AutoFill` +/// - a `u16` count to repeat the track N times. +/// - A `GridTrackRepetition::AutoFit` or `GridTrackRepetition::AutoFill`. /// /// Note: that in the common case you want a non-repeating track (repetition count 1), you may use the constructor methods on [`GridTrack`] /// to create a `RepeatedGridTrack`. i.e. `GridTrack::px(10.0)` is equivalent to `RepeatedGridTrack::px(1, 10.0)`. /// /// You may only use one auto-repetition per track list. And if your track list contains an auto repetition -/// then all track (in and outside of the repetition) must be fixed size (px or percent). Integer repetitions are just shorthand for writing out +/// then all tracks (in and outside of the repetition) must be fixed size (px or percent). Integer repetitions are just shorthand for writing out /// N tracks longhand and are not subject to the same limitations. #[derive(Clone, PartialEq, Debug, Serialize, Deserialize, Reflect)] #[reflect(PartialEq, Serialize, Deserialize)] @@ -1116,7 +1123,7 @@ impl RepeatedGridTrack { /// Create a repeating set of grid tracks with an `fr` size. /// Note that this will give the track a content-based minimum size. - /// Usually you are best off using `GridTrack::flex` instead which uses a zero minimum size + /// Usually you are best off using `GridTrack::flex` instead which uses a zero minimum size. pub fn fr>(repetition: u16, value: f32) -> T { Self { repetition: GridTrackRepetition::Count(repetition), @@ -1125,7 +1132,7 @@ impl RepeatedGridTrack { .into() } - /// Create a repeating set of grid tracks with an `minmax(0, Nfr)` size. + /// Create a repeating set of grid tracks with a `minmax(0, Nfr)` size. pub fn flex>(repetition: u16, value: f32) -> T { Self { repetition: GridTrackRepetition::Count(repetition), @@ -1245,11 +1252,18 @@ impl From for Vec { /// /// pub struct GridPlacement { - /// The grid line at which the item should start. Lines are 1-indexed. Negative indexes count backwards from the end of the grid. Zero is not a valid index. + /// The grid line at which the item should start. + /// Lines are 1-indexed. + /// Negative indexes count backwards from the end of the grid. + /// Zero is not a valid index. pub(crate) start: Option, - /// How many grid tracks the item should span. Defaults to 1. + /// How many grid tracks the item should span. + /// Defaults to 1. pub(crate) span: Option, - /// The grid line at which the item should end. Lines are 1-indexed. Negative indexes count backwards from the end of the grid. Zero is not a valid index. + /// The grid line at which the item should end. + /// Lines are 1-indexed. + /// Negative indexes count backwards from the end of the grid. + /// Zero is not a valid index. pub(crate) end: Option, } @@ -1269,7 +1283,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `span` is `0` + /// Panics if `span` is `0`. pub fn span(span: u16) -> Self { Self { start: None, @@ -1282,7 +1296,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `start` is `0` + /// Panics if `start` is `0`. pub fn start(start: i16) -> Self { Self { start: try_into_grid_index(start).expect("Invalid start value of 0."), @@ -1294,7 +1308,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `end` is `0` + /// Panics if `end` is `0`. pub fn end(end: i16) -> Self { Self { end: try_into_grid_index(end).expect("Invalid end value of 0."), @@ -1306,7 +1320,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `start` or `span` is `0` + /// Panics if `start` or `span` is `0`. pub fn start_span(start: i16, span: u16) -> Self { Self { start: try_into_grid_index(start).expect("Invalid start value of 0."), @@ -1319,7 +1333,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `start` or `end` is `0` + /// Panics if `start` or `end` is `0`. pub fn start_end(start: i16, end: i16) -> Self { Self { start: try_into_grid_index(start).expect("Invalid start value of 0."), @@ -1332,7 +1346,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `end` or `span` is `0` + /// Panics if `end` or `span` is `0`. pub fn end_span(end: i16, span: u16) -> Self { Self { start: None, @@ -1345,7 +1359,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `start` is `0` + /// Panics if `start` is `0`. pub fn set_start(mut self, start: i16) -> Self { self.start = try_into_grid_index(start).expect("Invalid start value of 0."); self @@ -1355,7 +1369,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `end` is `0` + /// Panics if `end` is `0`. pub fn set_end(mut self, end: i16) -> Self { self.end = try_into_grid_index(end).expect("Invalid end value of 0."); self @@ -1365,7 +1379,7 @@ impl GridPlacement { /// /// # Panics /// - /// Panics if `span` is `0` + /// Panics if `span` is `0`. pub fn set_span(mut self, span: u16) -> Self { self.span = try_into_grid_span(span).expect("Invalid span value of 0."); self @@ -1476,7 +1490,7 @@ impl Default for BorderColor { #[derive(Component, Copy, Clone, Default, Debug, Deserialize, Serialize, Reflect)] #[reflect(Component, Default, Deserialize, Serialize)] /// The [`Outline`] component adds an outline outside the edge of a UI node. -/// Outlines do not take up space in the layout +/// Outlines do not take up space in the layout. /// /// To add an [`Outline`] to a ui node you can spawn a `(NodeBundle, Outline)` tuple bundle: /// ``` @@ -1511,7 +1525,7 @@ impl Default for BorderColor { /// for (entity, interaction, mut maybe_outline) in node_query.iter_mut() { /// let outline_color = /// if matches!(*interaction, Interaction::Hovered) { -/// Color::WHITE +/// Color::WHITE /// } else { /// Color::NONE /// }; @@ -1528,16 +1542,16 @@ impl Default for BorderColor { pub struct Outline { /// The width of the outline. /// - /// Percentage `Val` values are resolved based on the width of the outlined [`Node`] + /// Percentage `Val` values are resolved based on the width of the outlined [`Node`]. pub width: Val, - /// The amount of space between a node's outline the edge of the node + /// The amount of space between a node's outline the edge of the node. /// - /// Percentage `Val` values are resolved based on the width of the outlined [`Node`] + /// Percentage `Val` values are resolved based on the width of the outlined [`Node`]. pub offset: Val, - /// Color of the outline + /// The color of the outline. /// /// If you are frequently toggling outlines for a UI node on and off it is recommended to set `Color::None` to hide the outline. - /// This avoids the table moves that would occcur from the repeated insertion and removal of the `Outline` component. + /// This avoids the table moves that would occur from the repeated insertion and removal of the `Outline` component. pub color: Color, } @@ -1572,14 +1586,14 @@ impl UiImage { } } - /// flip the image along its x-axis + /// Flip the image along its x-axis #[must_use] pub const fn with_flip_x(mut self) -> Self { self.flip_x = true; self } - /// flip the image along its y-axis + /// Flip the image along its y-axis #[must_use] pub const fn with_flip_y(mut self) -> Self { self.flip_y = true; @@ -1607,13 +1621,13 @@ pub struct CalculatedClip { /// /// UI nodes that have the same z-index will appear according to the order in which they /// appear in the UI hierarchy. In such a case, the last node to be added to its parent -/// will appear in front of this parent's other children. +/// will appear in front of its siblings. /// /// Internally, nodes with a global z-index share the stacking context of root UI nodes /// (nodes that have no parent). Because of this, there is no difference between using -/// [`ZIndex::Local(n)`] and [`ZIndex::Global(n)`] for root nodes. +/// `ZIndex::Local(n)` and `ZIndex::Global(n)` for root nodes. /// -/// Nodes without this component will be treated as if they had a value of [`ZIndex::Local(0)`]. +/// Nodes without this component will be treated as if they had a value of `ZIndex::Local(0)`. #[derive(Component, Copy, Clone, Debug, Reflect)] #[reflect(Component)] pub enum ZIndex { From 0716922165183968bd57fef93f559559cc9a976f Mon Sep 17 00:00:00 2001 From: Joseph <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 21 Oct 2023 11:07:52 -0700 Subject: [PATCH 03/11] `ParamSet`s containing non-send parameters should also be non-send (#10211) # Objective Fix #10207 ## Solution Mark a `ParamSet`'s `SystemMeta` as non-send if any of its component parameters are non-send. --- crates/bevy_ecs/macros/src/lib.rs | 4 +++ crates/bevy_ecs/src/system/system_param.rs | 30 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 28d63c6911b14..067191595405c 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -201,6 +201,10 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { #param::init_state(world, &mut #meta); let #param = #param::init_state(world, &mut system_meta.clone()); )* + // Make the ParamSet non-send if any of its parameters are non-send. + if false #(|| !#meta.is_send())* { + system_meta.set_non_send(); + } #( system_meta .component_access_set diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index e5343002df95a..2b5d73d1ccfe0 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1739,4 +1739,34 @@ mod tests { schedule.add_systems(non_sync_system); schedule.run(&mut world); } + + // Regression test for https://github.com/bevyengine/bevy/issues/10207. + #[test] + fn param_set_non_send_first() { + fn non_send_param_set(mut p: ParamSet<(NonSend<*mut u8>, ())>) { + let _ = p.p0(); + p.p1(); + } + + let mut world = World::new(); + world.insert_non_send_resource(std::ptr::null_mut::()); + let mut schedule = crate::schedule::Schedule::default(); + schedule.add_systems((non_send_param_set, non_send_param_set, non_send_param_set)); + schedule.run(&mut world); + } + + // Regression test for https://github.com/bevyengine/bevy/issues/10207. + #[test] + fn param_set_non_send_second() { + fn non_send_param_set(mut p: ParamSet<((), NonSendMut<*mut u8>)>) { + p.p0(); + let _ = p.p1(); + } + + let mut world = World::new(); + world.insert_non_send_resource(std::ptr::null_mut::()); + let mut schedule = crate::schedule::Schedule::default(); + schedule.add_systems((non_send_param_set, non_send_param_set, non_send_param_set)); + schedule.run(&mut world); + } } From 9cfada3f22ad99ff378b8128553ee347864ca038 Mon Sep 17 00:00:00 2001 From: Kanabenki Date: Sat, 21 Oct 2023 21:10:37 +0200 Subject: [PATCH 04/11] Detect cubemap for dds textures (#10222) # Objective - Closes #10049. - Detect DDS texture containing a cubemap or a cubemap array. ## Solution - When loading a dds texture, the header capabilities are checked for the cubemap flag. An error is returned if not all faces are provided. --- ## Changelog ### Added - Added a new texture error `TextureError::IncompleteCubemap`, used for dds cubemap textures containing less than 6 faces, as that is not supported on modern graphics APIs. ### Fixed - DDS cubemaps are now loaded as cubemaps instead of 2D textures. ## Migration Guide If you are matching on a `TextureError`, you will need to add a new branch to handle `TextureError::IncompleteCubemap`. --- crates/bevy_render/src/texture/dds.rs | 42 ++++++++++++++++++++----- crates/bevy_render/src/texture/image.rs | 3 ++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/crates/bevy_render/src/texture/dds.rs b/crates/bevy_render/src/texture/dds.rs index 6f773193ab9a0..16f1aa7240f97 100644 --- a/crates/bevy_render/src/texture/dds.rs +++ b/crates/bevy_render/src/texture/dds.rs @@ -1,6 +1,8 @@ -use ddsfile::{D3DFormat, Dds, DxgiFormat}; +use ddsfile::{Caps2, D3DFormat, Dds, DxgiFormat}; use std::io::Cursor; -use wgpu::{Extent3d, TextureDimension, TextureFormat}; +use wgpu::{ + Extent3d, TextureDimension, TextureFormat, TextureViewDescriptor, TextureViewDimension, +}; use super::{CompressedImageFormats, Image, TextureError}; @@ -18,14 +20,29 @@ pub fn dds_buffer_to_image( ))); } let mut image = Image::default(); + let is_cubemap = dds.header.caps2.contains(Caps2::CUBEMAP); + let mut depth_or_array_layers = if dds.get_num_array_layers() > 1 { + dds.get_num_array_layers() + } else { + dds.get_depth() + }; + if is_cubemap { + if !dds.header.caps2.contains( + Caps2::CUBEMAP_NEGATIVEX + | Caps2::CUBEMAP_NEGATIVEY + | Caps2::CUBEMAP_NEGATIVEZ + | Caps2::CUBEMAP_POSITIVEX + | Caps2::CUBEMAP_POSITIVEY + | Caps2::CUBEMAP_POSITIVEZ, + ) { + return Err(TextureError::IncompleteCubemap); + } + depth_or_array_layers *= 6; + } image.texture_descriptor.size = Extent3d { width: dds.get_width(), height: dds.get_height(), - depth_or_array_layers: if dds.get_num_array_layers() > 1 { - dds.get_num_array_layers() - } else { - dds.get_depth() - }, + depth_or_array_layers, } .physical_size(texture_format); image.texture_descriptor.mip_level_count = dds.get_num_mipmap_levels(); @@ -37,6 +54,17 @@ pub fn dds_buffer_to_image( } else { TextureDimension::D1 }; + if is_cubemap { + let dimension = if image.texture_descriptor.size.depth_or_array_layers > 6 { + TextureViewDimension::CubeArray + } else { + TextureViewDimension::Cube + }; + image.texture_view_descriptor = Some(TextureViewDescriptor { + dimension: Some(dimension), + ..Default::default() + }); + } image.data = dds.data; Ok(image) } diff --git a/crates/bevy_render/src/texture/image.rs b/crates/bevy_render/src/texture/image.rs index d14388d87504c..8adf57267a2fe 100644 --- a/crates/bevy_render/src/texture/image.rs +++ b/crates/bevy_render/src/texture/image.rs @@ -438,6 +438,9 @@ pub enum TextureError { TranscodeError(String), #[error("format requires transcoding: {0:?}")] FormatRequiresTranscodingError(TranscodeFormat), + /// Only cubemaps with six faces are supported. + #[error("only cubemaps with six faces are supported")] + IncompleteCubemap, } /// The type of a raw image buffer. From c3627248f5bdac9b00666b9170e6bfa575794631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Sun, 22 Oct 2023 00:19:46 +0200 Subject: [PATCH 05/11] Fix alignment on ios simulator (#10178) # Objective - Fix #10165 - On iOS simulator on apple silicon Macs, shader validation is going through the host, but device limits are reported for the device. They sometimes differ, and cause the validation to crash on something that should work ``` -[MTLDebugRenderCommandEncoder validateCommonDrawErrors:]:5775: failed assertion `Draw Errors Validation Fragment Function(fragment_): the offset into the buffer _naga_oil_mod_MJSXM6K7OBRHEOR2NVSXG2C7OZUWK527MJUW4ZDJNZTXG_memberfog that is bound at buffer index 6 must be a multiple of 256 but was set to 448. ``` ## Solution - Add a custom flag when building for the simulator and override the buffer alignment --- .../src/render_resource/uniform_buffer.rs | 12 ++++++++++-- examples/mobile/.cargo/config.toml | 5 +++++ examples/mobile/build_rust_deps.sh | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 examples/mobile/.cargo/config.toml diff --git a/crates/bevy_render/src/render_resource/uniform_buffer.rs b/crates/bevy_render/src/render_resource/uniform_buffer.rs index 95196568ff20a..7e1b86869c4e0 100644 --- a/crates/bevy_render/src/render_resource/uniform_buffer.rs +++ b/crates/bevy_render/src/render_resource/uniform_buffer.rs @@ -277,8 +277,16 @@ impl DynamicUniformBuffer { device: &RenderDevice, queue: &'a RenderQueue, ) -> Option> { - let alignment = - AlignmentValue::new(device.limits().min_uniform_buffer_offset_alignment as u64); + let alignment = if cfg!(ios_simulator) { + // On iOS simulator on silicon macs, metal validation check that the host OS alignment + // is respected, but the device reports the correct value for iOS, which is smaller. + // Use the larger value. + // See https://github.com/bevyengine/bevy/pull/10178 - remove if it's not needed anymore. + AlignmentValue::new(256) + } else { + AlignmentValue::new(device.limits().min_uniform_buffer_offset_alignment as u64) + }; + let mut capacity = self.buffer.as_deref().map(wgpu::Buffer::size).unwrap_or(0); let size = alignment .round_up(T::min_size().get()) diff --git a/examples/mobile/.cargo/config.toml b/examples/mobile/.cargo/config.toml new file mode 100644 index 0000000000000..19a634093de05 --- /dev/null +++ b/examples/mobile/.cargo/config.toml @@ -0,0 +1,5 @@ +# Flag to notify the compiler we're building for the iOS simulator from an Apple silicon mac +# This needs some workarounds for now +# See https://github.com/bevyengine/bevy/pull/10178 - remove if it's not needed anymore. +[target.aarch64-apple-ios-sim] +rustflags = ["--cfg=ios_simulator"] diff --git a/examples/mobile/build_rust_deps.sh b/examples/mobile/build_rust_deps.sh index 2914faad5c0d0..e232648a2be91 100755 --- a/examples/mobile/build_rust_deps.sh +++ b/examples/mobile/build_rust_deps.sh @@ -48,7 +48,7 @@ for arch in $ARCHS; do # Hardware iOS targets cargo rustc --crate-type staticlib --lib $RELFLAG --target aarch64-apple-ios else - # M1 iOS simulator -- currently in Nightly only and requires to build `libstd` + # M1 iOS simulator cargo rustc --crate-type staticlib --lib $RELFLAG --target aarch64-apple-ios-sim fi esac From 8efcbf3e4f1b645b6fefd979fa72e6e47f10dc6a Mon Sep 17 00:00:00 2001 From: st0rmbtw <61053971+st0rmbtw@users.noreply.github.com> Date: Sun, 22 Oct 2023 04:45:29 +0300 Subject: [PATCH 06/11] Add convenient methods for Image (#10221) # Objective To get the width or height of an image you do: ```rust self.texture_descriptor.size.{width, height} ``` that is quite verbose. This PR adds some convenient methods for Image to reduce verbosity. ## Changelog * Add a `width()` method for getting the width of an image. * Add a `height()` method for getting the height of an image. * Rename the `size()` method to `size_f32()`. * Add a `size()` method for getting the size of an image as u32. * Renamed the `aspect_2d()` method to `aspect_ratio()`. ## Migration Guide Replace calls to the `Image::size()` method with `size_f32()`. Replace calls to the `Image::aspect_2d()` method with `aspect_ratio()`. --- .../src/texture/compressed_image_saver.rs | 2 +- crates/bevy_render/src/texture/image.rs | 32 +++++++++++++------ crates/bevy_sprite/src/lib.rs | 2 +- examples/3d/tonemapping.rs | 2 +- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/crates/bevy_render/src/texture/compressed_image_saver.rs b/crates/bevy_render/src/texture/compressed_image_saver.rs index 26ab7d22785ce..a557447db3d46 100644 --- a/crates/bevy_render/src/texture/compressed_image_saver.rs +++ b/crates/bevy_render/src/texture/compressed_image_saver.rs @@ -40,7 +40,7 @@ impl AssetSaver for CompressedImageSaver { let mut source_image = compressor_params.source_image_mut(0); let size = image.size(); - source_image.init(&image.data, size.x as u32, size.y as u32, 4); + source_image.init(&image.data, size.x, size.y, 4); let mut compressor = basis_universal::Compressor::new(4); // SAFETY: the CompressorParams are "valid" to the best of our knowledge. The basis-universal diff --git a/crates/bevy_render/src/texture/image.rs b/crates/bevy_render/src/texture/image.rs index 8adf57267a2fe..aac9759278e1b 100644 --- a/crates/bevy_render/src/texture/image.rs +++ b/crates/bevy_render/src/texture/image.rs @@ -14,7 +14,7 @@ use crate::{ use bevy_asset::Asset; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::system::{lifetimeless::SRes, Resource, SystemParamItem}; -use bevy_math::Vec2; +use bevy_math::{UVec2, Vec2}; use bevy_reflect::Reflect; use serde::{Deserialize, Serialize}; use std::hash::Hash; @@ -255,16 +255,28 @@ impl Image { } /// Returns the aspect ratio (height/width) of a 2D image. - pub fn aspect_2d(&self) -> f32 { - self.texture_descriptor.size.height as f32 / self.texture_descriptor.size.width as f32 + pub fn aspect_ratio(&self) -> f32 { + self.height() as f32 / self.width() as f32 + } + + /// Returns the width of a 2D image. + pub fn width(&self) -> u32 { + self.texture_descriptor.size.width + } + + /// Returns the height of a 2D image. + pub fn height(&self) -> u32 { + self.texture_descriptor.size.height + } + + /// Returns the size of a 2D image as f32. + pub fn size_f32(&self) -> Vec2 { + Vec2::new(self.width() as f32, self.height() as f32) } /// Returns the size of a 2D image. - pub fn size(&self) -> Vec2 { - Vec2::new( - self.texture_descriptor.size.width as f32, - self.texture_descriptor.size.height as f32, - ) + pub fn size(&self) -> UVec2 { + UVec2::new(self.width(), self.height()) } /// Resizes the image to the new size, by removing information or appending 0 to the `data`. @@ -636,12 +648,12 @@ mod test { ); assert_eq!( Vec2::new(size.width as f32, size.height as f32), - image.size() + image.size_f32() ); } #[test] fn image_default_size() { let image = Image::default(); - assert_eq!(Vec2::ONE, image.size()); + assert_eq!(Vec2::ONE, image.size_f32()); } } diff --git a/crates/bevy_sprite/src/lib.rs b/crates/bevy_sprite/src/lib.rs index 1eb3b1a5cc64c..9f68a9a3b981e 100644 --- a/crates/bevy_sprite/src/lib.rs +++ b/crates/bevy_sprite/src/lib.rs @@ -138,7 +138,7 @@ pub fn calculate_bounds_2d( for (entity, sprite, texture_handle) in &sprites_without_aabb { if let Some(size) = sprite .custom_size - .or_else(|| images.get(texture_handle).map(|image| image.size())) + .or_else(|| images.get(texture_handle).map(|image| image.size_f32())) { let aabb = Aabb { center: (-sprite.anchor.as_vec() * size).extend(0.0).into(), diff --git a/examples/3d/tonemapping.rs b/examples/3d/tonemapping.rs index 595f3b0b4ecac..a7eb2973392ec 100644 --- a/examples/3d/tonemapping.rs +++ b/examples/3d/tonemapping.rs @@ -336,7 +336,7 @@ fn update_image_viewer( if let Some(base_color_texture) = mat.base_color_texture.clone() { if image_changed_id == base_color_texture.id() { if let Some(image_changed) = images.get(image_changed_id) { - let size = image_changed.size().normalize_or_zero() * 1.4; + let size = image_changed.size_f32().normalize_or_zero() * 1.4; // Resize Mesh let quad = Mesh::from(shape::Quad::new(size)); meshes.insert(mesh_h, quad); From 60773e6787d177e97458f9fcf118985906762b2a Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Sun, 22 Oct 2023 05:43:31 -0700 Subject: [PATCH 07/11] bevy_reflect: Fix ignored/skipped field order (#7575) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Fixes #5101 Alternative to #6511 ## Solution Corrected the behavior for ignored fields in `FromReflect`, which was previously using the incorrect field indexes. Similarly, fields marked with `#[reflect(skip_serializing)]` no longer break when using `FromReflect` after deserialization. This was done by modifying `SerializationData` to store a function pointer that can later be used to generate a default instance of the skipped field during deserialization. The function pointer points to a function generated by the derive macro using the behavior designated by `#[reflect(default)]` (or just `Default` if none provided). The entire output of the macro is now wrapped in an [unnamed constant](https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant) which keeps this behavior hygienic. #### Rationale The biggest downside to this approach is that it requires fields marked `#[reflect(skip_serializing)]` to provide the ability to create a default instance— either via a `Default` impl or by specifying a custom one. While this isn't great, I think it might be justified by the fact that we really need to create this value when using `FromReflect` on a deserialized object. And we need to do this _during_ deserialization because after that (at least for tuples and tuple structs) we lose information about which field is which: _"is the value at index 1 in this `DynamicTupleStruct` the actual value for index 1 or is it really the value for index 2 since index 1 is skippable...?"_ #### Alternatives An alternative would be to store `Option>` within `DynamicTuple` and `DynamicTupleStruct` instead of just `Box`. This would allow us to insert "empty"/"missing" fields during deserialization, thus saving the positional information of the skipped fields. However, this may require changing the API of `Tuple` and `TupleStruct` such that they can account for their dynamic counterparts returning `None` for a skipped field. In practice this would probably mean exposing the `Option`-ness of the dynamics onto implementors via methods like `Tuple::drain` or `TupleStruct::field`. Personally, I think requiring `Default` would be better than muddying up the API to account for these special cases. But I'm open to trying out this other approach if the community feels that it's better. --- ## Changelog ### Public Changes #### Fixed - The behaviors of `#[reflect(ignore)]` and `#[reflect(skip_serializing)]` are no longer dependent on field order #### Changed - Fields marked with `#[reflect(skip_serializing)]` now need to either implement `Default` or specify a custom default function using `#[reflect(default = "path::to::some_func")]` - Deserializing a type with fields marked `#[reflect(skip_serializing)]` will now include that field initialized to its specified default value - `SerializationData::new` now takes the new `SkippedField` struct along with the skipped field index - Renamed `SerializationData::is_ignored_field` to `SerializationData::is_field_skipped` #### Added - Added `SkippedField` struct - Added methods `SerializationData::generate_default` and `SerializationData::iter_skipped` ### Internal Changes #### Changed - Replaced `members_to_serialization_denylist` and `BitSet` with `SerializationDataDef` - The `Reflect` derive is more hygienic as it now outputs within an [unnamed constant](https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant) - `StructField::index` has been split up into `StructField::declaration_index` and `StructField::reflection_index` #### Removed - Removed `bitset` dependency ## Migration Guide * Fields marked `#[reflect(skip_serializing)]` now must implement `Default` or specify a custom default function with `#[reflect(default = "path::to::some_func")]` ```rust #[derive(Reflect)] struct MyStruct { #[reflect(skip_serializing)] #[reflect(default = "get_foo_default")] foo: Foo, // <- `Foo` does not impl `Default` so requires a custom function #[reflect(skip_serializing)] bar: Bar, // <- `Bar` impls `Default` } #[derive(Reflect)] struct Foo(i32); #[derive(Reflect, Default)] struct Bar(i32); fn get_foo_default() -> Foo { Foo(123) } ``` * `SerializationData::new` has been changed to expect an iterator of `(usize, SkippedField)` rather than one of just `usize` ```rust // BEFORE SerializationData::new([0, 3].into_iter()); // AFTER SerializationData::new([ (0, SkippedField::new(field_0_default_fn)), (3, SkippedField::new(field_3_default_fn)), ].into_iter()); ``` * `Serialization::is_ignored_field` has been renamed to `Serialization::is_field_skipped` * Fields marked `#[reflect(skip_serializing)]` are now included in deserialization output. This may affect logic that expected those fields to be absent. --- .../bevy_reflect_derive/Cargo.toml | 1 - .../bevy_reflect_derive/src/derive_data.rs | 62 ++-- .../bevy_reflect_derive/src/from_reflect.rs | 10 +- .../bevy_reflect_derive/src/impls/enums.rs | 6 +- .../bevy_reflect_derive/src/impls/structs.rs | 4 +- .../src/impls/tuple_structs.rs | 2 +- .../bevy_reflect_derive/src/lib.rs | 94 +++-- .../bevy_reflect_derive/src/registration.rs | 13 +- .../bevy_reflect_derive/src/serialization.rs | 91 +++++ .../bevy_reflect_derive/src/utility.rs | 38 -- crates/bevy_reflect/src/lib.rs | 33 ++ crates/bevy_reflect/src/serde/de.rs | 333 +++++++++--------- crates/bevy_reflect/src/serde/mod.rs | 67 +++- crates/bevy_reflect/src/serde/ser.rs | 4 +- crates/bevy_reflect/src/serde/type_data.rs | 132 +++++-- crates/bevy_reflect/src/tuple_struct.rs | 13 +- 16 files changed, 607 insertions(+), 296 deletions(-) create mode 100644 crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs diff --git a/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml b/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml index 5073873b638c5..bfb239e8105c5 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml +++ b/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml @@ -23,4 +23,3 @@ syn = { version = "2.0", features = ["full"] } proc-macro2 = "1.0" quote = "1.0" uuid = { version = "1.1", features = ["v4"] } -bit-set = "0.5.2" diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 0e5ef21dcaf69..59b3e2dd08a30 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,11 +1,11 @@ use crate::container_attributes::{FromReflectAttrs, ReflectTraits}; use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr}; use crate::type_path::parse_path_no_leading_colon; -use crate::utility::{members_to_serialization_denylist, StringExpr, WhereClauseOptions}; -use bit_set::BitSet; +use crate::utility::{StringExpr, WhereClauseOptions}; use quote::{quote, ToTokens}; use syn::token::Comma; +use crate::serialization::SerializationDataDef; use crate::{ utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME, TYPE_NAME_ATTRIBUTE_NAME, TYPE_PATH_ATTRIBUTE_NAME, @@ -65,7 +65,7 @@ pub(crate) struct ReflectMeta<'a> { /// ``` pub(crate) struct ReflectStruct<'a> { meta: ReflectMeta<'a>, - serialization_denylist: BitSet, + serialization_data: Option, fields: Vec>, } @@ -95,7 +95,14 @@ pub(crate) struct StructField<'a> { /// The reflection-based attributes on the field. pub attrs: ReflectFieldAttr, /// The index of this field within the struct. - pub index: usize, + pub declaration_index: usize, + /// The index of this field as seen by the reflection API. + /// + /// This index accounts for the removal of [ignored] fields. + /// It will only be `Some(index)` when the field is not ignored. + /// + /// [ignored]: crate::field_attributes::ReflectIgnoreBehavior::IgnoreAlways + pub reflection_index: Option, /// The documentation for this field, if any #[cfg(feature = "documentation")] pub doc: crate::documentation::Documentation, @@ -272,9 +279,7 @@ impl<'a> ReflectDerive<'a> { let fields = Self::collect_struct_fields(&data.fields)?; let reflect_struct = ReflectStruct { meta, - serialization_denylist: members_to_serialization_denylist( - fields.iter().map(|v| v.attrs.ignore), - ), + serialization_data: SerializationDataDef::new(&fields)?, fields, }; @@ -308,19 +313,31 @@ impl<'a> ReflectDerive<'a> { } fn collect_struct_fields(fields: &'a Fields) -> Result>, syn::Error> { + let mut active_index = 0; let sifter: utility::ResultSifter> = fields .iter() .enumerate() - .map(|(index, field)| -> Result { - let attrs = parse_field_attrs(&field.attrs)?; - Ok(StructField { - index, - attrs, - data: field, - #[cfg(feature = "documentation")] - doc: crate::documentation::Documentation::from_attributes(&field.attrs), - }) - }) + .map( + |(declaration_index, field)| -> Result { + let attrs = parse_field_attrs(&field.attrs)?; + + let reflection_index = if attrs.ignore.is_ignored() { + None + } else { + active_index += 1; + Some(active_index - 1) + }; + + Ok(StructField { + declaration_index, + reflection_index, + attrs, + data: field, + #[cfg(feature = "documentation")] + doc: crate::documentation::Documentation::from_attributes(&field.attrs), + }) + }, + ) .fold( utility::ResultSifter::default(), utility::ResultSifter::fold, @@ -420,12 +437,9 @@ impl<'a> ReflectStruct<'a> { &self.meta } - /// Access the data about which fields should be ignored during serialization. - /// - /// The returned bitset is a collection of indices obtained from the [`members_to_serialization_denylist`] function. - #[allow(dead_code)] - pub fn serialization_denylist(&self) -> &BitSet { - &self.serialization_denylist + /// Returns the [`SerializationDataDef`] for this struct. + pub fn serialization_data(&self) -> Option<&SerializationDataDef> { + self.serialization_data.as_ref() } /// Returns the `GetTypeRegistration` impl as a `TokenStream`. @@ -438,7 +452,7 @@ impl<'a> ReflectStruct<'a> { crate::registration::impl_get_type_registration( self.meta(), where_clause_options, - Some(&self.serialization_denylist), + self.serialization_data(), ) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs index 69525bd759210..bca7162de8b2d 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs @@ -189,7 +189,7 @@ fn get_ignored_fields(reflect_struct: &ReflectStruct) -> MemberValuePair { reflect_struct .ignored_fields() .map(|field| { - let member = ident_or_index(field.data.ident.as_ref(), field.index); + let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); let value = match &field.attrs.default { DefaultBehavior::Func(path) => quote! {#path()}, @@ -218,8 +218,12 @@ fn get_active_fields( reflect_struct .active_fields() .map(|field| { - let member = ident_or_index(field.data.ident.as_ref(), field.index); - let accessor = get_field_accessor(field.data, field.index, is_tuple); + let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let accessor = get_field_accessor( + field.data, + field.reflection_index.expect("field should be active"), + is_tuple, + ); let ty = field.data.ty.clone(); let get_field = quote! { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs index 8eec84fcac678..a733ec2e262bf 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -346,7 +346,11 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden // Ignored field continue; } - constructor_argument.push(generate_for_field(reflect_idx, field.index, field)); + constructor_argument.push(generate_for_field( + reflect_idx, + field.declaration_index, + field, + )); reflect_idx += 1; } constructor_argument diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs index 60a5c14cbc369..1bf46968cebdc 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs @@ -19,12 +19,12 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS .ident .as_ref() .map(|i| i.to_string()) - .unwrap_or_else(|| field.index.to_string()) + .unwrap_or_else(|| field.declaration_index.to_string()) }) .collect::>(); let field_idents = reflect_struct .active_fields() - .map(|field| ident_or_index(field.data.ident.as_ref(), field.index)) + .map(|field| ident_or_index(field.data.ident.as_ref(), field.declaration_index)) .collect::>(); let field_types = reflect_struct.active_types(); let field_count = field_idents.len(); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs index ed507f3714d10..e05226d7a52b6 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs @@ -14,7 +14,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: let field_idents = reflect_struct .active_fields() - .map(|field| Member::Unnamed(Index::from(field.index))) + .map(|field| Member::Unnamed(Index::from(field.declaration_index))) .collect::>(); let field_types = reflect_struct.active_types(); let field_count = field_idents.len(); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index 5474f143cda99..e87d3ccf5c8d3 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -24,6 +24,7 @@ mod from_reflect; mod impls; mod reflect_value; mod registration; +mod serialization; mod trait_reflection; mod type_path; mod type_uuid; @@ -201,8 +202,10 @@ pub fn derive_reflect(input: TokenStream) -> TokenStream { }; TokenStream::from(quote! { - #reflect_impls - #from_reflect_impl + const _: () = { + #reflect_impls + #from_reflect_impl + }; }) } @@ -241,15 +244,20 @@ pub fn derive_from_reflect(input: TokenStream) -> TokenStream { Err(err) => return err.into_compile_error().into(), }; - match derive_data { + let from_reflect_impl = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => { from_reflect::impl_struct(&struct_data) } ReflectDerive::TupleStruct(struct_data) => from_reflect::impl_tuple_struct(&struct_data), ReflectDerive::Enum(meta) => from_reflect::impl_enum(&meta), ReflectDerive::Value(meta) => from_reflect::impl_value(&meta), - } - .into() + }; + + TokenStream::from(quote! { + const _: () = { + #from_reflect_impl + }; + }) } /// Derives the `TypePath` trait, providing a stable alternative to [`std::any::type_name`]. @@ -275,21 +283,31 @@ pub fn derive_type_path(input: TokenStream) -> TokenStream { Err(err) => return err.into_compile_error().into(), }; - impls::impl_type_path( + let type_path_impl = impls::impl_type_path( derive_data.meta(), // Use `WhereClauseOptions::new_value` here so we don't enforce reflection bounds &WhereClauseOptions::new_value(derive_data.meta()), - ) - .into() + ); + + TokenStream::from(quote! { + const _: () = { + #type_path_impl + }; + }) } // From https://github.com/randomPoison/type-uuid #[proc_macro_derive(TypeUuid, attributes(uuid))] pub fn derive_type_uuid(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); - type_uuid::type_uuid_derive(input) - .unwrap_or_else(syn::Error::into_compile_error) - .into() + let uuid_impl = + type_uuid::type_uuid_derive(input).unwrap_or_else(syn::Error::into_compile_error); + + TokenStream::from(quote! { + const _: () = { + #uuid_impl + }; + }) } /// A macro that automatically generates type data for traits, which their implementors can then register. @@ -401,8 +419,10 @@ pub fn impl_reflect_value(input: TokenStream) -> TokenStream { let from_reflect_impl = from_reflect::impl_value(&meta); TokenStream::from(quote! { - #reflect_impls - #from_reflect_impl + const _: () = { + #reflect_impls + #from_reflect_impl + }; }) } @@ -446,7 +466,7 @@ pub fn impl_reflect_struct(input: TokenStream) -> TokenStream { Err(err) => return err.into_compile_error().into(), }; - match derive_data { + let output = match derive_data { ReflectDerive::Struct(struct_data) => { if !struct_data.meta().type_path().has_custom_path() { return syn::Error::new( @@ -460,27 +480,30 @@ pub fn impl_reflect_struct(input: TokenStream) -> TokenStream { let impl_struct = impls::impl_struct(&struct_data); let impl_from_struct = from_reflect::impl_struct(&struct_data); - TokenStream::from(quote! { + quote! { #impl_struct #impl_from_struct - }) + } } ReflectDerive::TupleStruct(..) => syn::Error::new( ast.span(), "impl_reflect_struct does not support tuple structs", ) - .into_compile_error() - .into(), + .into_compile_error(), ReflectDerive::UnitStruct(..) => syn::Error::new( ast.span(), "impl_reflect_struct does not support unit structs", ) - .into_compile_error() - .into(), + .into_compile_error(), _ => syn::Error::new(ast.span(), "impl_reflect_struct only supports structs") - .into_compile_error() - .into(), - } + .into_compile_error(), + }; + + TokenStream::from(quote! { + const _: () = { + #output + }; + }) } /// A macro used to generate a `FromReflect` trait implementation for the given type. @@ -521,7 +544,14 @@ pub fn impl_from_reflect_value(input: TokenStream) -> TokenStream { } }; - from_reflect::impl_value(&ReflectMeta::new(type_path, def.traits.unwrap_or_default())).into() + let from_reflect_impl = + from_reflect::impl_value(&ReflectMeta::new(type_path, def.traits.unwrap_or_default())); + + TokenStream::from(quote! { + const _: () = { + #from_reflect_impl + }; + }) } /// A replacement for [deriving `TypePath`] for use on foreign types. @@ -583,12 +613,24 @@ pub fn impl_type_path(input: TokenStream) -> TokenStream { let meta = ReflectMeta::new(type_path, ReflectTraits::default()); - impls::impl_type_path(&meta, &WhereClauseOptions::new_value(&meta)).into() + let type_path_impl = impls::impl_type_path(&meta, &WhereClauseOptions::new_value(&meta)); + + TokenStream::from(quote! { + const _: () = { + #type_path_impl + }; + }) } /// Derives `TypeUuid` for the given type. This is used internally to implement `TypeUuid` on foreign types, such as those in the std. This macro should be used in the format of `<[Generic Params]> [Type (Path)], [Uuid (String Literal)]`. #[proc_macro] pub fn impl_type_uuid(input: TokenStream) -> TokenStream { let def = parse_macro_input!(input as type_uuid::TypeUuidDef); - type_uuid::gen_impl_type_uuid(def).into() + let uuid_impl = type_uuid::gen_impl_type_uuid(def); + + TokenStream::from(quote! { + const _: () = { + #uuid_impl + }; + }) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 0b0a31e0a38fd..115274ad46ae1 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -1,8 +1,8 @@ //! Contains code related specifically to Bevy's type registration. use crate::derive_data::ReflectMeta; +use crate::serialization::SerializationDataDef; use crate::utility::{extend_where_clause, WhereClauseOptions}; -use bit_set::BitSet; use quote::quote; /// Creates the `GetTypeRegistration` impl for the given type data. @@ -10,7 +10,7 @@ use quote::quote; pub(crate) fn impl_get_type_registration( meta: &ReflectMeta, where_clause_options: &WhereClauseOptions, - serialization_denylist: Option<&BitSet>, + serialization_data: Option<&SerializationDataDef>, ) -> proc_macro2::TokenStream { let type_path = meta.type_path(); let bevy_reflect_path = meta.bevy_reflect_path(); @@ -20,17 +20,16 @@ pub(crate) fn impl_get_type_registration( let from_reflect_data = if meta.from_reflect().should_auto_derive() { Some(quote! { - registration.insert::<#bevy_reflect_path::ReflectFromReflect>(#bevy_reflect_path::FromType::::from_type()); + registration.insert::<#bevy_reflect_path::ReflectFromReflect>(#bevy_reflect_path::FromType::::from_type()); }) } else { None }; - let serialization_data = serialization_denylist.map(|denylist| { - let denylist = denylist.into_iter(); + let serialization_data = serialization_data.map(|data| { + let serialization_data = data.as_serialization_data(bevy_reflect_path); quote! { - let ignored_indices = ::core::iter::IntoIterator::into_iter([#(#denylist),*]); - registration.insert::<#bevy_reflect_path::serde::SerializationData>(#bevy_reflect_path::serde::SerializationData::new(ignored_indices)); + registration.insert::<#bevy_reflect_path::serde::SerializationData>(#serialization_data); } }); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs b/crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs new file mode 100644 index 0000000000000..0242947b5c91f --- /dev/null +++ b/crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs @@ -0,0 +1,91 @@ +use crate::derive_data::StructField; +use crate::field_attributes::{DefaultBehavior, ReflectIgnoreBehavior}; +use bevy_macro_utils::fq_std::{FQBox, FQDefault}; +use quote::quote; +use std::collections::HashMap; +use syn::spanned::Spanned; +use syn::Path; + +type ReflectionIndex = usize; + +/// Collected serialization data used to generate a `SerializationData` type. +pub(crate) struct SerializationDataDef { + /// Maps a field's _reflection_ index to its [`SkippedFieldDef`] if marked as `#[reflect(skip_serializing)]`. + skipped: HashMap, +} + +impl SerializationDataDef { + /// Attempts to create a new `SerializationDataDef` from the given collection of fields. + /// + /// Returns `Ok(Some(data))` if there are any fields needing to be skipped during serialization. + /// Otherwise, returns `Ok(None)`. + pub fn new(fields: &[StructField<'_>]) -> Result, syn::Error> { + let mut skipped = HashMap::default(); + + for field in fields { + match field.attrs.ignore { + ReflectIgnoreBehavior::IgnoreSerialization => { + skipped.insert( + field.reflection_index.ok_or_else(|| { + syn::Error::new( + field.data.span(), + "internal error: field is missing a reflection index", + ) + })?, + SkippedFieldDef::new(field)?, + ); + } + _ => continue, + } + } + + if skipped.is_empty() { + Ok(None) + } else { + Ok(Some(Self { skipped })) + } + } + + /// Returns a `TokenStream` containing an initialized `SerializationData` type. + pub fn as_serialization_data(&self, bevy_reflect_path: &Path) -> proc_macro2::TokenStream { + let fields = + self.skipped + .iter() + .map(|(reflection_index, SkippedFieldDef { default_fn })| { + quote! {( + #reflection_index, + #bevy_reflect_path::serde::SkippedField::new(#default_fn) + )} + }); + quote! { + #bevy_reflect_path::serde::SerializationData::new( + ::core::iter::IntoIterator::into_iter([#(#fields),*]) + ) + } + } +} + +/// Collected field data used to generate a `SkippedField` type. +pub(crate) struct SkippedFieldDef { + /// The default function for this field. + /// + /// This is of type `fn() -> Box`. + default_fn: proc_macro2::TokenStream, +} + +impl SkippedFieldDef { + pub fn new(field: &StructField<'_>) -> Result { + let ty = &field.data.ty; + + let default_fn = match &field.attrs.default { + DefaultBehavior::Func(func) => quote! { + || { #FQBox::new(#func()) } + }, + _ => quote! { + || { #FQBox::new(<#ty as #FQDefault>::default()) } + }, + }; + + Ok(Self { default_fn }) + } +} diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 9d25e35a37533..0cd4c88b4cae9 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -1,12 +1,10 @@ //! General-purpose utility functions for internal usage within this crate. use crate::derive_data::{ReflectMeta, StructField}; -use crate::field_attributes::ReflectIgnoreBehavior; use bevy_macro_utils::{ fq_std::{FQAny, FQOption, FQSend, FQSync}, BevyManifest, }; -use bit_set::BitSet; use proc_macro2::{Ident, Span}; use quote::{quote, ToTokens}; use syn::{spanned::Spanned, LitStr, Member, Path, Type, WhereClause}; @@ -286,42 +284,6 @@ impl ResultSifter { } } -/// Converts an iterator over ignore behavior of members to a bitset of ignored members. -/// -/// Takes into account the fact that always ignored (non-reflected) members are skipped. -/// -/// # Example -/// ```rust,ignore -/// pub struct HelloWorld { -/// reflected_field: u32 // index: 0 -/// -/// #[reflect(ignore)] -/// non_reflected_field: u32 // index: N/A (not 1!) -/// -/// #[reflect(skip_serializing)] -/// non_serialized_field: u32 // index: 1 -/// } -/// ``` -/// Would convert to the `0b01` bitset (i.e second field is NOT serialized) -/// -pub(crate) fn members_to_serialization_denylist(member_iter: T) -> BitSet -where - T: Iterator, -{ - let mut bitset = BitSet::default(); - - member_iter.fold(0, |next_idx, member| match member { - ReflectIgnoreBehavior::IgnoreAlways => next_idx, - ReflectIgnoreBehavior::IgnoreSerialization => { - bitset.insert(next_idx); - next_idx + 1 - } - ReflectIgnoreBehavior::None => next_idx + 1, - }); - - bitset -} - /// Turns an `Option` into a `TokenStream` for an `Option`. pub(crate) fn wrap_in_option(tokens: Option) -> proc_macro2::TokenStream { match tokens { diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index b01adece410bb..1a02cf4ed838d 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -764,6 +764,39 @@ mod tests { .unwrap_or_default()); } + #[test] + fn from_reflect_should_allow_ignored_unnamed_fields() { + #[derive(Reflect, Eq, PartialEq, Debug)] + struct MyTupleStruct(i8, #[reflect(ignore)] i16, i32); + + let expected = MyTupleStruct(1, 0, 3); + + let mut dyn_tuple_struct = DynamicTupleStruct::default(); + dyn_tuple_struct.insert(1_i8); + dyn_tuple_struct.insert(3_i32); + let my_tuple_struct = ::from_reflect(&dyn_tuple_struct); + + assert_eq!(Some(expected), my_tuple_struct); + + #[derive(Reflect, Eq, PartialEq, Debug)] + enum MyEnum { + Tuple(i8, #[reflect(ignore)] i16, i32), + } + + let expected = MyEnum::Tuple(1, 0, 3); + + let mut dyn_tuple = DynamicTuple::default(); + dyn_tuple.insert(1_i8); + dyn_tuple.insert(3_i32); + + let mut dyn_enum = DynamicEnum::default(); + dyn_enum.set_variant("Tuple", dyn_tuple); + + let my_enum = ::from_reflect(&dyn_enum); + + assert_eq!(Some(expected), my_enum); + } + #[test] fn from_reflect_should_use_default_field_attributes() { #[derive(Reflect, Eq, PartialEq, Debug)] diff --git a/crates/bevy_reflect/src/serde/de.rs b/crates/bevy_reflect/src/serde/de.rs index 38f1795186d9a..170c6c941cf1f 100644 --- a/crates/bevy_reflect/src/serde/de.rs +++ b/crates/bevy_reflect/src/serde/de.rs @@ -2,9 +2,8 @@ use crate::serde::SerializationData; use crate::{ ArrayInfo, DynamicArray, DynamicEnum, DynamicList, DynamicMap, DynamicStruct, DynamicTuple, DynamicTupleStruct, DynamicVariant, EnumInfo, ListInfo, Map, MapInfo, NamedField, Reflect, - ReflectDeserialize, StructInfo, StructVariantInfo, Tuple, TupleInfo, TupleStruct, - TupleStructInfo, TupleVariantInfo, TypeInfo, TypeRegistration, TypeRegistry, UnnamedField, - VariantInfo, + ReflectDeserialize, StructInfo, StructVariantInfo, TupleInfo, TupleStructInfo, + TupleVariantInfo, TypeInfo, TypeRegistration, TypeRegistry, UnnamedField, VariantInfo, }; use erased_serde::Deserializer; use serde::de::{ @@ -27,6 +26,8 @@ pub trait DeserializeValue { trait StructLikeInfo { fn get_path(&self) -> &str; fn get_field(&self, name: &str) -> Option<&NamedField>; + fn field_at(&self, index: usize) -> Option<&NamedField>; + fn get_field_len(&self) -> usize; fn iter_fields(&self) -> Iter<'_, NamedField>; } @@ -49,10 +50,18 @@ impl StructLikeInfo for StructInfo { self.type_path() } + fn field_at(&self, index: usize) -> Option<&NamedField> { + self.field_at(index) + } + fn get_field(&self, name: &str) -> Option<&NamedField> { self.field(name) } + fn get_field_len(&self) -> usize { + self.field_len() + } + fn iter_fields(&self) -> Iter<'_, NamedField> { self.iter() } @@ -80,10 +89,18 @@ impl StructLikeInfo for StructVariantInfo { self.name() } + fn field_at(&self, index: usize) -> Option<&NamedField> { + self.field_at(index) + } + fn get_field(&self, name: &str) -> Option<&NamedField> { self.field(name) } + fn get_field_len(&self) -> usize { + self.field_len() + } + fn iter_fields(&self) -> Iter<'_, NamedField> { self.iter() } @@ -120,6 +137,54 @@ impl TupleLikeInfo for TupleInfo { } } +impl Container for TupleInfo { + fn get_field_registration<'a, E: Error>( + &self, + index: usize, + registry: &'a TypeRegistry, + ) -> Result<&'a TypeRegistration, E> { + let field = self.field_at(index).ok_or_else(|| { + de::Error::custom(format_args!( + "no field at index {} on tuple {}", + index, + self.type_path(), + )) + })?; + get_registration(field.type_id(), field.type_path(), registry) + } +} + +impl TupleLikeInfo for TupleStructInfo { + fn get_path(&self) -> &str { + self.type_path() + } + + fn get_field(&self, index: usize) -> Option<&UnnamedField> { + self.field_at(index) + } + + fn get_field_len(&self) -> usize { + self.field_len() + } +} + +impl Container for TupleStructInfo { + fn get_field_registration<'a, E: Error>( + &self, + index: usize, + registry: &'a TypeRegistry, + ) -> Result<&'a TypeRegistration, E> { + let field = self.field_at(index).ok_or_else(|| { + de::Error::custom(format_args!( + "no field at index {} on tuple struct {}", + index, + self.type_path(), + )) + })?; + get_registration(field.type_id(), field.type_path(), registry) + } +} + impl TupleLikeInfo for TupleVariantInfo { fn get_path(&self) -> &str { self.name() @@ -134,6 +199,23 @@ impl TupleLikeInfo for TupleVariantInfo { } } +impl Container for TupleVariantInfo { + fn get_field_registration<'a, E: Error>( + &self, + index: usize, + registry: &'a TypeRegistry, + ) -> Result<&'a TypeRegistration, E> { + let field = self.field_at(index).ok_or_else(|| { + de::Error::custom(format_args!( + "no field at index {} on tuple variant {}", + index, + self.name(), + )) + })?; + get_registration(field.type_id(), field.type_path(), registry) + } +} + /// A debug struct used for error messages that displays a list of expected values. /// /// # Example @@ -444,6 +526,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { tuple_info.field_len(), TupleVisitor { tuple_info, + registration: self.registration, registry: self.registry, }, )?; @@ -500,43 +583,14 @@ impl<'a, 'de> Visitor<'de> for StructVisitor<'a> { where V: MapAccess<'de>, { - visit_struct(&mut map, self.struct_info, self.registry) + visit_struct(&mut map, self.struct_info, self.registration, self.registry) } fn visit_seq(self, mut seq: A) -> Result where A: SeqAccess<'de>, { - let mut index = 0usize; - let mut output = DynamicStruct::default(); - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self.struct_info.field_len().saturating_sub(ignored_len); - - if field_len == 0 { - // Handle unit structs and ignored fields - return Ok(output); - } - - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: self - .struct_info - .get_field_registration(index, self.registry)?, - registry: self.registry, - })? { - let name = self.struct_info.field_at(index).unwrap().name(); - output.insert_boxed(name, value); - index += 1; - if index >= self.struct_info.field_len() { - break; - } - } - - Ok(output) + visit_struct_seq(&mut seq, self.struct_info, self.registration, self.registry) } } @@ -557,64 +611,19 @@ impl<'a, 'de> Visitor<'de> for TupleStructVisitor<'a> { where V: SeqAccess<'de>, { - let mut index = 0usize; - let mut tuple_struct = DynamicTupleStruct::default(); - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self - .tuple_struct_info - .field_len() - .saturating_sub(ignored_len); - - if field_len == 0 { - // Handle unit structs and ignored fields - return Ok(tuple_struct); - } - - let get_field_registration = |index: usize| -> Result<&'a TypeRegistration, V::Error> { - let field = self.tuple_struct_info.field_at(index).ok_or_else(|| { - de::Error::custom(format_args!( - "no field at index {} on tuple {}", - index, - self.tuple_struct_info.type_path(), - )) - })?; - get_registration(field.type_id(), field.type_path(), self.registry) - }; - - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: get_field_registration(index)?, - registry: self.registry, - })? { - tuple_struct.insert_boxed(value); - index += 1; - if index >= self.tuple_struct_info.field_len() { - break; - } - } - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - if tuple_struct.field_len() != self.tuple_struct_info.field_len() - ignored_len { - return Err(Error::invalid_length( - tuple_struct.field_len(), - &self.tuple_struct_info.field_len().to_string().as_str(), - )); - } - - Ok(tuple_struct) + visit_tuple( + &mut seq, + self.tuple_struct_info, + self.registration, + self.registry, + ) + .map(DynamicTupleStruct::from) } } struct TupleVisitor<'a> { tuple_info: &'static TupleInfo, + registration: &'a TypeRegistration, registry: &'a TypeRegistry, } @@ -629,7 +638,7 @@ impl<'a, 'de> Visitor<'de> for TupleVisitor<'a> { where V: SeqAccess<'de>, { - visit_tuple(&mut seq, self.tuple_info, self.registry) + visit_tuple(&mut seq, self.tuple_info, self.registration, self.registry) } } @@ -782,9 +791,7 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> { )? .into(), VariantInfo::Tuple(tuple_info) if tuple_info.field_len() == 1 => { - let field = tuple_info.field_at(0).unwrap(); - let registration = - get_registration(field.type_id(), field.type_path(), self.registry)?; + let registration = tuple_info.get_field_registration(0, self.registry)?; let value = variant.newtype_variant_seed(TypedReflectDeserializer { registration, registry: self.registry, @@ -879,43 +886,14 @@ impl<'a, 'de> Visitor<'de> for StructVariantVisitor<'a> { where V: MapAccess<'de>, { - visit_struct(&mut map, self.struct_info, self.registry) + visit_struct(&mut map, self.struct_info, self.registration, self.registry) } fn visit_seq(self, mut seq: A) -> Result where A: SeqAccess<'de>, { - let mut index = 0usize; - let mut output = DynamicStruct::default(); - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self.struct_info.field_len().saturating_sub(ignored_len); - - if field_len == 0 { - // Handle all fields being ignored - return Ok(output); - } - - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: self - .struct_info - .get_field_registration(index, self.registry)?, - registry: self.registry, - })? { - let name = self.struct_info.field_at(index).unwrap().name(); - output.insert_boxed(name, value); - index += 1; - if index >= self.struct_info.field_len() { - break; - } - } - - Ok(output) + visit_struct_seq(&mut seq, self.struct_info, self.registration, self.registry) } } @@ -936,19 +914,7 @@ impl<'a, 'de> Visitor<'de> for TupleVariantVisitor<'a> { where V: SeqAccess<'de>, { - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self.tuple_info.field_len().saturating_sub(ignored_len); - - if field_len == 0 { - // Handle all fields being ignored - return Ok(DynamicTuple::default()); - } - - visit_tuple(&mut seq, self.tuple_info, self.registry) + visit_tuple(&mut seq, self.tuple_info, self.registration, self.registry) } } @@ -1005,6 +971,7 @@ impl<'a, 'de> Visitor<'de> for OptionVisitor<'a> { fn visit_struct<'de, T, V>( map: &mut V, info: &'static T, + registration: &TypeRegistration, registry: &TypeRegistry, ) -> Result where @@ -1029,49 +996,101 @@ where dynamic_struct.insert_boxed(&key, value); } + if let Some(serialization_data) = registration.data::() { + for (skipped_index, skipped_field) in serialization_data.iter_skipped() { + let Some(field) = info.field_at(*skipped_index) else { + continue; + }; + dynamic_struct.insert_boxed(field.name(), skipped_field.generate_default()); + } + } + Ok(dynamic_struct) } fn visit_tuple<'de, T, V>( seq: &mut V, info: &T, + registration: &TypeRegistration, registry: &TypeRegistry, ) -> Result where - T: TupleLikeInfo, + T: TupleLikeInfo + Container, V: SeqAccess<'de>, { let mut tuple = DynamicTuple::default(); - let mut index = 0usize; - let get_field_registration = |index: usize| -> Result<&TypeRegistration, V::Error> { - let field = info.get_field(index).ok_or_else(|| { - Error::invalid_length(index, &info.get_field_len().to_string().as_str()) - })?; - get_registration(field.type_id(), field.type_path(), registry) - }; + let len = info.get_field_len(); - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: get_field_registration(index)?, - registry, - })? { - tuple.insert_boxed(value); - index += 1; - if index >= info.get_field_len() { - break; + if len == 0 { + // Handle empty tuple/tuple struct + return Ok(tuple); + } + + let serialization_data = registration.data::(); + + for index in 0..len { + if let Some(value) = serialization_data.and_then(|data| data.generate_default(index)) { + tuple.insert_boxed(value); + continue; } + + let value = seq + .next_element_seed(TypedReflectDeserializer { + registration: info.get_field_registration(index, registry)?, + registry, + })? + .ok_or_else(|| Error::invalid_length(index, &len.to_string().as_str()))?; + tuple.insert_boxed(value); } + Ok(tuple) +} + +fn visit_struct_seq<'de, T, V>( + seq: &mut V, + info: &T, + registration: &TypeRegistration, + registry: &TypeRegistry, +) -> Result +where + T: StructLikeInfo + Container, + V: SeqAccess<'de>, +{ + let mut dynamic_struct = DynamicStruct::default(); + let len = info.get_field_len(); - if tuple.field_len() != len { - return Err(Error::invalid_length( - tuple.field_len(), - &len.to_string().as_str(), - )); + if len == 0 { + // Handle unit structs + return Ok(dynamic_struct); } - Ok(tuple) + let serialization_data = registration.data::(); + + for index in 0..len { + let name = info.field_at(index).unwrap().name(); + + if serialization_data + .map(|data| data.is_field_skipped(index)) + .unwrap_or_default() + { + if let Some(value) = serialization_data.unwrap().generate_default(index) { + dynamic_struct.insert_boxed(name, value); + } + continue; + } + + let value = seq + .next_element_seed(TypedReflectDeserializer { + registration: info.get_field_registration(index, registry)?, + registry, + })? + .ok_or_else(|| Error::invalid_length(index, &len.to_string().as_str()))?; + dynamic_struct.insert_boxed(name, value); + } + + Ok(dynamic_struct) } fn get_registration<'a, E: Error>( diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index 5f87eba8f304b..c444279fa928a 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -12,7 +12,7 @@ mod tests { use crate::{ serde::{ReflectSerializer, UntypedReflectDeserializer}, type_registry::TypeRegistry, - DynamicStruct, Reflect, + DynamicStruct, FromReflect, Reflect, }; use serde::de::DeserializeSeed; @@ -26,7 +26,14 @@ mod tests { b: i32, #[reflect(skip_serializing)] c: i32, + #[reflect(skip_serializing)] + #[reflect(default = "custom_default")] d: i32, + e: i32, + } + + fn custom_default() -> i32 { + -1 } let mut registry = TypeRegistry::default(); @@ -37,24 +44,42 @@ mod tests { b: 4, c: 5, d: 6, + e: 7, }; let serializer = ReflectSerializer::new(&test_struct, ®istry); let serialized = ron::ser::to_string_pretty(&serializer, ron::ser::PrettyConfig::default()).unwrap(); - let mut expected = DynamicStruct::default(); - expected.insert("a", 3); - expected.insert("d", 6); - let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); let reflect_deserializer = UntypedReflectDeserializer::new(®istry); let value = reflect_deserializer.deserialize(&mut deserializer).unwrap(); let deserialized = value.take::().unwrap(); + let mut expected = DynamicStruct::default(); + expected.insert("a", 3); + // Ignored: expected.insert("b", 0); + expected.insert("c", 0); + expected.insert("d", -1); + expected.insert("e", 7); + assert!( expected.reflect_partial_eq(&deserialized).unwrap(), - "Expected {expected:?} found {deserialized:?}" + "Deserialization failed: expected {expected:?} found {deserialized:?}" + ); + + let expected = TestStruct { + a: 3, + b: 0, + c: 0, + d: -1, + e: 7, + }; + let received = ::from_reflect(&deserialized).unwrap(); + + assert_eq!( + expected, received, + "FromReflect failed: expected {expected:?} found {received:?}" ); } @@ -66,30 +91,48 @@ mod tests { i32, #[reflect(ignore)] i32, #[reflect(skip_serializing)] i32, + #[reflect(skip_serializing)] + #[reflect(default = "custom_default")] + i32, i32, ); + fn custom_default() -> i32 { + -1 + } + let mut registry = TypeRegistry::default(); registry.register::(); - let test_struct = TestStruct(3, 4, 5, 6); + let test_struct = TestStruct(3, 4, 5, 6, 7); let serializer = ReflectSerializer::new(&test_struct, ®istry); let serialized = ron::ser::to_string_pretty(&serializer, ron::ser::PrettyConfig::default()).unwrap(); - let mut expected = DynamicTupleStruct::default(); - expected.insert(3); - expected.insert(6); - let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); let reflect_deserializer = UntypedReflectDeserializer::new(®istry); let value = reflect_deserializer.deserialize(&mut deserializer).unwrap(); let deserialized = value.take::().unwrap(); + let mut expected = DynamicTupleStruct::default(); + expected.insert(3); + // Ignored: expected.insert(0); + expected.insert(0); + expected.insert(-1); + expected.insert(7); + assert!( expected.reflect_partial_eq(&deserialized).unwrap(), - "Expected {expected:?} found {deserialized:?}" + "Deserialization failed: expected {expected:?} found {deserialized:?}" + ); + + let expected = TestStruct(3, 0, 0, -1, 7); + let received = ::from_reflect(&deserialized).unwrap(); + + assert_eq!( + expected, received, + "FromReflect failed: expected {expected:?} found {received:?}" ); } diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 79ec73099a457..fc072a8d2de18 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -212,7 +212,7 @@ impl<'a> Serialize for StructSerializer<'a> { for (index, value) in self.struct_value.iter_fields().enumerate() { if serialization_data - .map(|data| data.is_ignored_field(index)) + .map(|data| data.is_field_skipped(index)) .unwrap_or(false) { continue; @@ -265,7 +265,7 @@ impl<'a> Serialize for TupleStructSerializer<'a> { for (index, value) in self.tuple_struct.iter_fields().enumerate() { if serialization_data - .map(|data| data.is_ignored_field(index)) + .map(|data| data.is_field_skipped(index)) .unwrap_or(false) { continue; diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index ee69a390d09cb..d82f3b4579095 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -1,44 +1,136 @@ -use std::collections::HashSet; +use crate::Reflect; +use bevy_utils::hashbrown::hash_map::Iter; +use bevy_utils::HashMap; -/// Contains data relevant to the automatic reflect powered serialization of a type +/// Contains data relevant to the automatic reflect powered (de)serialization of a type. #[derive(Debug, Clone)] pub struct SerializationData { - ignored_field_indices: HashSet, + skipped_fields: HashMap, } impl SerializationData { - /// Creates a new `SerializationData` instance given: + /// Creates a new `SerializationData` instance with the given skipped fields. /// - /// - `ignored_iter`: the iterator of member indices to be ignored during serialization. Indices are assigned only to reflected members, those which are not reflected are skipped. - pub fn new>(ignored_iter: I) -> Self { + /// # Arguments + /// + /// * `skipped_iter`: The iterator of field indices to be skipped during (de)serialization. + /// Indices are assigned only to reflected fields. + /// Ignored fields (i.e. those marked `#[reflect(ignore)]`) are implicitly skipped + /// and do not need to be included in this iterator. + pub fn new>(skipped_iter: I) -> Self { Self { - ignored_field_indices: ignored_iter.collect(), + skipped_fields: skipped_iter.collect(), } } - /// Returns true if the given index corresponds to a field meant to be ignored in serialization. - /// - /// Indices start from 0 and ignored fields are skipped. + /// Returns true if the given index corresponds to a field meant to be skipped during (de)serialization. /// /// # Example /// - /// ```rust,ignore + /// ``` + /// # use std::any::TypeId; + /// # use bevy_reflect::{Reflect, Struct, TypeRegistry, serde::SerializationData}; + /// #[derive(Reflect)] + /// struct MyStruct { + /// serialize_me: i32, + /// #[reflect(skip_serializing)] + /// skip_me: i32 + /// } + /// + /// let mut registry = TypeRegistry::new(); + /// registry.register::(); + /// + /// let my_struct = MyStruct { + /// serialize_me: 123, + /// skip_me: 321, + /// }; + /// + /// let serialization_data = registry.get_type_data::(TypeId::of::()).unwrap(); + /// /// for (idx, field) in my_struct.iter_fields().enumerate(){ - /// if serialization_data.is_ignored_field(idx){ - /// // serialize ... - /// } + /// if serialization_data.is_field_skipped(idx) { + /// // Skipped! + /// assert_eq!(1, idx); + /// } else { + /// // Not Skipped! + /// assert_eq!(0, idx); + /// } + /// } + /// ``` + pub fn is_field_skipped(&self, index: usize) -> bool { + self.skipped_fields.contains_key(&index) + } + + /// Generates a default instance of the skipped field at the given index. + /// + /// Returns `None` if the field is not skipped. + /// + /// # Example + /// + /// ``` + /// # use std::any::TypeId; + /// # use bevy_reflect::{Reflect, Struct, TypeRegistry, serde::SerializationData}; + /// #[derive(Reflect)] + /// struct MyStruct { + /// serialize_me: i32, + /// #[reflect(skip_serializing)] + /// #[reflect(default = "skip_me_default")] + /// skip_me: i32 /// } + /// + /// fn skip_me_default() -> i32 { + /// 789 + /// } + /// + /// let mut registry = TypeRegistry::new(); + /// registry.register::(); + /// + /// let serialization_data = registry.get_type_data::(TypeId::of::()).unwrap(); + /// assert_eq!(789, serialization_data.generate_default(1).unwrap().take::().unwrap()); /// ``` - pub fn is_ignored_field(&self, index: usize) -> bool { - self.ignored_field_indices.contains(&index) + pub fn generate_default(&self, index: usize) -> Option> { + self.skipped_fields + .get(&index) + .map(|field| field.generate_default()) } - /// Returns the number of ignored fields. + /// Returns the number of skipped fields. pub fn len(&self) -> usize { - self.ignored_field_indices.len() + self.skipped_fields.len() } - /// Returns true if there are no ignored fields. + /// Returns true if there are no skipped fields. pub fn is_empty(&self) -> bool { - self.ignored_field_indices.is_empty() + self.skipped_fields.is_empty() + } + + /// Returns an iterator over the skipped fields. + /// + /// Each item in the iterator is a tuple containing: + /// 1. The reflected index of the field + /// 2. The (de)serialization metadata of the field + pub fn iter_skipped(&self) -> Iter<'_, usize, SkippedField> { + self.skipped_fields.iter() + } +} + +/// Data needed for (de)serialization of a skipped field. +#[derive(Debug, Clone)] +pub struct SkippedField { + default_fn: fn() -> Box, +} + +impl SkippedField { + /// Create a new `SkippedField`. + /// + /// # Arguments + /// + /// * `default_fn`: A function pointer used to generate a default instance of the field. + pub fn new(default_fn: fn() -> Box) -> Self { + Self { default_fn } + } + + /// Generates a default instance of the field. + pub fn generate_default(&self) -> Box { + (self.default_fn)() } } diff --git a/crates/bevy_reflect/src/tuple_struct.rs b/crates/bevy_reflect/src/tuple_struct.rs index 9d12490871980..ff9c53d5481aa 100644 --- a/crates/bevy_reflect/src/tuple_struct.rs +++ b/crates/bevy_reflect/src/tuple_struct.rs @@ -1,8 +1,8 @@ use bevy_reflect_derive::impl_type_path; use crate::{ - self as bevy_reflect, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, - TypePathTable, UnnamedField, + self as bevy_reflect, DynamicTuple, Reflect, ReflectMut, ReflectOwned, ReflectRef, Tuple, + TypeInfo, TypePath, TypePathTable, UnnamedField, }; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; @@ -390,6 +390,15 @@ impl Debug for DynamicTupleStruct { } } +impl From for DynamicTupleStruct { + fn from(value: DynamicTuple) -> Self { + Self { + represented_type: None, + fields: Box::new(value).drain(), + } + } +} + /// Compares a [`TupleStruct`] with a [`Reflect`] value. /// /// Returns true if and only if all of the following are true: From f9ef989def5a77d0e0a37c70a0a26939e0b9b3a3 Mon Sep 17 00:00:00 2001 From: anarelion Date: Sun, 22 Oct 2023 23:59:39 +0100 Subject: [PATCH 08/11] Implement source into Display for AssetPath (#10217) # Objective When debugging and printing asset paths, I am not 100% if I am in the right source or not ## Solution Add the output of the source --- crates/bevy_asset/src/path.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 43b7d2cab5c77..a6cf38db73848 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -67,6 +67,9 @@ impl<'a> Debug for AssetPath<'a> { impl<'a> Display for AssetPath<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let AssetSourceId::Name(name) = self.source() { + write!(f, "{name}://")?; + } write!(f, "{}", self.path.display())?; if let Some(label) = &self.label { write!(f, "#{label}")?; From 1e9258910cdec60e21db10e8efb7651222f76947 Mon Sep 17 00:00:00 2001 From: "Ame :]" <104745335+ameknite@users.noreply.github.com> Date: Sun, 22 Oct 2023 18:01:28 -0500 Subject: [PATCH 09/11] re-export `debug_glam_assert` feature (#10206) # Objective - I want to use the `debug_glam_assert` feature with bevy. ## Solution - Re-export the feature flag --- ## Changelog - Re-export `debug_glam_assert` feature flag from glam. --- Cargo.toml | 3 +++ crates/bevy_internal/Cargo.toml | 4 ++++ crates/bevy_math/Cargo.toml | 2 ++ docs/cargo_features.md | 1 + 4 files changed, 10 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 141cee12392a0..8dd252ffb8cd8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -232,6 +232,9 @@ accesskit_unix = ["bevy_internal/accesskit_unix"] # Enable assertions to check the validity of parameters passed to glam glam_assert = ["bevy_internal/glam_assert"] +# Enable assertions in debug builds to check the validity of parameters passed to glam +debug_glam_assert = ["bevy_internal/debug_glam_assert"] + # Include a default font, containing only ASCII characters, at the cost of a 20kB binary size increase default_font = ["bevy_internal/default_font"] diff --git a/crates/bevy_internal/Cargo.toml b/crates/bevy_internal/Cargo.toml index a7019d5461ea9..5dac70366d10c 100644 --- a/crates/bevy_internal/Cargo.toml +++ b/crates/bevy_internal/Cargo.toml @@ -97,9 +97,13 @@ accesskit_unix = ["bevy_winit/accesskit_unix"] bevy_text = ["dep:bevy_text", "bevy_ui?/bevy_text"] bevy_render = ["dep:bevy_render", "bevy_scene?/bevy_render"] + # Enable assertions to check the validity of parameters passed to glam glam_assert = ["bevy_math/glam_assert"] +# Enable assertions in debug builds to check the validity of parameters passed to glam +debug_glam_assert = ["bevy_math/debug_glam_assert"] + default_font = ["bevy_text?/default_font"] # Enables the built-in asset processor for processed assets. diff --git a/crates/bevy_math/Cargo.toml b/crates/bevy_math/Cargo.toml index f627ec9e6dfbc..df9389effee80 100644 --- a/crates/bevy_math/Cargo.toml +++ b/crates/bevy_math/Cargo.toml @@ -18,3 +18,5 @@ serialize = ["dep:serde", "glam/serde"] mint = ["glam/mint"] # Enable assertions to check the validity of parameters passed to glam glam_assert = ["glam/glam-assert"] +# Enable assertions in debug builds to check the validity of parameters passed to glam +debug_glam_assert = ["glam/debug-glam-assert"] diff --git a/docs/cargo_features.md b/docs/cargo_features.md index 4882b3801a060..47ac836eb5b7f 100644 --- a/docs/cargo_features.md +++ b/docs/cargo_features.md @@ -50,6 +50,7 @@ The default feature set enables most of the expected features of a game engine, |bevy_dynamic_plugin|Plugin for dynamic loading (using [libloading](https://crates.io/crates/libloading))| |bmp|BMP image format support| |dds|DDS compressed texture support| +|debug_glam_assert|Enable assertions in debug builds to check the validity of parameters passed to glam| |detailed_trace|Enable detailed trace event logging. These trace events are expensive even when off, thus they require compile time opt-in| |dynamic_linking|Force dynamic linking, which improves iterative compile times| |embedded_watcher|Enables watching in memory asset providers for Bevy Asset hot-reloading| From e59085a67f255647647820f1c235f97eba67a0c1 Mon Sep 17 00:00:00 2001 From: Marco Buono Date: Mon, 23 Oct 2023 00:26:20 -0300 Subject: [PATCH 10/11] =?UTF-8?q?Use=20=E2=80=9Cspecular=20occlusion?= =?UTF-8?q?=E2=80=9D=20term=20to=20consistently=20extinguish=20fresnel=20o?= =?UTF-8?q?n=20Ambient=20and=20Environment=20Map=20lights=20(#10182)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Even at `reflectance == 0.0`, our ambient and environment map light implementations still produce fresnel/specular highlights. Such a low `reflectance` value lies outside of the physically possible range and is already used by our directional, point and spot light implementations (via the `fresnel()` function) to enable artistic control, effectively disabling the fresnel "look" for non-physically realistic materials. Since ambient and environment lights use a different formulation, they were not honoring this same principle. This PR aims to bring consistency to all light types, offering the same fresnel extinguishing control to ambient and environment lights. Thanks to `@nathanf` for [pointing out](https://discord.com/channels/691052431525675048/743663924229963868/1164083373514440744) the [Filament docs section about this](https://google.github.io/filament/Filament.md.html#lighting/occlusion/specularocclusion). ## Solution - We use [the same formulation](https://github.com/bevyengine/bevy/blob/ffc572728fb7874996a13c31a82e86ef98515995/crates/bevy_pbr/src/render/pbr_lighting.wgsl#L99) already used by the `fresnel()` function in `bevy_pbr::lighting` to modulate the F90, to modulate the specular component of Ambient and Environment Map lights. ## Comparison ⚠️ **Modified version of the PBR example for demo purposes, that shows reflectance (_NOT_ part of this PR)** ⚠️ Also, keep in mind this is a very subtle difference (look for the fresnel highlights on the lower left spheres, you might need to zoom in. ### Before Screenshot 2023-10-18 at 23 02 25 ### After Screenshot 2023-10-18 at 23 01 43 --- ## Changelog - Ambient and Environment Map lights will now honor values of `reflectance` that are below the physically possible range (⪅ 0.35) by extinguishing their fresnel highlights. (Just like point, directional and spot lights already did.) This allows for more consistent artistic control and for non-physically realistic looks with all light types. ## Migration Guide - If Fresnel highlights from Ambient and Environment Map lights are no longer visible in your materials, make sure you're using a higher, physically plausible value of `reflectance` (⪆ 0.35). --- crates/bevy_pbr/src/environment_map/environment_map.wgsl | 7 ++++++- crates/bevy_pbr/src/render/pbr_ambient.wgsl | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/bevy_pbr/src/environment_map/environment_map.wgsl b/crates/bevy_pbr/src/environment_map/environment_map.wgsl index 2288d7d07c8bb..f188a578e478f 100644 --- a/crates/bevy_pbr/src/environment_map/environment_map.wgsl +++ b/crates/bevy_pbr/src/environment_map/environment_map.wgsl @@ -25,12 +25,17 @@ fn environment_map_light( let irradiance = textureSample(bindings::environment_map_diffuse, bindings::environment_map_sampler, vec3(N.xy, -N.z)).rgb; let radiance = textureSampleLevel(bindings::environment_map_specular, bindings::environment_map_sampler, vec3(R.xy, -R.z), radiance_level).rgb; + // No real world material has specular values under 0.02, so we use this range as a + // "pre-baked specular occlusion" that extinguishes the fresnel term, for artistic control. + // See: https://google.github.io/filament/Filament.html#specularocclusion + let specular_occlusion = saturate(dot(F0, vec3(50.0 * 0.33))); + // Multiscattering approximation: https://www.jcgt.org/published/0008/01/03/paper.pdf // Useful reference: https://bruop.github.io/ibl let Fr = max(vec3(1.0 - roughness), F0) - F0; let kS = F0 + Fr * pow(1.0 - NdotV, 5.0); - let FssEss = kS * f_ab.x + f_ab.y; let Ess = f_ab.x + f_ab.y; + let FssEss = kS * Ess * specular_occlusion; let Ems = 1.0 - Ess; let Favg = F0 + (1.0 - F0) / 21.0; let Fms = FssEss * Favg / (1.0 - Ems * Favg); diff --git a/crates/bevy_pbr/src/render/pbr_ambient.wgsl b/crates/bevy_pbr/src/render/pbr_ambient.wgsl index 23d5cf29b235a..7b174da35c9db 100644 --- a/crates/bevy_pbr/src/render/pbr_ambient.wgsl +++ b/crates/bevy_pbr/src/render/pbr_ambient.wgsl @@ -20,5 +20,10 @@ fn ambient_light( let diffuse_ambient = EnvBRDFApprox(diffuse_color, F_AB(1.0, NdotV)); let specular_ambient = EnvBRDFApprox(specular_color, F_AB(perceptual_roughness, NdotV)); - return (diffuse_ambient + specular_ambient) * lights.ambient_color.rgb * occlusion; + // No real world material has specular values under 0.02, so we use this range as a + // "pre-baked specular occlusion" that extinguishes the fresnel term, for artistic control. + // See: https://google.github.io/filament/Filament.html#specularocclusion + let specular_occlusion = saturate(dot(specular_color, vec3(50.0 * 0.33))); + + return (diffuse_ambient + specular_ambient * specular_occlusion) * lights.ambient_color.rgb * occlusion; } From d938275b9c2fda522c01fac84d118cb57f5474cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Mon, 23 Oct 2023 06:15:04 +0200 Subject: [PATCH 11/11] assets: use blake3 instead of md5 (#10208) # Objective - Replace md5 by another hasher, as suggested in https://github.com/bevyengine/bevy/pull/8624#discussion_r1359291028 - md5 is not secure, and is slow. use something more secure and faster ## Solution - Replace md5 by blake3 Putting this PR in the 0.12 as once it's released, changing the hash algorithm will be a painful breaking change --- crates/bevy_asset/Cargo.toml | 2 +- crates/bevy_asset/src/meta.rs | 20 +++++++++----------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/bevy_asset/Cargo.toml b/crates/bevy_asset/Cargo.toml index 922b622b89d8c..d67340effc47f 100644 --- a/crates/bevy_asset/Cargo.toml +++ b/crates/bevy_asset/Cargo.toml @@ -33,7 +33,7 @@ crossbeam-channel = "0.5" downcast-rs = "1.2" futures-io = "0.3" futures-lite = "1.12" -md5 = "0.7" +blake3 = "1.5" parking_lot = { version = "0.12", features = ["arc_lock", "send_guard"] } ron = "0.8" serde = { version = "1", features = ["derive"] } diff --git a/crates/bevy_asset/src/meta.rs b/crates/bevy_asset/src/meta.rs index e6d65b8ecd535..dbcd7d7feb57d 100644 --- a/crates/bevy_asset/src/meta.rs +++ b/crates/bevy_asset/src/meta.rs @@ -225,15 +225,14 @@ pub(crate) fn loader_settings_meta_transform( }) } -pub type AssetHash = [u8; 16]; +pub type AssetHash = [u8; 32]; /// NOTE: changing the hashing logic here is a _breaking change_ that requires a [`META_FORMAT_VERSION`] bump. pub(crate) fn get_asset_hash(meta_bytes: &[u8], asset_bytes: &[u8]) -> AssetHash { - let mut context = md5::Context::new(); - context.consume(meta_bytes); - context.consume(asset_bytes); - let digest = context.compute(); - digest.0 + let mut hasher = blake3::Hasher::new(); + hasher.update(meta_bytes); + hasher.update(asset_bytes); + *hasher.finalize().as_bytes() } /// NOTE: changing the hashing logic here is a _breaking change_ that requires a [`META_FORMAT_VERSION`] bump. @@ -241,11 +240,10 @@ pub(crate) fn get_full_asset_hash( asset_hash: AssetHash, dependency_hashes: impl Iterator, ) -> AssetHash { - let mut context = md5::Context::new(); - context.consume(asset_hash); + let mut hasher = blake3::Hasher::new(); + hasher.update(&asset_hash); for hash in dependency_hashes { - context.consume(hash); + hasher.update(&hash); } - let digest = context.compute(); - digest.0 + *hasher.finalize().as_bytes() }