From a9622408665662eff9ddae83d913c6cfa2fa61d2 Mon Sep 17 00:00:00 2001 From: IceSentry Date: Thu, 5 Oct 2023 08:12:08 -0400 Subject: [PATCH] Alternate wireframe override api (#10023) # Objective https://github.com/bevyengine/bevy/pull/7328 introduced an API to override the global wireframe config. I believe it is flawed for a few reasons. This PR uses a non-breaking API. Instead of making the `Wireframe` an enum I introduced the `NeverRenderWireframe` component. Here's the reason why I think this is better: - Easier to migrate since it doesn't change the old behaviour. Essentially nothing to migrate. Right now this PR is a breaking change but I don't think it has to be. - It's similar to other "per mesh" rendering features like NotShadowCaster/NotShadowReceiver - It doesn't force new users to also think about global vs not global if all they want is to render a wireframe - This would also let you filter at the query definition level instead of filtering when running the query ## Solution - Introduce a `NeverRenderWireframe` component that ignores the global config --- ## Changelog - Added a `NeverRenderWireframe` component that ignores the global `WireframeConfig` --- crates/bevy_pbr/src/wireframe.rs | 62 ++++++++++++++++++-------------- examples/3d/wireframe.rs | 14 ++++---- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/crates/bevy_pbr/src/wireframe.rs b/crates/bevy_pbr/src/wireframe.rs index 9d29fcdbeeef7..114f148705195 100644 --- a/crates/bevy_pbr/src/wireframe.rs +++ b/crates/bevy_pbr/src/wireframe.rs @@ -1,15 +1,13 @@ -use crate::MeshPipeline; use crate::{ - DrawMesh, MeshPipelineKey, RenderMeshInstance, RenderMeshInstances, SetMeshBindGroup, - SetMeshViewBindGroup, + DrawMesh, MeshPipeline, MeshPipelineKey, RenderMeshInstance, RenderMeshInstances, + SetMeshBindGroup, SetMeshViewBindGroup, }; use bevy_app::Plugin; use bevy_asset::{load_internal_asset, Handle}; use bevy_core_pipeline::core_3d::Opaque3d; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{prelude::*, reflect::ReflectComponent}; -use bevy_reflect::std_traits::ReflectDefault; -use bevy_reflect::Reflect; +use bevy_reflect::{std_traits::ReflectDefault, Reflect}; use bevy_render::{ extract_resource::{ExtractResource, ExtractResourcePlugin}, mesh::{Mesh, MeshVertexBufferLayout}, @@ -23,8 +21,7 @@ use bevy_render::{ RenderApp, RenderSet, }; use bevy_render::{Extract, ExtractSchedule, Render}; -use bevy_utils::tracing::error; -use bevy_utils::EntityHashMap; +use bevy_utils::{tracing::error, EntityHashSet}; pub const WIREFRAME_SHADER_HANDLE: Handle = Handle::weak_from_u128(192598014480025766); @@ -41,6 +38,7 @@ impl Plugin for WireframePlugin { ); app.register_type::() + .register_type::() .register_type::() .init_resource::() .add_plugins((ExtractResourcePlugin::::default(),)); @@ -50,6 +48,7 @@ impl Plugin for WireframePlugin { .add_render_command::() .init_resource::>() .init_resource::() + .init_resource::() .add_systems(ExtractSchedule, extract_wireframes) .add_systems(Render, queue_wireframes.in_set(RenderSet::QueueMeshes)); } @@ -62,38 +61,46 @@ impl Plugin for WireframePlugin { } } -/// Overrides the global [`WireframeConfig`] for a single mesh. +/// Enables wireframe rendering for any entity it is attached to. +/// It will ignore the [`WireframeConfig`] global setting. +/// +/// This requires the [`WireframePlugin`] to be enabled. #[derive(Component, Debug, Clone, Default, Reflect, Eq, PartialEq)] #[reflect(Component, Default)] -pub enum Wireframe { - /// Always render the wireframe for this entity, regardless of global config. - #[default] - AlwaysRender, - /// Never render the wireframe for this entity, regardless of global config. - NeverRender, -} +pub struct Wireframe; + +/// Disables wireframe rendering for any entity it is attached to. +/// It will ignore the [`WireframeConfig`] global setting. +/// +/// This requires the [`WireframePlugin`] to be enabled. +#[derive(Component, Debug, Clone, Default, Reflect, Eq, PartialEq)] +#[reflect(Component, Default)] +pub struct NoWireframe; #[derive(Resource, Debug, Clone, Default, ExtractResource, Reflect)] #[reflect(Resource)] pub struct WireframeConfig { /// Whether to show wireframes for all meshes. - /// Can be overridden for individual meshes by adding a [`Wireframe`] component. + /// Can be overridden for individual meshes by adding a [`Wireframe`] or [`NoWireframe`] component. pub global: bool, } #[derive(Resource, Default, Deref, DerefMut)] -pub struct Wireframes(EntityHashMap); +pub struct Wireframes(EntityHashSet); + +#[derive(Resource, Default, Deref, DerefMut)] +pub struct NoWireframes(EntityHashSet); fn extract_wireframes( mut wireframes: ResMut, - query: Extract>, + mut no_wireframes: ResMut, + wireframe_query: Extract>>, + no_wireframe_query: Extract>>, ) { wireframes.clear(); - wireframes.extend( - query - .iter() - .map(|(entity, wireframe)| (entity, wireframe.clone())), - ); + wireframes.extend(wireframe_query.iter()); + no_wireframes.clear(); + no_wireframes.extend(no_wireframe_query.iter()); } #[derive(Resource, Clone)] @@ -137,6 +144,7 @@ fn queue_wireframes( render_meshes: Res>, render_mesh_instances: Res, wireframes: Res, + no_wireframes: Res, wireframe_config: Res, wireframe_pipeline: Res, mut pipelines: ResMut>, @@ -185,11 +193,11 @@ fn queue_wireframes( .entities .iter() .filter_map(|visible_entity| { - let wireframe_override = wireframes.get(visible_entity); + if no_wireframes.get(visible_entity).is_some() { + return None; + } - if (wireframe_config.global || wireframe_override == Some(&Wireframe::AlwaysRender)) - && wireframe_override != Some(&Wireframe::NeverRender) - { + if wireframe_config.global || wireframes.get(visible_entity).is_some() { render_mesh_instances .get(visible_entity) .map(|mesh_instance| (*visible_entity, mesh_instance)) diff --git a/examples/3d/wireframe.rs b/examples/3d/wireframe.rs index bdab092afc919..239a12526bd08 100644 --- a/examples/3d/wireframe.rs +++ b/examples/3d/wireframe.rs @@ -1,7 +1,7 @@ //! Showcases wireframe rendering. use bevy::{ - pbr::wireframe::{Wireframe, WireframeConfig, WireframePlugin}, + pbr::wireframe::{NoWireframe, Wireframe, WireframeConfig, WireframePlugin}, prelude::*, render::{render_resource::WgpuFeatures, settings::WgpuSettings, RenderPlugin}, }; @@ -48,7 +48,7 @@ fn setup( transform: Transform::from_xyz(-1.0, 0.5, -1.0), ..default() }) - .insert(Wireframe::NeverRender); + .insert(NoWireframe); // Orange cube: Follows global wireframe setting commands.spawn(PbrBundle { mesh: meshes.add(Mesh::from(shape::Cube { size: 1.0 })), @@ -64,7 +64,7 @@ fn setup( transform: Transform::from_xyz(1.0, 0.5, 1.0), ..default() }) - .insert(Wireframe::AlwaysRender); + .insert(Wireframe); // light commands.spawn(PointLightBundle { @@ -83,16 +83,16 @@ fn setup( struct WireframeToggleTimer(Timer); /// Periodically turns the global wireframe setting on and off, to show the differences between -/// [`Wireframe::AlwaysRender`], [`Wireframe::NeverRender`], and no override. +/// [`Wireframe`], [`NoWireframe`], and just a mesh. fn toggle_global_wireframe_setting( time: Res