[HDRP][Path Tracing] Added proper support for interleaved tiling

[eturquin]

Modified camera ray generation slightly to accomodate interleaved tiling, following an internal request from Verticals.

[github-actions]

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]

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page).
See the PR template for more information.
Thank you!

[eturquin]

Btw, @patrickp-unity3d: I'd be tempted to include the pixel offset directly into the code, so that you don't have to shift the viewproj matrix for each tile, the way you do in raster. This way, it'd actually become a ready-to-use solution for anyone to render ultra-high res images, where you just render the exact same image several times, only changing the path-tracer tiling parameters. And we could add support for it in the recorder too (@pmavridis). Would it be a huge deal to add a condition in your code, so that the matrix modifications would only be applied in raster mode?

[patrickp-unity3d]

Btw, @patrickp-unity3d: I'd be tempted to include the pixel offset directly into the code, so that you don't have to shift the viewproj matrix for each tile, the way you do in raster. This way, it'd actually become a ready-to-use solution for anyone to render ultra-high res images, where you just render the exact same image several times, only changing the path-tracer tiling parameters. And we could add support for it in the recorder too (@pmavridis). Would it be a huge deal to add a condition in your code, so that the matrix modifications would only be applied in raster mode?

I think as long as we keep tileindex and tilecount as our agreed interface, and as long as I can still offset the viewproj in raster, I I can have a condition in the code to fork between both logic. I already have a LOT of branching for all the various HDRP features that we're trying to support under a single roof. ;)

I honestly prefer what is best for HDRP. And if this improves HDRP, I'm happy we're contributing.

If anyone wants my output pixel reordering shader that we run after rendering let me know.

[eturquin]

I think as long as we keep tileindex and tilecount as our agreed interface, and as long as I can still offset the viewproj in raster, I I can have a condition in the code to fork between both logic. I already have a LOT of branching for all the various HDRP features that we're trying to support under a single roof. ;)

Cool, I'll add that then, so that we have a self-contained solution. :)

If anyone wants my output pixel reordering shader that we run after rendering let me know.

We may! @sebastienlagarde, @pmavridis, what do you guys think?

[patrickp-unity3d]

I think as long as we keep tileindex and tilecount as our agreed interface, and as long as I can still offset the viewproj in raster, I I can have a condition in the code to fork between both logic. I already have a LOT of branching for all the various HDRP features that we're trying to support under a single roof. ;)

Cool, I'll add that then, so that we have a self-contained solution. :)

If anyone wants my output pixel reordering shader that we run after rendering let me know.

We may! @sebastienlagarde, @pmavridis, what do you guys think?

It's just a simple blit material that undoes the interleaving: https://github.com/Unity-Technologies/RenderStudio-Dev/blob/master/Packages/com.unity.industrial.renderstudio/Runtime/Resources/unity.industrial.renderstudio/Graphics/Untiling.shader

            // Unity builtin, size of the given texture
            float4 _MainTex_TexelSize;

            // source texture
            Texture2D _MainTex;
            SamplerState my_point_clamp_sampler;

            float _TileCount;

            float4 Frag(Varyings input) : SV_Target
            {
                float2 texcoord = input.texcoord.xy;

                uint tileCount = _TileCount;
                int pixelX = texcoord.x * _MainTex_TexelSize.z;
                int pixelY = texcoord.y * _MainTex_TexelSize.w;
                int tileIndexX = tileCount - 1 - (pixelX - (pixelX / tileCount) * tileCount);
                int tileIndexY = tileCount - 1 - (pixelY - (pixelY / tileCount) * tileCount);
                int tileWidth = _MainTex_TexelSize.z / tileCount;
                int tileHeight = _MainTex_TexelSize.w / tileCount;

                float2 origin =
                    float2(tileIndexX * tileWidth + (pixelX + 0.5) / _TileCount,
                           tileIndexY * tileHeight + (pixelY + 0.5) / _TileCount)
                    / float2(_MainTex_TexelSize.z, _MainTex_TexelSize.w);
                return _MainTex.Sample(my_point_clamp_sampler, origin);
            }

[patrickp-unity3d]

Actually, @eturquin I think I must use the viewproj matrix because many shaders depend on the screen space position. Chromatic aberration as well as the street in the above image need it to be in full resolution. If I don't offset the viewproj matrix these will not use the correct pixel. Unless I am very wrong, I think we either need to do as discussed originally or permit both strategies.

[eturquin]

Actually, @eturquin I think I must use the viewproj matrix because many shaders depend on the screen space position. Chromatic aberration as well as the street in the above image need it to be in full resolution. If I don't offset the viewproj matrix these will not use the correct pixel. Unless I am very wrong, I think we either need to do as discussed originally or permit both strategies.

Good point, ok I'll revert the latest change, and let the matrix do its work. :)

[pmavridis]

This change is small and as long as it solves the issues that the verticals team found, I think we should merge it. DoF sampling might require a small change, as noted below.

[patrickp-unity3d]

Looks good, I tested depth of field as well (32 samples only)

Here's some detail at the transition (screengrab via Parsec at 150%)

some detail of the nice expected noise pattern (screengrab via Parsec at 200%)

[eturquin]

Looks good, I tested depth of field as well
Here's some detail at the transition (screengrab via Parsec at 150%)

This is the kind of 8x8 grid-aligned artefacts that I would expect with screen-space, post-process DoF (applied on each 8k tile individually). The path-traced DoF should not exhibit such artefacts. Can you confirm which one it was here?

[pmavridis]

screen-space, post-process DoF (applied on each 8k tile individually

@patrickp-unity3d Note that you get path traced DoF only if the focus mode is set to "PhysicalCamera". If it's set to "Manual Ranges" then you get the same post-process DoF as rasterization because the path tracer can only compute physically-based DoF.

[patrickp-unity3d]

screen-space, post-process DoF (applied on each 8k tile individually

@patrickp-unity3d Note that you get path traced DoF only if the focus mode is set to "PhysicalCamera". If it's set to "Manual Ranges" then you get the same post-process DoF as rasterization because the path tracer can only compute physically-based DoF.

Correct I used manual ranges since our users generally have a specific 'hero' framing in mind (I don't mind the tiling but you're correct that this isn't the test we wanted)

This is with physical camera and a 4.6m focus distance (should be the test we want) (looks much more like path tracing than post-processing ;) )

(32 samples only)

Out of focus tire vs (mostly) in focus tire (screengrab via Parsec at 150%):

[eturquin]

This is with physical camera and a 4.6m focus distance (should be the test we want) (looks much more like path tracing than post-processing ;) )

Looking good! I say, ship it! ;)

[patrickp-unity3d]

Hi! So if I understand correctly this will be in HDRP 12.2 and 13.2?

[eturquin]

Hi! So if I understand correctly this will be in HDRP 12.2 and 13.2?

Yes, in the first patch for 12.2.