bevy
bevy copied to clipboard
Merge PipelineIds and CachedPipelineIds
Objective
- Motivated by #7188
- Simplifies the pipeline IDs.
Currently, we use atomic IDs for all of our render resources (RenderPipelineId, ComputePipelineId).
Additionally, we assign a unique CachedRenderPipelineId/CachedComputePipelineId to each of our pipelines in the PipelineCache.
This PR merges both of them together, to simplify the API.
Solution
- This PR uses the atomically incrementing IDs directly to index into a cached pipeline vector. This enables cheap and safe lookups.
- This implies that all pipelines have to be created via the
PipelineCache. - Simply queue your pipeline into the cache (or let a specialized pipeline do the job) and retrieve it via its ID.
- We can still create raw pipelines using the wgpu API directly, to allow for third-party integration.
Changelog
- replaced all occurrences of
CachedRenderPipelineIdandCachedComputePipelineIdin favor ofRenderPipelineIdandComputePipelineId - removed
PipelineCache::pipelines - renamed
PipelineCache::get_render_pipeline_statetoPipelineCache::render_pipeline_state - renamed
PipelineCache::get_compute_pipeline_statetoPipelineCache::compute_pipeline_state - renamed
PipelineCache::get_render_pipeline_descriptortoPipelineCache::render_pipeline_descriptor - renamed
PipelineCache::get_compute_pipeline_descriptortoPipelineCache::compute_pipeline_descriptor
Migration Guide
- replaced all occurrences of
CachedRenderPipelineIdandCachedComputePipelineIdin favor ofRenderPipelineIdandComputePipelineId - removed
PipelineCache::pipelines - renamed
PipelineCache::get_render_pipeline_statetoPipelineCache::render_pipeline_state - renamed
PipelineCache::get_compute_pipeline_statetoPipelineCache::compute_pipeline_state - renamed
PipelineCache::get_render_pipeline_descriptortoPipelineCache::render_pipeline_descriptor - renamed
PipelineCache::get_compute_pipeline_descriptortoPipelineCache::compute_pipeline_descriptor
My first thought when seeing the description of this PR is that when we start removing unused pipelines, how do we manage that they have been removed so we can destroy them? That makes me think of generation indices, and that makes me think "pipelines as entities?" :)
Mhh, that sounds interesting. Not sure what that would look like though. This solution is not really a regression, because we have been doing this up until now anyways.
I will think about it.
Also we would probably need persistent entities in the render world first.
I have made a quick demo of the concept here.
I really like this design, unfortunately, we still can't persist entities in the render world.
fn old(pipeline_cache: Res<PipelineCache>) {
let descriptor = RenderPipelineDescriptor::default();
let id = pipeline_cache.queue_render_pipeline(descriptor);
let pipeline = pipeline_cache.get_render_pipeline(id).unwrap();
}
fn new(mut commands: Commands, pipelines: Query<&RenderPipeline>) {
let descriptor = RenderPipelineDescriptor::default();
let entity = commands.spawn((descriptor, PipelineState::Queued));
// or
let entity = commands.add_render_pipeline(descriptor);
let pipeline = pipelines.get(entity).unwrap();
}
But once that is resolved, I will implement this.
Could we separate this from #7188? It's unclear if we want to merge those two, and I would like for this redundancy to be removed.
Yeah sure, I will do that today. 👍
Could we separate this from #7188? It's unclear if we want to merge those two, and I would like for this redundancy to be removed.
This PR is now independent.
I still believe that there is value in both PRs. #7188 should not be controversial if we can not store raw wgpu Ids anyway. Also, it has already gotten cart's approval (and it's a pretty trivial change).
I'm on board for the move toward simplification / more opinionated apis and we can then break out when the need arises.