Skip to content
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

VideoBackends: add support to allow rendering to multiple output targets #11859

Merged
merged 2 commits into from
Jun 9, 2023

Conversation

iwubcode
Copy link
Contributor

This was used in #10362 and will be used in future features. Splitting it out to cut down on scope and make it easier to review

@iwubcode iwubcode force-pushed the backend-multi-output branch 5 times, most recently from 56fb8cc to 72af1dd Compare May 29, 2023 07:44
@iwubcode iwubcode marked this pull request as draft May 30, 2023 04:58
@iwubcode

This comment was marked as resolved.

@iwubcode iwubcode force-pushed the backend-multi-output branch 2 times, most recently from af918f4 to a1715f1 Compare May 31, 2023 00:57
@iwubcode iwubcode marked this pull request as ready for review May 31, 2023 01:13
@iwubcode iwubcode force-pushed the backend-multi-output branch from a1715f1 to 8c3dc5b Compare June 3, 2023 19:52
Copy link
Contributor

@TellowKrinkle TellowKrinkle left a comment

Choose a reason for hiding this comment

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

Up to you whether you want to hope no one somehow ended up with nonzero additional color attachments in their UIDs

@@ -19,7 +19,7 @@ namespace VideoCommon
// As pipelines encompass both shader UIDs and render states, changes to either of these should
// also increment the pipeline UID version. Incrementing the UID version will cause all UID
// caches to be invalidated.
constexpr u32 GX_PIPELINE_UID_VERSION = 6; // Last changed in PR 10890
constexpr u32 GX_PIPELINE_UID_VERSION = 7; // Last changed in PR 11859
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, adding a new field to an existing bitfield where zero is the same as the current usage shouldn't break any UIDs

@AdmiralCurtiss
Copy link
Contributor

So are we bumping the UID or not?

@iwubcode
Copy link
Contributor Author

iwubcode commented Jun 9, 2023

@AdmiralCurtiss -

FYI I asked Tellow what they meant by their comment and this was their response:

As in, I can't deny the possibility that we accidentally left garbage in the padding in some code somewhere
So if you're worried about that, then invalidating UIDs would be a good idea

I wouldn't say I'm worried but I didn't think it hurt anything, so figured why not be safe? Happy to go either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants