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

Fix ray tracing with XR and camera relative rendering #4666

Merged
merged 6 commits into from May 31, 2021

Conversation

fabien-unity
Copy link
Contributor


Purpose of this PR

Fix XR multi-pass and single-pass modes with ray tracing and camera relative rendering
Fix for https://fogbugz.unity3d.com/f/cases/1336608


Testing status

Tested locally using template scene + ray-traced reflections and AO


Comments to reviewers

I also tested the repro project from the ticket with 2020.3 to confirm it works, but the shaders have changed a bit and it will not be a straight backport.

@anisunity
Copy link
Contributor

looks fine to me except for some indentation issues

@fabien-unity fabien-unity marked this pull request as ready for review May 26, 2021 18:15
@m0nsky
Copy link

m0nsky commented May 26, 2021

Hi, sorry to bother. I've tested this locally and can confirm this fixes the issue, but you've forgotten to apply the same changes to RaytracingShadow.raytrace, after that it should be perfect. (Just did it manually and can confirm it will fix the RT shadows in XR)

Thanks for fixing my bugreport so quick, I appreciate it.

@fabien-unity
Copy link
Contributor Author

@m0nsky ah good point! Thanks for reporting and following up. I'll add the code to the shadows tomorrow after some more testing.

@@ -116,6 +116,9 @@ void TraceGBuffer(PixelCoordinates coords)
rayDescriptor.Direction = direction.xyz;
rayDescriptor.TMin = 0.0;
rayDescriptor.TMax = _RaytracingRayMaxLength;

// Adjust world-space position to match the RAS setup with XR single-pass and camera relative
ApplyCameraRelativeXR(rayDescriptor.Origin);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@anisunity / @fabien-unity by doing call like this we will always make mistake with copy/paste

As far as I can see the code is always the same. We should do a function and call it.

RayDesc rayDescriptor;
InitRayDesc(rayDescriptor, posInput.positionWS, normalData.normalWS, direction);

with

InitRayDesc(RayDesc rayDescriptor, ...)
{
rayDescriptor.Origin = ApplyCameraRelativeXR(posInput.positionWS + normalData.normalWS * _RaytracingRayBias);
rayDescriptor.Direction = direction.xyz;
rayDescriptor.TMin = 0.0;
rayDescriptor.TMax = _RaytracingRayMaxLength;
}

Thought?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that'll be ideal. I can refactor the code, unless @anisunity wants to do it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

i'll Do it. Does it have to be in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm ok with the refactor being done after this PR lands.

Copy link
Collaborator

Choose a reason for hiding this comment

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

alright, landing this PR then :)

@sebastienlagarde sebastienlagarde merged commit 372a801 into master May 31, 2021
@sebastienlagarde sebastienlagarde deleted the HDRP/xr/fix-ray-tracing-camera-relative branch May 31, 2021 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants