Skip to content

Conversation

@pbbastian
Copy link
Contributor

Purpose of this PR

Previously, URP would force rendering to go through an intermediate renderer if the Renderer had any Renderer Features active. On some platforms, this has significant performance implications. Due to that, Renderer Features are now expected to declare their inputs using ScriptableRenderPass.ConfigureInput. This information is used to decide automatically whether rendering via an intermediate texture is necessary.

For compatibility reasons, a new property Intermediate Texture has been added to the Universal Renderer. This allows for either using the new behaviour, or to force the use of an intermediate texture. The latter should only be used if a Renderer Feature does not declare its inputs using ScriptableRenderPass.ConfigureInput.

All existing Universal Renderer assets that were using any Renderer Features (excluding those included with URP) are upgraded to force the use of an intermediate texture, such that existing setups will continue to work correctly. Any newly created Universal Renderer assets will default to the new behaviour.

Screenshot 2021-08-04 at 15 23 49


Testing status

Verified the behaviour of the option and the asset upgrades locally.


Comments to reviewers

Please do test the upgrading, and see if it acts as expected.

@pbbastian pbbastian requested review from a team, TheoWong-pixel and oleks-k August 4, 2021 13:30
@github-actions
Copy link

github-actions bot commented Aug 4, 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_2021.2

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.

@TheoWong-pixel
Copy link

TheoWong-pixel commented Aug 4, 2021

Would suggest that we leave out the explanation about Always for the docs. we normally just have a generalised description of the feature and what it does, maybe speak more to that and explain the performance implications more, without referencing the options.

Copy link
Contributor

@thomas-zeng thomas-zeng left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@thomas-zeng
Copy link
Contributor

We will need this PR for 2021.2(requested by msft hololens). The branch off for 2021.2 is scheduled for Sep 1st and it's likely the PR will land afterward. So we will need to backport this PR to 2021.2 branch once it lands.

@pbbastian pbbastian marked this pull request as ready for review September 1, 2021 11:37
Copy link

@ernestasKupciunas ernestasKupciunas left a comment

Choose a reason for hiding this comment

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

Tested the upgradability, works as expected.
Also ran Boat Attack and URP Rendering Examples builds in the Editor and MacStandalone build with Always and When Needed, seems to work as intended too.

Device under testing:
MacBook Pro (15-inch, 2017), 2,8 GHz Quad-Core Intel Core i7, 16 GB 2133 MHz LPDDR3, Radeon Pro 555 2 GB, macOS Big Sur 11.5.1 (20G80)

Unity used for testing:
Version: 2022.1.0a8.1153 Personal
Revision: trunk a6d7f27874c0
Built: Sat, 04 Sep 2021 19:12:24 GMT

Good job with the PR description and attached images!

{
public enum IntermediateTextureMode
{
WhenNeeded,
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: maybe use Auto as that's a pattern we use already in other properties.

# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md
#	com.unity.render-pipelines.universal/Documentation~/upgrade-guide-12-0-x.md
pbbastian added a commit that referenced this pull request Sep 23, 2021
Copy link
Contributor

@oleks-k oleks-k 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.

@pbbastian pbbastian force-pushed the universal/intermediate-texture-mode branch from 57ce156 to 9783e62 Compare September 24, 2021 09:20
@pbbastian
Copy link
Contributor Author

I've gone through all the failing jobs, and verified that they're failing in the same way on master.

@pbbastian pbbastian merged commit f1f4b3a into master Sep 24, 2021
@pbbastian pbbastian deleted the universal/intermediate-texture-mode branch September 24, 2021 13:10
pbbastian added a commit that referenced this pull request Sep 24, 2021
pbbastian added a commit that referenced this pull request Nov 12, 2021
phi-lira pushed a commit that referenced this pull request Dec 9, 2021
pbbastian added a commit that referenced this pull request Jan 11, 2022
This reverts commit 1c6e049.
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.

8 participants