Skip to content

Conversation

PaulDemeulenaere
Copy link
Contributor

@PaulDemeulenaere PaulDemeulenaere commented Aug 19, 2021


Purpose of this PR


Testing status

I pushed the debug scene.
Before
image

After
image

However, I can't use this scene for graphicTest because the framework doesn't correctly save the result when there is an overlay :
image
⚠️ The overlay fix has been reverted and will be independently fixed. ⚠️

Also, testing manually with renderscale update modification:
_renderscale_update


Comments to reviewers

This PR follows up with several fixes done in this change : #4733
@Unity-Technologies/gfx-qa-vfx This change also impacts newly introduced HDRP decals but, afaik, HDRP is never using rendering without offscreen texture.

@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Aug 23, 2021
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Aug 23, 2021
@PaulDemeulenaere PaulDemeulenaere requested review from a team and ludovic-theobald August 23, 2021 12:33
@VitaVFX VitaVFX requested review from VitaVFX and removed request for a team August 23, 2021 12:44
There is probably something wrong in SetPerCameraShaderVariables
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Sep 27, 2021
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Sep 27, 2021
Copy link
Contributor

@ludovic-theobald ludovic-theobald left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@phi-lira phi-lira requested a review from lukaschod October 6, 2021 14:33
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Oct 11, 2021
@PaulDemeulenaere PaulDemeulenaere requested a review from a team as a code owner October 25, 2021 09:15
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Oct 25, 2021
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Oct 25, 2021
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.

Thank you for the fix, Paul. Couldn't find any outstanding issues related to PR. 🟢
URP: Retested Render Scale, tweaked various camera settings, built standalone
HDRP: Checked Transparent Render Queue, tweaked Dynamic Resolution settings, built standalone (used HDRP decal output for tests)

@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Nov 10, 2021
@PaulDemeulenaere PaulDemeulenaere removed the request for review from lukaschod November 10, 2021 11:15
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Nov 22, 2021
@PaulDemeulenaere
Copy link
Contributor Author

Double check at 19b5a36 the behavior is still valid:
_RenderScale
And also still needed (reverting locally changes in Pass.template):
_RenderScale_fail

@julienf-unity julienf-unity merged commit b49b820 into master Nov 24, 2021
@julienf-unity julienf-unity deleted the vfx/fix/1343674-fix-renderscale branch November 24, 2021 16:14
@Unity-Technologies Unity-Technologies deleted a comment from github-actions bot Dec 8, 2021
PaulDemeulenaere added a commit that referenced this pull request Dec 13, 2021
* Use abstraction to compute screen space uv

The URP implementation relies on screen scale & graphics api
See https://github.com/Unity-Technologies/Graphics/blob/b468fa4627d191f06200a0f33df3d9cddee2c3f9/com.unity.render-pipelines.universal/ShaderLibrary/ShaderVariablesFunctions.hlsl#L347

* *Add Graphics Test

* Experiment : Revert Pass.template from #4601

* Enable GraphicTest & add RenderScale

* Revert "Experiment : Revert Pass.template from #4601"

This reverts commit b8bfebb.

* Fix overlay decal

* *Update test Scene

* *Remove 104_Decal_And_SoftParticle (Graphic Test can't capture correctly overlay when there is a renderScale)

* Remove unexpected "if (_ProjectionParams.x < 0)" & Update ChangeLog

In discussion with @ludovic-theobald about this behavior.

* Restore actually correct _ProjectionParams.x test

There is probably something wrong in SetPerCameraShaderVariables

* Revert change in URP

* Minor: Apply suggestion from @julienf

#5401 (comment)

* Unexpected change in changelog

* Unexpected modification in CHANGELOG.md

* Fix changelog ordering

Co-authored-by: julienf-unity <julienf@unity3D.com>
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md
sebastienlagarde pushed a commit that referenced this pull request Jan 13, 2022
* Use abstraction to compute screen space uv

The URP implementation relies on screen scale & graphics api
See https://github.com/Unity-Technologies/Graphics/blob/b468fa4627d191f06200a0f33df3d9cddee2c3f9/com.unity.render-pipelines.universal/ShaderLibrary/ShaderVariablesFunctions.hlsl#L347

* *Add Graphics Test

* Experiment : Revert Pass.template from #4601

* Enable GraphicTest & add RenderScale

* Revert "Experiment : Revert Pass.template from #4601"

This reverts commit b8bfebb.

* Fix overlay decal

* *Update test Scene

* *Remove 104_Decal_And_SoftParticle (Graphic Test can't capture correctly overlay when there is a renderScale)

* Remove unexpected "if (_ProjectionParams.x < 0)" & Update ChangeLog

In discussion with @ludovic-theobald about this behavior.

* Restore actually correct _ProjectionParams.x test

There is probably something wrong in SetPerCameraShaderVariables

* Revert change in URP

* Minor: Apply suggestion from @julienf

#5401 (comment)

* Unexpected change in changelog

* Unexpected modification in CHANGELOG.md

* Fix changelog ordering

Co-authored-by: julienf-unity <julienf@unity3D.com>
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md
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.

5 participants