Modify NaN propagation for multiplication #779
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dpw13
force-pushed
the
dev/dwagner-nan-prop-1
branch
from
36d590a to
762c146
Compare
dpw13
commented
Feb 16, 2024
| @@ -619,8 +619,8 @@ const Intrinsic _intrinsic[] = | |||
| INTRINSIC_FLOAT2_FUNCTION( "step" ), | |||
| INTRINSIC_FLOAT2_FUNCTION( "reflect" ), | |||
|
|
|||
| INTRINSIC_FLOAT1_FUNCTION("isnan"), | |||
| INTRINSIC_FLOAT1_FUNCTION("isinf"), | |||
| Intrinsic("isnan", HLSLBaseType_Bool, HLSLBaseType_Float), | |||
There was a problem hiding this comment.
I'm not aware of any presets that use these functions, but in trying to identify NaNs and Infs in shaders I found that the return type was declared incorrectly here. Changing this line results in the resulting shader correctly casting the output where necessary.
kblaschke
approved these changes
Feb 21, 2024
dpw13
force-pushed
the
dev/dwagner-nan-prop-1
branch
from
762c146 to
b3a1483
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This solves some edge cases in presets that suppress NaN or infinities by multiplying by zero. It's not clear what the performance impact is with this change, and it may not be something we want to always enable. Because of that, I've added an option flag to
GLSLGenerator::Generate()to enable or disable the behavior. This may be something we don't want to end up merging if the performance impact is too high.