Skip to content

Conversation

elizabeth-legros
Copy link
Contributor

Backport of #1413
Ran graphics tests in ShaderGraph test project locally

@elizabeth-legros elizabeth-legros marked this pull request as ready for review October 7, 2020 21:56
@elizabeth-legros elizabeth-legros requested a review from a team as a code owner October 7, 2020 21:56
@elizabeth-legros elizabeth-legros requested review from a team, cdxntchou and marctem October 7, 2020 21:57
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.

Things tested:

  1. Confirmed that Property -> Inline and back is the same in Universal for both Linear and Gamma space
  2. Confirmed that Property -> Inline and back is the same in HDRP in Linear space
  3. Confirmed ability to create deprecated node or property with proper option enabled in preferences
  4. Confirmed node updating works for both inline and property with "show deprecated" on
  5. Confirmed "hover for info" tooltip works
  6. Confirmed node updating works for both inline and property with "show deprecated" off
  7. Updated node from before PR and confirmed color property and inline become deprecated and can be upgraded

Repro steps for bug

  1. Gamma space, universal RP, new lit shadergraph, show deprecated nodes on. Make sure you have a scene with HDR color enabled, post processing enabled, and a volume with bloom and intensity set to 1
  2. Create a deprecated color node with HDR and intensity above 0, click it and upgrade it in the graph inspector
  3. Plug it into emission
  4. Save the shadergraph, put it on material on a sphere in your scene
  5. Convert the color property node to inline
  6. Look at the change in bloom level on object in scene

Alternative to step 5: instead of converting it to inline, duplicate it and plug the duplicated property into emission

This bug reproduces on Master which means it was a problem with the original PR. Will await further instruction on what to do about it.

@ghost ghost self-requested a review October 14, 2020 19:53
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.

Pretty sure the bug that's happening has nothing to do with this PR. In practice the result of the bug is an HDR color that has a specific color in the preview and in the shadergraph but is black in the material. I can't figure out how to consistently reproduce this bug but I did reproduce it once on master. I don't think it's caused by this PR so I'm going to approve.

@marctem marctem merged commit e17508f into 9.x.x/release Oct 19, 2020
@marctem marctem deleted the 9.x.x/sg/deprecate-color-property branch October 19, 2020 21:11
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.

3 participants