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

EMSUSD-1896 avoid crash when shader has no uvLink #4136

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

pierrebai-adsk
Copy link
Collaborator

The uvLink command can return nothing if it cannot determine the UV link of the shader node. This would break assumptions in the export code and lead to a crash when accessing missing data. Handle the case where the uvLink command returns nothing to prevent that crash.

Add a unit test that reproduced the crash and now no longer crashes.

@pierrebai-adsk pierrebai-adsk added bug Something isn't working import-export Related to Import and/or Export crash labels Feb 25, 2025
The uvLink command can return nothing if it cannot determine the UV link
of the shader node. This would break assumptions in the export code and
lead to a crash when accessing missing data. Handle the case where the
uvLink command returns nothing to prevent that crash.

Add a unit test that reproduced the crash and now no longer crashes.
@pierrebai-adsk pierrebai-adsk force-pushed the bailp/EMSUSD-1896/export-shader-crash branch from ed52c58 to 135f140 Compare February 25, 2025 17:14
@pierrebai-adsk pierrebai-adsk self-assigned this Feb 25, 2025
@pierrebai-adsk
Copy link
Collaborator Author

The changes are good? I was unsure how to best handle the missing uvLink. I was wondering if maybe the command did not support the connected node? It might be worth digging into this further in the future?

For now, all I wanted was to at least avid the crash.

@JGamache-autodesk
Copy link
Collaborator

A slightly better fix would be to pick the first UV set available on the geometry if there is any, but I am perfectly happy with "not crashing, no UV" when using exotic nodes like that doubleSwitch.

@pierrebai-adsk
Copy link
Collaborator Author

There were UV in the resulting file for that node. The UV were not replaced by that "largest" one.

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Feb 26, 2025
@seando-adsk seando-adsk merged commit b5d7517 into dev Feb 28, 2025
13 of 15 checks passed
@seando-adsk seando-adsk deleted the bailp/EMSUSD-1896/export-shader-crash branch February 28, 2025 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crash import-export Related to Import and/or Export ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants