Bug fix: guard asin/acos inputs outside [-1, 1] in osl#2913
Open
bhouston wants to merge 2 commits into
Open
Conversation
Apply the out-of-range asin/acos fallback behavior component-wise for vector2, vector3, and vector4 so the fix is not limited to ND_asin_float and ND_acos_float.
Member
|
@bhouston This is an important issue to address in the cross-language parity for MaterialX, but I don't think the solution is quite as simple as adding new guards in OSL. See #2914 (comment) for thoughts on how we might proceed with this category of improvement. |
This file contains hidden or 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
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.
I noticed via the https://material-fidelity.ben3d.ca test suite that out of range asin/acos values render different in MaterialX OSL as compared to the GLSL / Metal versions.
Before fix render comparisons:

This updates the
genoslstdlib implementations forasinandacosto explicitly guard out-of-domain inputs so that the resulting value matches that produced by materialx glsl.ND_asin_float:asin(in)->mx_ternary(abs(in) <= 1.0, asin(in), 0.0)ND_acos_float:acos(in)->mx_ternary(abs(in) <= 1.0, acos(in), 0.0)Why
In degenerate-but-valid authoring cases, some graphs intentionally feed values outside
[-1, 1]into inverse trig nodes. Backend behavior for out-of-domain inputs can differ (NaN propagation, clamping, or undefined results), which causes renderer divergence.This change makes
genoslbehavior explicit and deterministic for those edge cases, improving cross-backend consistency.Validation
After fix render (I also fixed the blender-materialx-importer as well) comparisons:

Notes
This PR is intentionally minimal and only changes
genoslsource expressions for floatasin/acos.