Skip to content

Conversation

ellioman
Copy link
Contributor

@ellioman ellioman commented Sep 9, 2021

Purpose of this PR

In #4242 we introduced a setting to control the updates URP does to the Volume Framework.
There was however an issue when switching between EveryFrame and ViaScripting in the URP Asset after moving the camera. The issue is that the camera would use an old stack that had been saved earlier instead of an updated one right before the switch to ViaScripting. The reason for this was that the volumeStack is only updated if it was null or when the setting is changed on the camera.

The solution is to destroy the volumeStack if the Volume Update Setting is set to EveryFrame. That guarantees that the volumeStack will be created and updated when the switch between EveryFrame and ViaScripting happens. This PR also also makes sure we do not update the stack when switching between ViaScripting and UsePipeline on the camera when the URP Asset is set to ViaScripting.

Testing

What has been tested

  • Tested in the Editor (Mac) with three different cameras and two volumes

Yamato

https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/universal%252Fvolume-update-fix

@github-actions
Copy link

github-actions bot commented Sep 9, 2021

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)

URP
/.yamato%252Fall-urp.yml%2523PR_URP_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
Contributor

@Verasl Verasl left a comment

Choose a reason for hiding this comment

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

LGTM simple path that was overlooked, this solves the issue, we might need to revisit a lot of this when we have time to re-do the volume system.

Copy link
Contributor

@erikabar erikabar left a comment

Choose a reason for hiding this comment

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

Tested with URP-validation-project 07-VolumeFramework scene. Both Editor and Mac Standalone build are working as expected.

@ellioman
Copy link
Contributor Author

Failures on Yamato not related to this PR.

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