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

Video: Fix Post Process shader options issues #11899

Merged
merged 1 commit into from Jun 8, 2023

Conversation

Filoppi
Copy link
Contributor

@Filoppi Filoppi commented Jun 7, 2023

  • An assert would be erroneously thrown when shaders declared an array of 4 int or float options, despite 4 being the max supported (a simple <= / < mistake)
  • When changing the type of a shader option (e.g. from bool to float), the serialization would be stuck appending the value from the previous option type, making the shader fail to build permanently until the cache were cleaned

-An assert would be erroneously thrown when shaders declared an array of 4 int or float options, despite 4 being the max supported (a simple <= / < mistake)
-When changing the type of a shader option (e.g. from bool to float), the serialization would be stuck appending the value from the previous option type, making the shader fail to build permanently until the cache were cleaned
Comment on lines +258 to +264
{
auto integer_values = it.second.m_integer_values;
if (TryParseVector(value, &integer_values))
{
it.second.m_integer_values = integer_values;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand why exactly this behaves differently than before. I guess in case of parse failure the previous values are kept instead of deleted, but why would that break the parsing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the parameters for the post process shaders ([configuration]) are read they are saved within the options array (inside m_integer_values e.g.), and the shader can also define their default values, which will be baked in the final shader string that is later constructed.

If these options had already previously been serialized with one type (e.g. bool) and have now changed type, the deserialization (loading) will fail to fill up the (e.g.) m_float_values (because the serialized data wasn't float). So the m_float_values will be cleared by the TryParseVector() and the defaults values from the shader itself will be cleared.
The shader will then proceed to add a string like:
float some_var = ; without a value assigned to it and fail to compile.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay. That seems poorly designed but yeah okay, that's a reasonable workaround then.

@AdmiralCurtiss AdmiralCurtiss merged commit 78f5c5f into dolphin-emu:master Jun 8, 2023
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants