Terrain ocean depth - draw instanced

[Draydak]

I just did a pull from today to test the changes to project since I last looked.

I noticed that if you use Unity terrain and have "Draw Instanced" enabled
For some reason it does not get the depth mask correctly, it completely ignores the terrain.

Switch off the "Draw Instanced" and it's all fine.
This is the option in the Terrain settings which arrived with Unity 2018.3

It's something to be aware of, not sure how easy it is to work around as I'm not 100% certain why it's affecting it like this.

[huwb]

Ah thats a shame. I hadnt tried that combination.

I tried just making the OceanDepths.shader support instancing but when drawing the terrain the vert shader ends up outputting a bunch of nans, i guess the terrain needs the proper shader code to draw.

I think maybe doing a RenderWithShader using OceanDepths.shader is the mistake here. Another option would be to trigger the camera to render with disabled colour writes so it populates the depth. This might still compute the fragment shading though (?) and might be expensive. And potentially fidgety to get working.

The proper solution here would be to emulate DepthOnlyPass.cs from SRP - this does exactly the work needed to populate the depth. I think this will probably be my plan.

For the time being, an ugly hack would be to turn on instancing after the depth cache has populated. Not pretty, but maybe this can suffice as a workaround for now until SRP comes online.

[holdingjason]

Ha did not even notice this and yes that hack does work ok. You can flip it back and forth in the editor as well to just see the cpu difference with it on and off. So seems like it is fine to have off until the depth cache is setup.

[superjayman]

Turning on Instancing afterwards makes the terrain disappear in executable mode, in editor it works fine switching (I'm testing in VR).

[holdingjason]

@superjayman Found this. So it appears its because the build does not include the instanced shader for the terrain. I know you can force/add shaders to the build. We did not notice this because we are using Micro Splat with our terrain and it appears to be handling this for us. Doing a stand alone build right now to make sure that is also picking it up.

Update: Nope same issue once I did the build. So need to figure out how to force the inclusion of the shaders or time it so that we flip the instanced off and back on again after the Depth has been calculated.

UPDATE #2: Ok timing it worked fine. Ok in the OceanDepthCache script you can do this.

Just assign the terrain object however you like. I just make the OceanDepthCache a child object of the terrain do do this

        var terrain = GetComponentInParent<Terrain>();

        if (_populateOnStartup)
        {
            if (terrain != null)
                terrain.drawInstanced = false;

            PopulateCache();

            if (terrain != null)
                terrain.drawInstanced = true;
        }

https://forum.unity.com/threads/terrain-setting-drawinstanced-at-runtime.576598/

[superjayman]

Cheers! That works

[huwb]

I had a first swing at this in the LWRP version of crest. In LWRP i should be able to draw the terrain into the depths from code and the above workaround should not be required. I got behaviour i didnt understand when i added a new empty render pass for it, so i bugged it and I'll revisit when the next version of LWRP/unity comes out.

[huwb]

Good news: I have this working in LWRP, package with this fix has been submitted to store

Bad news: I could not get the same fix to work in this github version of crest. It "almost works" in the commit referenced above - it kinda captures the right shape of the island etc, but the depths seem to be offset by a bunch and as a result there is way too much shallow water. There's no errors in the log and I could not work out why this was happening in the limited time i had, so i parked it in the above commit, maybe someone else can figure it out.

[huwb]

The lwrp version is up (version 1.5) and terrain should render depth properly, even with instancing.

For the built-in render pipeline (this version on github), the above workaround is required.

[daleeidd]

This is now fixed for anyone coming across this on an older version of Crest.