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

Small Fix for Vortex subgraph addition #6332

Merged
merged 5 commits into from Feb 9, 2022
Merged

Conversation

peeweek
Copy link
Contributor

@peeweek peeweek commented Nov 18, 2021


Purpose of this PR

Small fix that prevents NaNs in Subgraph block Vortex (included in VFX Additions)


Testing status

N/A


Comments to reviewers

N/A

@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Dec 13, 2021
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Dec 13, 2021
# Conflicts:
#	com.unity.visualeffectgraph/CHANGELOG.md
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Jan 3, 2022
@PaulDemeulenaere
Copy link
Contributor

The actual change is minimal:
Before
image
After
image

However, some remarks about this subgraph:

  1. Branch can be confusing to user (since the ouput will be always world)
    image
    I would suggest to use explicit conversion here 🔽
    image

  2. The space management with subgraph is still buggy (see the change of behavior when the button "save" is pressed):

_vortex_test_bug.mp4

Copy link
Contributor

@PaulDemeulenaere PaulDemeulenaere left a comment

Choose a reason for hiding this comment

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

The actual change is minimal, however, the sugraphblock embedded in additionnal will probably deserve attention. I'm involving @Unity-Technologies/gfx-qa-vfx for visibility.

@PaulDemeulenaere PaulDemeulenaere requested a review from a team January 3, 2022 11:25
Copy link
Contributor

@VladNeykov VladNeykov left a comment

Choose a reason for hiding this comment

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

Minimal change on a VFX sample; no additional testing required.

@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Feb 1, 2022
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Feb 9, 2022
@PaulDemeulenaere PaulDemeulenaere merged commit fa39c64 into master Feb 9, 2022
@PaulDemeulenaere PaulDemeulenaere deleted the vfx/fix/vortex-nan branch February 9, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants