Skip to content

Conversation

remi-chapelain
Copy link
Contributor

@remi-chapelain remi-chapelain commented Oct 14, 2021

Purpose of this PR

Fix for this one. (forum thread)

  • Few month ago, made this PR to fix an LOD renderer issue with raytracing when building RTAS. LOD wasn't supported back then but we were still looping through higher level LOD to keep a reference so they weren't taken into account when processing the rest of the renderer.
  • In the meantime, support for LOD in raytracing was added and now we loop through every LOD and add them to the RTAS.
  • The problem is: If your setup is bad and there's missing renderer in your LOD group, the RayTracingManager still tries to add the renderer and fail. Which makes the render fail => black screen.

This PR skips adding the renderer to the RTAS if it's null which can happen in large projects.
The only "bad things" is that the fail is silent and ray traced effects won't be applied on the LOD level if it does not have a renderer.
The Check for RayTracing issue menu item still shows which gameobject has issues if you want to fix it.

Gif shows that now we can have missing renderer and still have a render. The RTAO will show only on the third level lod which is the only one that has a renderer. Still a bad setup but no errors and we keep the rendering working.
0bb42412227a530b2714336022639ecc

Check scene content for raytracing setup issues
image


Testing status

  • Checked the repro case ✔️
  • Checked that the RT effect were not applied if renderer was not filled in one of the LOD Level ✔️
  • Launch DXR tests locally ✔️

Comments to reviewers

N/A

@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://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/jobDefinition/.yamato%2Fall-hdrp.yml%23HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_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.

@github-actions github-actions bot added the HDRP label Oct 14, 2021
@remi-chapelain remi-chapelain changed the title skip adding renderer if null [HDRP][DXR] Skip adding higher level Renderer to RTAS if null Oct 14, 2021
@remi-chapelain remi-chapelain marked this pull request as ready for review October 14, 2021 09:23
Copy link
Contributor

@TomasKiniulis TomasKiniulis 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, testing is sufficient.

@anisunity anisunity closed this Oct 14, 2021
@anisunity
Copy link
Contributor

That code will soon be gone (removed by this PR #5988) so we shouldn't be changing it

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