Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - Make PipelineCache internally mutable. #7205

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - Make PipelineCache internally mutable. #7205

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
794ab87
Make PipelineCache internally mutable.
danchia Jan 15, 2023
8399a50
Review comments round 1
danchia Jan 15, 2023
1a159a5
Change mutable usages of PipelineCache to immutable where possible
danchia Jan 15, 2023
1851375
update system ordering
danchia Jan 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/bloom/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl FromWorld for BloomPipelines {
],
});

let mut pipeline_cache = world.resource_mut::<PipelineCache>();
let pipeline_cache = world.resource::<PipelineCache>();

let downsampling_prefilter_pipeline =
pipeline_cache.queue_render_pipeline(RenderPipelineDescriptor {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/fxaa/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl SpecializedRenderPipeline for FxaaPipeline {

pub fn prepare_fxaa_pipelines(
mut commands: Commands,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
mut pipelines: ResMut<SpecializedRenderPipelines<FxaaPipeline>>,
fxaa_pipeline: Res<FxaaPipeline>,
views: Query<(Entity, &ExtractedView, &Fxaa)>,
Expand All @@ -233,7 +233,7 @@ pub fn prepare_fxaa_pipelines(
continue;
}
let pipeline_id = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&fxaa_pipeline,
FxaaPipelineKey {
edge_threshold: fxaa.edge_threshold,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/tonemapping/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub struct ViewTonemappingPipeline(CachedRenderPipelineId);

pub fn queue_view_tonemapping_pipelines(
mut commands: Commands,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
mut pipelines: ResMut<SpecializedRenderPipelines<TonemappingPipeline>>,
upscaling_pipeline: Res<TonemappingPipeline>,
view_targets: Query<(Entity, &Tonemapping)>,
Expand All @@ -136,7 +136,7 @@ pub fn queue_view_tonemapping_pipelines(
let key = TonemappingPipelineKey {
deband_dither: *deband_dither,
};
let pipeline = pipelines.specialize(&mut pipeline_cache, &upscaling_pipeline, key);
let pipeline = pipelines.specialize(&pipeline_cache, &upscaling_pipeline, key);

commands
.entity(entity)
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/upscaling/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub struct ViewUpscalingPipeline(CachedRenderPipelineId);

fn queue_view_upscaling_pipelines(
mut commands: Commands,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
mut pipelines: ResMut<SpecializedRenderPipelines<UpscalingPipeline>>,
upscaling_pipeline: Res<UpscalingPipeline>,
view_targets: Query<(Entity, &ViewTarget)>,
Expand All @@ -122,7 +122,7 @@ fn queue_view_upscaling_pipelines(
upscaling_mode: UpscalingMode::Filtering,
texture_format: view_target.out_texture_format(),
};
let pipeline = pipelines.specialize(&mut pipeline_cache, &upscaling_pipeline, key);
let pipeline = pipelines.specialize(&pipeline_cache, &upscaling_pipeline, key);

commands
.entity(entity)
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/material.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ pub fn queue_material_meshes<M: Material>(
transparent_draw_functions: Res<DrawFunctions<Transparent3d>>,
material_pipeline: Res<MaterialPipeline<M>>,
mut pipelines: ResMut<SpecializedMeshPipelines<MaterialPipeline<M>>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
msaa: Res<Msaa>,
render_meshes: Res<RenderAssets<Mesh>>,
render_materials: Res<RenderMaterials<M>>,
Expand Down Expand Up @@ -379,7 +379,7 @@ pub fn queue_material_meshes<M: Material>(
}

let pipeline_id = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&material_pipeline,
MaterialPipelineKey {
mesh_key,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/render/light.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1626,7 +1626,7 @@ pub fn queue_shadows(
casting_meshes: Query<&Handle<Mesh>, Without<NotShadowCaster>>,
render_meshes: Res<RenderAssets<Mesh>>,
mut pipelines: ResMut<SpecializedMeshPipelines<ShadowPipeline>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
view_lights: Query<&ViewLightEntities>,
mut view_light_shadow_phases: Query<(&LightEntity, &mut RenderPhase<Shadow>)>,
point_light_entities: Query<&CubemapVisibleEntities, With<ExtractedPointLight>>,
Expand Down Expand Up @@ -1661,7 +1661,7 @@ pub fn queue_shadows(
let key =
ShadowPipelineKey::from_primitive_topology(mesh.primitive_topology);
let pipeline_id = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&shadow_pipeline,
key,
&mesh.layout,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/wireframe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ fn queue_wireframes(
wireframe_config: Res<WireframeConfig>,
wireframe_pipeline: Res<WireframePipeline>,
mut pipelines: ResMut<SpecializedMeshPipelines<WireframePipeline>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
msaa: Res<Msaa>,
mut material_meshes: ParamSet<(
Query<(Entity, &Handle<Mesh>, &MeshUniform)>,
Expand All @@ -128,7 +128,7 @@ fn queue_wireframes(
let key = view_key
| MeshPipelineKey::from_primitive_topology(mesh.primitive_topology);
let pipeline_id = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&wireframe_pipeline,
key,
&mesh.layout,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ impl Plugin for RenderPlugin {
.add_stage(
RenderStage::Render,
SystemStage::parallel()
.with_system(PipelineCache::process_pipeline_queue_system)
.with_system(PipelineCache::process_pipeline_queue_system.at_start())
Copy link
Member

Choose a reason for hiding this comment

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

Was this strictly necessary for this PR? Probably makes sense if we ever add other systems to the stage (i.e. command buffer/render bundle based parallelization).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't try, but I believe we do need to make sure this runs before any of the render nodes. Otherwise, you potentially won't be able to access a RenderPipeline that you inserted in the same frame depending on system execution order.

Copy link
Member

Choose a reason for hiding this comment

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

The at_end on the render_system below ensures that all non-exclusive systems, including this one, run before it. Though that might change due to stageless in the not-so-distant future, it's probably fine to omit the at_start here for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah that makes sense. I removed the at_start and left a comment instead, in case we move things around in future.

.with_system(render_system.at_end()),
)
.add_stage(
Expand Down
30 changes: 21 additions & 9 deletions crates/bevy_render/src/render_resource/pipeline_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use bevy_utils::{
tracing::{debug, error},
Entry, HashMap, HashSet,
};
use parking_lot::Mutex;
use std::{hash::Hash, iter::FusedIterator, mem, ops::Deref};
use thiserror::Error;
use wgpu::{PipelineLayoutDescriptor, VertexBufferLayout as RawVertexBufferLayout};
Expand Down Expand Up @@ -343,6 +344,7 @@ pub struct PipelineCache {
device: RenderDevice,
pipelines: Vec<CachedPipeline>,
waiting_pipelines: HashSet<CachedPipelineId>,
new_pipelines: Mutex<Vec<CachedPipeline>>,
}

impl PipelineCache {
Expand All @@ -357,6 +359,7 @@ impl PipelineCache {
layout_cache: default(),
shader_cache: default(),
waiting_pipelines: default(),
new_pipelines: default(),
pipelines: default(),
}
}
Expand Down Expand Up @@ -455,15 +458,15 @@ impl PipelineCache {
/// [`get_render_pipeline_state()`]: PipelineCache::get_render_pipeline_state
/// [`get_render_pipeline()`]: PipelineCache::get_render_pipeline
pub fn queue_render_pipeline(
&mut self,
&self,
descriptor: RenderPipelineDescriptor,
) -> CachedRenderPipelineId {
let id = CachedRenderPipelineId(self.pipelines.len());
self.pipelines.push(CachedPipeline {
let mut new_pipelines = self.new_pipelines.lock();
let id = CachedRenderPipelineId(self.pipelines.len() + new_pipelines.len());
new_pipelines.push(CachedPipeline {
descriptor: PipelineDescriptor::RenderPipelineDescriptor(Box::new(descriptor)),
state: CachedPipelineState::Queued,
});
self.waiting_pipelines.insert(id.0);
id
}

Expand All @@ -481,15 +484,15 @@ impl PipelineCache {
/// [`get_compute_pipeline_state()`]: PipelineCache::get_compute_pipeline_state
/// [`get_compute_pipeline()`]: PipelineCache::get_compute_pipeline
pub fn queue_compute_pipeline(
&mut self,
&self,
descriptor: ComputePipelineDescriptor,
) -> CachedComputePipelineId {
let id = CachedComputePipelineId(self.pipelines.len());
self.pipelines.push(CachedPipeline {
let mut new_pipelines = self.new_pipelines.lock();
let id = CachedComputePipelineId(self.pipelines.len() + new_pipelines.len());
new_pipelines.push(CachedPipeline {
descriptor: PipelineDescriptor::ComputePipelineDescriptor(Box::new(descriptor)),
state: CachedPipelineState::Queued,
});
self.waiting_pipelines.insert(id.0);
id
}

Expand Down Expand Up @@ -632,9 +635,18 @@ impl PipelineCache {
///
/// [`RenderStage::Render`]: crate::RenderStage::Render
pub fn process_queue(&mut self) {
let waiting_pipelines = mem::take(&mut self.waiting_pipelines);
let mut waiting_pipelines = mem::take(&mut self.waiting_pipelines);
let mut pipelines = mem::take(&mut self.pipelines);

{
let mut new_pipelines = self.new_pipelines.lock();
for new_pipeline in new_pipelines.drain(..) {
let id = pipelines.len();
pipelines.push(new_pipeline);
waiting_pipelines.insert(id);
}
}

for id in waiting_pipelines {
let pipeline = &mut pipelines[id];
if matches!(pipeline.state, CachedPipelineState::Ok(_)) {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_render/src/render_resource/pipeline_specializer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl<S: SpecializedRenderPipeline> Default for SpecializedRenderPipelines<S> {
impl<S: SpecializedRenderPipeline> SpecializedRenderPipelines<S> {
pub fn specialize(
&mut self,
cache: &mut PipelineCache,
cache: &PipelineCache,
specialize_pipeline: &S,
key: S::Key,
) -> CachedRenderPipelineId {
Expand Down Expand Up @@ -103,7 +103,7 @@ impl<S: SpecializedMeshPipeline> SpecializedMeshPipelines<S> {
#[inline]
pub fn specialize(
&mut self,
cache: &mut PipelineCache,
cache: &PipelineCache,
specialize_pipeline: &S,
key: S::Key,
layout: &MeshVertexBufferLayout,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_sprite/src/mesh2d/material.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ pub fn queue_material2d_meshes<M: Material2d>(
transparent_draw_functions: Res<DrawFunctions<Transparent2d>>,
material2d_pipeline: Res<Material2dPipeline<M>>,
mut pipelines: ResMut<SpecializedMeshPipelines<Material2dPipeline<M>>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
msaa: Res<Msaa>,
render_meshes: Res<RenderAssets<Mesh>>,
render_materials: Res<RenderMaterials2d<M>>,
Expand Down Expand Up @@ -351,7 +351,7 @@ pub fn queue_material2d_meshes<M: Material2d>(
| Mesh2dPipelineKey::from_primitive_topology(mesh.primitive_topology);

let pipeline_id = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&material2d_pipeline,
Material2dKey {
mesh_key,
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_sprite/src/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ pub fn queue_sprites(
view_uniforms: Res<ViewUniforms>,
sprite_pipeline: Res<SpritePipeline>,
mut pipelines: ResMut<SpecializedRenderPipelines<SpritePipeline>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
mut image_bind_groups: ResMut<ImageBindGroups>,
gpu_images: Res<RenderAssets<Image>>,
msaa: Res<Msaa>,
Expand Down Expand Up @@ -528,12 +528,12 @@ pub fn queue_sprites(
}
}
let pipeline = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&sprite_pipeline,
view_key | SpritePipelineKey::from_colored(false),
);
let colored_pipeline = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&sprite_pipeline,
view_key | SpritePipelineKey::from_colored(true),
);
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ui/src/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ pub fn queue_uinodes(
view_uniforms: Res<ViewUniforms>,
ui_pipeline: Res<UiPipeline>,
mut pipelines: ResMut<SpecializedRenderPipelines<UiPipeline>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
mut image_bind_groups: ResMut<UiImageBindGroups>,
gpu_images: Res<RenderAssets<Image>>,
ui_batches: Query<(Entity, &UiBatch)>,
Expand Down Expand Up @@ -586,7 +586,7 @@ pub fn queue_uinodes(
let draw_ui_function = draw_functions.read().id::<DrawUi>();
for (view, mut transparent_phase) in &mut views {
let pipeline = pipelines.specialize(
&mut pipeline_cache,
&pipeline_cache,
&ui_pipeline,
UiPipelineKey { hdr: view.hdr },
);
Expand Down
4 changes: 2 additions & 2 deletions examples/2d/mesh2d_manual.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ pub fn queue_colored_mesh2d(
transparent_draw_functions: Res<DrawFunctions<Transparent2d>>,
colored_mesh2d_pipeline: Res<ColoredMesh2dPipeline>,
mut pipelines: ResMut<SpecializedRenderPipelines<ColoredMesh2dPipeline>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
msaa: Res<Msaa>,
render_meshes: Res<RenderAssets<Mesh>>,
colored_mesh2d: Query<(&Mesh2dHandle, &Mesh2dUniform), With<ColoredMesh2d>>,
Expand Down Expand Up @@ -343,7 +343,7 @@ pub fn queue_colored_mesh2d(
}

let pipeline_id =
pipelines.specialize(&mut pipeline_cache, &colored_mesh2d_pipeline, mesh2d_key);
pipelines.specialize(&pipeline_cache, &colored_mesh2d_pipeline, mesh2d_key);

let mesh_z = mesh2d_uniform.transform.w_axis.z;
transparent_phase.add(Transparent2d {
Expand Down
2 changes: 1 addition & 1 deletion examples/shader/compute_shader_game_of_life.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl FromWorld for GameOfLifePipeline {
let shader = world
.resource::<AssetServer>()
.load("shaders/game_of_life.wgsl");
let mut pipeline_cache = world.resource_mut::<PipelineCache>();
let pipeline_cache = world.resource::<PipelineCache>();
let init_pipeline = pipeline_cache.queue_compute_pipeline(ComputePipelineDescriptor {
label: None,
layout: Some(vec![texture_bind_group_layout.clone()]),
Expand Down
4 changes: 2 additions & 2 deletions examples/shader/shader_instancing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ fn queue_custom(
custom_pipeline: Res<CustomPipeline>,
msaa: Res<Msaa>,
mut pipelines: ResMut<SpecializedMeshPipelines<CustomPipeline>>,
mut pipeline_cache: ResMut<PipelineCache>,
pipeline_cache: Res<PipelineCache>,
meshes: Res<RenderAssets<Mesh>>,
material_meshes: Query<(Entity, &MeshUniform, &Handle<Mesh>), With<InstanceMaterialData>>,
mut views: Query<(&ExtractedView, &mut RenderPhase<Transparent3d>)>,
Expand All @@ -121,7 +121,7 @@ fn queue_custom(
let key =
view_key | MeshPipelineKey::from_primitive_topology(mesh.primitive_topology);
let pipeline = pipelines
.specialize(&mut pipeline_cache, &custom_pipeline, key, &mesh.layout)
.specialize(&pipeline_cache, &custom_pipeline, key, &mesh.layout)
.unwrap();
transparent_phase.add(Transparent3d {
entity,
Expand Down