-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Retain Rendering World #14449
Retain Rendering World #14449
Changes from 63 commits
95b7f0b
17f2bb0
35fe01e
f9bae87
34853b2
000cef4
2255b21
f90c360
756b082
5b5ef76
e15e3a3
e4e0bc0
33cae8d
4a58b0e
3fc2207
f9c8d5a
44d36a4
9a7d641
7989d21
9485455
bb3289b
39e4f2e
4b6506e
03a6e09
b8b8225
24212f5
2de636a
1fbb76c
c25d560
457e83f
16c1093
cba1183
2821849
a86dd39
8422a48
dc5f10f
2a6df8d
ce2ea36
c1a4fb9
672778d
0eed037
5a83da4
ed296f1
bb8774d
94be0cd
560ce59
ab22138
fd36379
4bdfd60
6214e6d
67e7a07
1507437
e5254be
d9ecccd
faf5ef2
f127719
c7ba466
23a8bba
37944a8
1e8601f
7167cc0
bfad1de
8d2c81a
d857e6e
9c1a523
753d286
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ use crate::tonemapping::{DebandDither, Tonemapping}; | |
use bevy_ecs::prelude::*; | ||
use bevy_reflect::Reflect; | ||
use bevy_render::prelude::Msaa; | ||
use bevy_render::world_sync::SyncRenderWorld; | ||
use bevy_render::{ | ||
camera::{ | ||
Camera, CameraMainTextureUsages, CameraProjection, CameraRenderGraph, | ||
|
@@ -37,6 +38,8 @@ pub struct Camera2dBundle { | |
pub deband_dither: DebandDither, | ||
pub main_texture_usages: CameraMainTextureUsages, | ||
pub msaa: Msaa, | ||
/// Marker component that indicates that its entity needs to be Synchronized to the render world | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For here and all other comments, please do not capitalize "Synchronized" in the middle of the sentence. |
||
pub sync: SyncRenderWorld, | ||
} | ||
|
||
impl Default for Camera2dBundle { | ||
|
@@ -61,6 +64,7 @@ impl Default for Camera2dBundle { | |
deband_dither: DebandDither::Disabled, | ||
main_texture_usages: Default::default(), | ||
msaa: Default::default(), | ||
sync: Default::default(), | ||
} | ||
} | ||
} | ||
|
@@ -94,6 +98,7 @@ impl Camera2dBundle { | |
deband_dither: DebandDither::Disabled, | ||
main_texture_usages: Default::default(), | ||
msaa: Default::default(), | ||
sync: Default::default(), | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ use bevy_render::{ | |
renderer::{RenderContext, RenderDevice}, | ||
texture::{BevyDefault, CachedTexture, TextureCache}, | ||
view::{ExtractedView, Msaa, ViewTarget}, | ||
world_sync::RenderEntity, | ||
ExtractSchedule, MainWorld, Render, RenderApp, RenderSet, | ||
}; | ||
use bevy_utils::tracing::warn; | ||
|
@@ -346,17 +347,22 @@ impl SpecializedRenderPipeline for TaaPipeline { | |
} | ||
|
||
fn extract_taa_settings(mut commands: Commands, mut main_world: ResMut<MainWorld>) { | ||
let mut cameras_3d = main_world | ||
.query_filtered::<(Entity, &Camera, &Projection, &mut TemporalAntiAliasSettings), ( | ||
With<Camera3d>, | ||
With<TemporalJitter>, | ||
With<DepthPrepass>, | ||
With<MotionVectorPrepass>, | ||
)>(); | ||
let mut cameras_3d = main_world.query_filtered::<( | ||
&RenderEntity, | ||
&Camera, | ||
&Projection, | ||
&mut TemporalAntiAliasSettings, | ||
), ( | ||
With<Camera3d>, | ||
With<TemporalJitter>, | ||
With<DepthPrepass>, | ||
With<MotionVectorPrepass>, | ||
)>(); | ||
|
||
for (entity, camera, camera_projection, mut taa_settings) in | ||
cameras_3d.iter_mut(&mut main_world) | ||
{ | ||
let entity = entity.id(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tiny nit: entity is only used once here, I'd rather not create a new variable, and just use entity.id() directly in commands.get_or_spawn() |
||
let has_perspective_projection = matches!(camera_projection, Projection::Perspective(_)); | ||
if camera.is_active && has_perspective_projection { | ||
commands.get_or_spawn(entity).insert(taa_settings.clone()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets see if I understand this correctly: this could be just get_mut() here instead of get_or_spawn(), as the entity should already exist as camera bundles have SyncRenderWorld, correct? (That said I do think we should leave it as get_or_spawn() in case users aren't using the bundle, this comment is just to see if I understand this PR correctly or not) |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ use bevy_render::{ | |
UniformBuffer, | ||
}, | ||
renderer::{RenderDevice, RenderQueue}, | ||
world_sync::RenderEntity, | ||
Extract, | ||
}; | ||
use bevy_utils::{hashbrown::HashSet, tracing::warn}; | ||
|
@@ -512,7 +513,8 @@ pub(crate) fn clusterable_object_order( | |
/// Extracts clusters from the main world from the render world. | ||
pub fn extract_clusters( | ||
mut commands: Commands, | ||
views: Extract<Query<(Entity, &Clusters, &Camera)>>, | ||
views: Extract<Query<(&RenderEntity, &Clusters, &Camera)>>, | ||
mapper: Extract<Query<&RenderEntity>>, | ||
re0312 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) { | ||
for (entity, clusters, camera) in &views { | ||
if !camera.is_active { | ||
|
@@ -531,13 +533,15 @@ pub fn extract_clusters( | |
cluster_objects.spot_light_count as u32, | ||
)); | ||
for clusterable_entity in &cluster_objects.entities { | ||
data.push(ExtractedClusterableObjectElement::ClusterableObjectEntity( | ||
*clusterable_entity, | ||
)); | ||
if let Ok(entity) = mapper.get(*clusterable_entity) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we modify the clusterizer system to store RenderEntity on Clusters directly, instead of using another query here? |
||
data.push(ExtractedClusterableObjectElement::ClusterableObjectEntity( | ||
entity.id(), | ||
)); | ||
} | ||
} | ||
} | ||
|
||
commands.get_or_spawn(entity).insert(( | ||
commands.get_or_spawn(entity.id()).insert(( | ||
ExtractedClusterableObjects { data }, | ||
ExtractedClusterConfig { | ||
near: clusters.near, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -420,6 +420,9 @@ impl Plugin for PbrPlugin { | |
) | ||
.init_resource::<LightMeta>(); | ||
|
||
render_app.world_mut().observe(add_light_view_entities); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are these two lines doing here ? |
||
render_app.world_mut().observe(remove_light_view_entities); | ||
|
||
let shadow_pass_node = ShadowPassNode::new(render_app.world_mut()); | ||
let mut graph = render_app.world_mut().resource_mut::<RenderGraph>(); | ||
let draw_3d_graph = graph.get_sub_graph_mut(Core3d).unwrap(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's quite a lot of noise in this PR due to the need to insert these
RenderEntity::id
calls. This might be a controversial suggestion, but we can implementWorldQuery
forRenderEntity
so it still behaves like aComponent
, but also returns the innerEntity
when queried directly.unsafe impl WorldQuery for RenderEntity { ... }
This would then allow for render systems to query for
RenderEntity
and directly receive the innerEntity
, reducing the noise in this PR. Crucially, this doesn't conflict with the existingderive(Component)
, so you can still query for&RenderEntity
, the mutable version, use it in filters, etc.