HDR Cubemap Encoding support

[pema99]

Purpose of this PR

This trunk PR: https://ono.unity3d.com/unity/unity/pull-request/133667/_/lighting/reflection-probe-encoding3 adds a new setting (re)named "HDR Cubemap Encoding" which controls the (manual) encoding of HDR cubemaps. Since HDRP currently expects no manual encoding (native HDR format), I had to add support for these encoded cubemap to the relevant shaders, and make sure that HDR cubemaps are decoded before putting in the probe cache. Most of the time, that decoding will essentially be a no-op since a majority will be using the default of native HDR.

The 3 shaders I touched are:

• The skybox shader used for HDRI sky
• The shader used for probe preview gizmo
• The shader used for blitting a cubemap face into the probe cache

Here is a video showing the setting working correctly in an HDRP project: https://i.imgur.com/zX4Up3h.mp4
In the same shot we see an HDRI sky, reflection probe gizmo and renderer with reflective material using an encoded HDR cubemap.

---

Testing status

Automated testing has been added in the trunk PR. QA will verify HDRP functionality once that PR lands. I don't think automated testing for the HDRP-specific part of this would be very straight forward or warranted, since a project setting is involved, and the changes are fairly small. Let me know if you think otherwise while reviewing. The common code path should be sufficiently covered by existing tests.

---

Comments to reviewers

A few things are worth noting here:

• The <texturename>_HDR uniforms are already exposed to shaders, no extra code was needed. This is core unity functionality. The decoding function is also very similar to the one used in BiRP.
• RGBM and dLDR only support a fairly limited range of HDR values. Thus, they will sometimes not fit some of the comparatively very large values used in HDRP. This caveat isn't really specific to HDRP. In general, if you use a manual encoding format and try to put very large values in it, they'll just be clipped.

Also, I'm not very familiar with the HDRP codebase, so please feel free to point out if something is off here. I spoke to @sebastienlagarde who told me that it should be fine, if I provided a minimal working changeset.

[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.

[sebastienlagarde]

@RSlysz I added you just for awareness. Pema have added control on reflection probe quality like what is done for lightmap. But for HDRP we always want to use high quality, exactly like for lightmap, so we will need to plan to add to HDRP wizard a check that reflection probe quality is indeed high quality (like we do for lightmap) once this PR lands, thanks

[pema99]

@sebastienlagarde If you want more elaboration on the whole _TextureName_HDR uniform thing, I can point you to some code in trunk. I don't think there is any issue here.

[JulienIgnace-Unity]

When is this setting applied? During importing of textures?
We completely overrode the reflection probe baking system in HDRP so I want to make sure we are not missing something.
Also what about realtime HDR reflection probes that we generate at runtime? Those will not have any encoding regardless of the reflection probe quality setting but they will still be used alongside encoded texture. I expect that using realtime reflection with dLDR for example will not work.

[pema99]

@JulienIgnace-Unity indeed, this setting is applied in the importer, just as with Lightmap Encoding. If you never import the texture, the setting won't affect you. So if the realtime reflection cubemaps only exist in memory at runtime, they shouldn't be affected, as you have pointed out.

For affected textures, when a texture has no manual encoding (RGBM, dLDR), the decoding code in the shaders will essentially be a no-op - the _HDR uniforms are filled in automatically based on how the texture was (manually) encoded, for which one option is no manual encoding at all.

[pema99]

To be completely clear, the setting only affects textures which are:

• Actual files on disk
• HDR
• Cubemaps

[sebastienlagarde]

@pema99 ok, the PR looks fine but I still don't understand who setup the _InputTex_HDR in the case of cache cubemap. The ConvertTexture is a SRP is implemented like this:

    void ConvertTexture(CommandBuffer cmd, Texture input, RenderTexture target)
    {
        m_ConvertTextureMPB.SetTexture(HDShaderIDs._InputTex, input);
        m_ConvertTextureMPB.SetFloat(HDShaderIDs._LoD, 0.0f); // We want to convert mip 0 to whatever the size of the destination cache is.
        for (int f = 0; f < 6; ++f)
        {
            m_ConvertTextureMPB.SetFloat(HDShaderIDs._FaceIndex, (float)f);
            CoreUtils.SetRenderTarget(cmd, target, ClearFlag.None, Color.black, 0, (CubemapFace)f);
            CoreUtils.DrawFullScreen(cmd, m_ConvertTextureMaterial, m_ConvertTextureMPB);
        }
    }
    
    nobody setup _InputTex_HDR. So I expect it is some kind of global setup ? but when does it happen? and in this case if it is setup permanently it will not works with our real time cubemap as it will not be override.

[sebastienlagarde]

oh sorry I missed your comment: "The _HDR uniforms are set automatically for any corresponding texture uniform. This is core Unity functionality. These are based only the 'texture usage' (which for lightmap can only be None, dLDR or RGBM, where None means either native HDR or not an HDR texture). Basically synonymous with the Lightmap Encoding and HDR Cubemap Encoding settings. In cases where the the texture is native HDR or not HDR (None), the uniform will be set such that the HDR decode code does nothing to the value read.".

[sebastienlagarde]

triggered a nightly build on Yamato with the custom revision of C++ PR: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/lighting%252Fhdr-cubemap-encoding/.yamato%252Fall-hdrp.yml%2523Nightly_HDRP_CUSTOM-REVISION/9448040/job/pipeline

[sebastienlagarde]

Yamato catch an error in 5009_HDRI_Sky_Flow:
https://yamato-artifactviewer.prd.cds.internal.unity3d.com/e7e6b0d5-33a6-41f8-8d27-d5bd66ef461c%2Flogs%2FTestProjects%2FHDRP_Tests%2Ftest-results/TestReport.html

Unhandled log message: '[Error] Shader error in 'Hidden/HDRP/Sky/HDRISky': 'DecodeHDREnvironment': cannot implicitly convert from 'float3' to 'float4' at line 210 (on d3d11)

(note: other error Speedtree (1205) and the skin (9601/9602) have issue due to cache server and aren't related to your PR
also failure on 007-BasicAPV.png are expected (guess you haven't merge master)

[pema99]

Yamato catch an error in 5009_HDRI_Sky_Flow: https://yamato-artifactviewer.prd.cds.internal.unity3d.com/e7e6b0d5-33a6-41f8-8d27-d5bd66ef461c%2Flogs%2FTestProjects%2FHDRP_Tests%2Ftest-results/TestReport.html

Unhandled log message: '[Error] Shader error in 'Hidden/HDRP/Sky/HDRISky': 'DecodeHDREnvironment': cannot implicitly convert from 'float3' to 'float4' at line 210 (on d3d11)

(note: other error Speedtree (1205) and the skin (9601/9602) have issue due to cache server and aren't related to your PR also failure on 007-BasicAPV.png are expected (guess you haven't merge master)

Wow, really strange that the shader compiler didn't error for me locally. I see the issue, will fix. About the other failures, not sure what I can do here. Should I merge Graphics master again? The branch was based on a fairly recent commit.

[sebastienlagarde]

yep, can you merge master and retrigger yamato with custom revision. I think the PR is good to go now just wait for C++ Pr to land and for a QA pass.

[JMargevics]

Checked on HDRP side. Things tested:

• migrating a template project
• rebaking lightmaps
• Importing heavy .hdr and .exr
• HDRI ambient light both in static and dynamic modes
• LookDev

No issues or regressions found ✔👍

I'd only emphasize that it's important to give a warning message for users if they switch to Medium or Low settings on HDRP.

[pema99]

Trunk PR landed. Will try to kick off tests with newest revision tomorrow, I've been having a lot of issues with random unrelated failures.