Skip to content

Move ShaderCache shader defs into a resource #8202

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 1 commit into
base: main
Choose a base branch
from

Conversation

ProfLander
Copy link
Contributor

Objective

Solution

  • Move said defs into a new BaseShaderDefs newtype resource, and have consuming pipelines retrieve it in their FromWorld implementation for use during specialization.

Changelog

  • Removed late def injection from ShaderCache
  • BaseShaderDefs resource implemented to hold affected defs, inserted into the render world by RenderPlugin
  • Extended existing consumers of the related defs with logic to fetch data from BaseShaderDefs in FromWorld, and use it in lieu of constructing an empty defs vec in specialize:
    • MeshPipeline
    • Mesh2dPipeline
    • PrepassPipeline
    • GizmoPipeline

Migration Guide

This is a non-breaking change, but future pipeline implementors should be made aware that the defs in question should be manually sourced from the ECS.

@ProfLander ProfLander force-pushed the shader-defs-resource-main branch from 14c5663 to f1b18fa Compare March 24, 2023 23:44
@james7132 james7132 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Mar 29, 2023
Copy link
Member

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

This helps clean up the ShaderCache code itself and I like having these in a default location. The disadvantage now is that users of pipeline specialization don't get these "for free." Overall seems like a good change since it helps prevent an annoying error, but I'd like more documentation somewhere about what our "base" shader defs are for users who might want to use them for consistency.

@tychedelia tychedelia added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Aug 16, 2024
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-Bug An unexpected or incorrect behavior S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Material::specialize is provided an incomplete set of engine-sourced shader definitions
3 participants