-
-
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
[Merged by Bors] - Implement Clone
for all pipeline types
#6653
Conversation
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.
not an expert here but LGTM
@@ -237,6 +237,18 @@ pub struct MaterialPipeline<M: Material> { | |||
marker: PhantomData<M>, | |||
} | |||
|
|||
impl<M: Material> Clone for MaterialPipeline<M> { | |||
fn clone(&self) -> Self { |
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.
fn clone(&self) -> Self { | |
// This needs to be manually implemented, as we do not want to force a M: Clone trait bound | |
fn clone(&self) -> Self { |
Left a suggestion for a comment to clarify the pattern used, but it's non-blocking. I'll let the rendering folks (cc @superdump) merge this one though. |
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.
Not sure if this will be a common use case, but for small wrapper pipelines, this seems totally reasonable. Do you have a specific example in mind?
LGTM
I believe my use case was impl FromWorld for Wireframe2dPipeline {
fn from_world(world: &mut World) -> Self {
let p = &world.resource::<Material2dPipeline<ColorMaterial>>();
Self {
mesh2d_pipeline: p.mesh2d_pipeline.clone(),
material2d_layout: p.material2d_layout.clone(),
vertex_shader: p.vertex_shader.clone(),
fragment_shader: p.fragment_shader.clone(),
}
}
} If |
That makes sense, thank you! Maybe add this to the PR description :). |
Done |
bors r+ |
# Objective Pipelines can be customized by wrapping an existing pipeline in a newtype and adding custom logic to its implementation of `SpecializedMeshPipeline::specialize`. To make that easier, the wrapped pipeline type needs to implement `Clone`. For example, the current non-cloneable pipelines require wrapper pipelines to pull apart the wrapped pipeline like this: ```rust impl FromWorld for Wireframe2dPipeline { fn from_world(world: &mut World) -> Self { let p = &world.resource::<Material2dPipeline<ColorMaterial>>(); Self { mesh2d_pipeline: p.mesh2d_pipeline.clone(), material2d_layout: p.material2d_layout.clone(), vertex_shader: p.vertex_shader.clone(), fragment_shader: p.fragment_shader.clone(), } } } ``` ## Solution Derive or implement `Clone` on all built-in pipeline types. This is easy to do since they mostly just contain cheaply clonable reference-counted types. --- ## Changelog Implement `Clone` for all pipeline types.
Clone
for all pipeline typesClone
for all pipeline types
# Objective Pipelines can be customized by wrapping an existing pipeline in a newtype and adding custom logic to its implementation of `SpecializedMeshPipeline::specialize`. To make that easier, the wrapped pipeline type needs to implement `Clone`. For example, the current non-cloneable pipelines require wrapper pipelines to pull apart the wrapped pipeline like this: ```rust impl FromWorld for Wireframe2dPipeline { fn from_world(world: &mut World) -> Self { let p = &world.resource::<Material2dPipeline<ColorMaterial>>(); Self { mesh2d_pipeline: p.mesh2d_pipeline.clone(), material2d_layout: p.material2d_layout.clone(), vertex_shader: p.vertex_shader.clone(), fragment_shader: p.fragment_shader.clone(), } } } ``` ## Solution Derive or implement `Clone` on all built-in pipeline types. This is easy to do since they mostly just contain cheaply clonable reference-counted types. --- ## Changelog Implement `Clone` for all pipeline types.
# Objective Pipelines can be customized by wrapping an existing pipeline in a newtype and adding custom logic to its implementation of `SpecializedMeshPipeline::specialize`. To make that easier, the wrapped pipeline type needs to implement `Clone`. For example, the current non-cloneable pipelines require wrapper pipelines to pull apart the wrapped pipeline like this: ```rust impl FromWorld for Wireframe2dPipeline { fn from_world(world: &mut World) -> Self { let p = &world.resource::<Material2dPipeline<ColorMaterial>>(); Self { mesh2d_pipeline: p.mesh2d_pipeline.clone(), material2d_layout: p.material2d_layout.clone(), vertex_shader: p.vertex_shader.clone(), fragment_shader: p.fragment_shader.clone(), } } } ``` ## Solution Derive or implement `Clone` on all built-in pipeline types. This is easy to do since they mostly just contain cheaply clonable reference-counted types. --- ## Changelog Implement `Clone` for all pipeline types.
Objective
Pipelines can be customized by wrapping an existing pipeline in a newtype and adding custom logic to its implementation of
SpecializedMeshPipeline::specialize
. To make that easier, the wrapped pipeline type needs to implementClone
.For example, the current non-cloneable pipelines require wrapper pipelines to pull apart the wrapped pipeline like this:
Solution
Derive or implement
Clone
on all built-in pipeline types. This is easy to do since they mostly just contain cheaply clonable reference-counted types.Changelog
Implement
Clone
for all pipeline types.