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

[ShaderGraph] [bugfix 1264932] "Properly" handle null textures in preview #1702

Merged
merged 4 commits into from
Sep 1, 2020

Conversation

cdxntchou
Copy link
Contributor

@cdxntchou cdxntchou commented Aug 28, 2020

Purpose of this PR

Fixes bug https://fogbugz.unity3d.com/f/cases/1264932/
Null textures were not updated by the preview code.
Turns out it is kind of impossible to revert a Material or MaterialPropertyBlock back to the "null" state.
So instead we set the texture to the known default (luckily all ShaderGraph textures default to "white").


Testing status

Manual Tests: What did you do?
Tested ShaderGraph in editor with Texture2D properties and inline assets, Cubemap properties and inline assets; setting values to textures, then none and back.
Tested initial state is correct on graph load.
Tested setting null or texture using undo/redo works properly.

Yamato: (Select your branch):
https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics/tree/sg%252Ffix%252F1264932

Any test projects to go with this to help reviewers?


Comments to reviewers

Notes for the reviewers you have assigned.

@cdxntchou cdxntchou requested a review from a user August 28, 2020 18:31
@cdxntchou cdxntchou marked this pull request as ready for review August 28, 2020 18:34
@cdxntchou cdxntchou requested a review from a team as a code owner August 28, 2020 18:34
works exactly the same, but might as well make it match
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Developer manual tests are sufficient

@cdxntchou
Copy link
Contributor Author

cdxntchou commented Sep 1, 2020

Yamato is all green, except OSX Metal, which is failing on trunk as well.

@cdxntchou cdxntchou merged commit 97e1048 into master Sep 1, 2020
@cdxntchou cdxntchou deleted the sg/fix/1264932 branch September 1, 2020 18:47
@ghost ghost removed the needs-backport-9.x label Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants