Skip to content

Conversation

@PaulDemeulenaere
Copy link
Contributor

@PaulDemeulenaere PaulDemeulenaere commented Sep 24, 2021


Purpose of this PR

Fix regression introduced at #5416
https://fogbugz.unity3d.com/f/cases/1367167/
We shouldn't only list texture declared as input slot but also internal texture listed in textureInfos
It only concerns the new SG integration.


Testing status

  • Tested the repro case from the fogbugz
  • I slightly modified one of our graphicTest to use the two possibilities (without changing the expected rendering)

In URP We use an internal texture declaration (which ends in textureInfos)
image

In HDRP We use an exposed texture (which ends in InputSlot).
image

  • Yamato🟢: The failure aren't related to this PR

Comments to reviewers

N/A

Regression introduced by #5416
We should also include texture internally declared in SG -_-'
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Sep 24, 2021
@PaulDemeulenaere PaulDemeulenaere marked this pull request as ready for review September 24, 2021 15:34
@github-actions
Copy link

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

VFX
/.yamato%252Fall-vfx.yml%2523PR_VFX_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

Copy link

@VitaVFX VitaVFX left a comment

Choose a reason for hiding this comment

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

Looking great, thank you!

Tested:

  1. Gather Texture 2D/3D,Texture 2D Array, Load/Sample Texture 2D Array, Sample/Load Texture 2D/3D, cubemap
  2. Ran FTP
  3. Checked URP and HDRP

@PaulDemeulenaere PaulDemeulenaere merged commit 9c71fab into master Oct 5, 2021
@PaulDemeulenaere PaulDemeulenaere deleted the vfx/fix/1367167-regression-of-texture-redefinition branch October 5, 2021 13:05
PaulDemeulenaere added a commit that referenced this pull request Oct 5, 2021
* Fix case https://fogbugz.unity3d.com/f/cases/1367167/

Regression introduced by #5416
We should also include texture internally declared in SG -_-'

* *Improve coverage using a texture only internally in shaderGraph

It shouldn't modify the image reference

* *Update changelog.md

(cherry picked from commit 9c71fab)

# Conflicts:
#	com.unity.visualeffectgraph/CHANGELOG.md
PaulDemeulenaere added a commit that referenced this pull request Oct 22, 2021
* Fix case https://fogbugz.unity3d.com/f/cases/1367167/

Regression introduced by #5416
We should also include texture internally declared in SG -_-'

* *Improve coverage using a texture only internally in shaderGraph

It shouldn't modify the image reference

* *Update changelog.md

(cherry picked from commit 9c71fab)

# Conflicts:
#	com.unity.visualeffectgraph/CHANGELOG.md
@PaulDemeulenaere
Copy link
Contributor Author

Already backported in #5914

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.

4 participants