Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HDRP][DXR] Remove Custom Interpolators from SpeedTree8 Graph to support raytracing -bis #6397

Merged
merged 1 commit into from Nov 25, 2021

Conversation

sebastienlagarde
Copy link
Collaborator

@sebastienlagarde sebastienlagarde commented Nov 25, 2021

Previous PR was having a corrupt SG, not sure why, redoing it
#6370

Purpose of this PR

PR fixes the issue of black tree in the pathtracer. See GIF
b87541d8652dcc6055d3f38bd0b7c02e

The issue comes from the fact that the graph uses custom intepolators to pass data (normals, tangeants, bitangenats) through the vertex stage to the fragment stage. Custom intepolators are not supported in raytracing so it fails. Atfer talking with multiple people, no one saw the addition of using custom interpolators so they have been removed to support raytracing by default.

Here's the old and new graph.
Before
Before_Fix

After
image


Testing status

  • Added multiple trees in a scene and now they are rendering fine with the pathtracer. ✔️
  • Did the same with Ray Traced Reflection. ✔️

Comments to reviewers

  • They are still some more issue related to LOD with pathtracer with this graph but this is beyond the scope of this PR and it will be discussed after
  • It's not supposed to change anything on the rasterization side. (We have a test in the HDRP_Runtime test with tree (005) so issue will be caught there if anything. (when yamato will be back up)

@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%23PR_HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%2Fall-lightmapping.yml%23PR_Lightmapping_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 Nov 25, 2021
@sebastienlagarde sebastienlagarde marked this pull request as ready for review November 25, 2021 17:41
@sebastienlagarde sebastienlagarde merged commit 7766f14 into master Nov 25, 2021
@sebastienlagarde sebastienlagarde deleted the HDRP/fix-speedtree-for-raytracing-bis branch November 25, 2021 17:46
@sebastienlagarde sebastienlagarde restored the HDRP/fix-speedtree-for-raytracing-bis branch November 25, 2021 18:31
@sebastienlagarde
Copy link
Collaborator Author

Revert this PR again. There is a big change in rasterizer visualization, so we need to evaluate the impact before merging . cc @remi-chapelain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant