Skip to content

Remove needless color specializaion for SpritePipeline - #12559

Merged
james7132 merged 1 commit into
bevyengine:mainfrom
Brezak:remove-color-specialization-from-sprite-pipeline@origin
Mar 23, 2024
Merged

james7132 merged 1 commit into
bevyengine:mainfrom
Brezak:remove-color-specialization-from-sprite-pipeline@origin

Conversation

@Brezak

@Brezak Brezak commented Mar 18, 2024 •

Copy link
Copy Markdown
Contributor

Objective

Remove color specialization from SpritePipeline after it became useless in #9597

Solution

Removed the COLORED flag from the pipeline key and removed the specializing the pipeline over it.


Changelog

Removed

  • SpritePipelineKey no longer contains the COLORED flag. The flag has had no effect on how the pipeline operates for a while.

Migration Guide

  • The raw values for the HDR, TONEMAP_IN_SHADER and DEBAND_DITHER flags have changed, so if you were constructing the pipeline key from raw u32s you'll have to account for that.

@github-actions

Copy link
Copy Markdown
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Mar 18, 2024
@github-actions

Copy link
Copy Markdown
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@alice-i-cecile

Copy link
Copy Markdown
Member

This is a breaking change because we changed the pipeline keys, correct?

@Brezak

Brezak commented Mar 18, 2024

Copy link
Copy Markdown
Contributor Author

Appears to be. Question then is. Will the removal impact anybody. The flag currently doesn't affect the shader at all.

@alice-i-cecile

Copy link
Copy Markdown
Member

Looks like they may need to move their DEBAND_DITHER code around to compensate right? I have no problem with it being a breaking change, I just want a one sentence migration guide in the PR description :)

@james7132 james7132 added this to the 0.14 milestone Mar 18, 2024
@Brezak

Brezak commented Mar 18, 2024

Copy link
Copy Markdown
Contributor Author

Updated the PR description to include a change log and migration guide.

@james7132

Copy link
Copy Markdown
Member

Was the shaderdef not being used in the shader?

@Brezak

Brezak commented Mar 19, 2024 •

Copy link
Copy Markdown
Contributor Author

Was the shaderdef not being used in the shader?

No. The pr linked in the description removed it.

@james7132
james7132 added this pull request to the merge queue Mar 23, 2024
Merged via the queue into bevyengine:main with commit d80f05c Mar 23, 2024
@Brezak
Brezak deleted the remove-color-specialization-from-sprite-pipeline@origin branch March 23, 2024 09:41
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-Code-Quality A section of code that is hard to understand or change M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants