Skip to content

Improve documentation on Specialized*Pipeline*. #19494

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

andriyDev
Copy link
Contributor

Objective

  • Title.

Solution

  • Add an explanation of the various traits and structs.
  • Make VertexLayoutCache private - this type is not used anywhere, and it's just a type alias for a HashMap with no other utility functions.
  • Rename specialize_pipeline argument to pipeline_specializer to clarify this is more like a factory than a render pipeline.

Technically making VertexLayoutCache private is a breaking change, but it seems highly unlikely that anyone is using this (since it's just a hashmap alias. I don't think this needs a migration guide.

@andriyDev andriyDev added C-Docs An addition or correction to our documentation A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 4, 2025
@andriyDev andriyDev requested a review from JMS55 June 4, 2025 21:16
@@ -18,11 +18,29 @@ use core::{fmt::Debug, hash::Hash};
use thiserror::Error;
use tracing::error;

/// A trait that allows constructing different variants of a render pipeline from a key.
///
/// Note: If your key is a ZST, you do not need to "specialize" anything. Just create the render
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really get what this note is saying 🤔

Copy link
Contributor Author

@andriyDev andriyDev Jun 5, 2025

Choose a reason for hiding this comment

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

The only reason to specialize a pipeline is if your key contains some information that results in different versions of the render pipeline. If your key is a zero-sized type (ZST), there is no reason to use this (since all your render pipelines would just be the same). It is simpler and easier to just use the PipelineCache directly to create the render pipeline.

Is there anything I can adjust to make this clearer? Not sure where the confusion is (perhaps ZST was confusing?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also please correct me if I'm missing something here! This is my understanding based on just investigating the code.

Copy link
Member

Choose a reason for hiding this comment

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

I would maybe just rephrase it to say that specialization is intended to modify the pipeline descriptor and cached on the basis of the key. The relevant scenario here would be if someone was using AsBindGroup derive without the #[bind_group_data] attr, then you don't need to specialize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've reworded it to match what you've said!

@andriyDev andriyDev requested a review from JMS55 June 5, 2025 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants