Invalid sampling data - LOD to sample from was unspecified.

[controlaware]

Describe the bug GPUReadbackDisps.cs :: ComputeUndisplacedPosition(...)

Using Dynamic Water Physics 2, the DWP collision provider queries the Crest Ocean collision provider for information. It seems like when the LOD changes, the PerLodData tag is not filled in. This means that the function mentioned above throws null-reference errors and the collision does not work anymore. DWP doesn't get the height collision and the object falls through the ocean.

Log GPUReadbackDisps.cs, NullReferenceException thrown on line:

for (int i = 0; i < 4 && lodData._resultData.InterpolateARGB16(ref guess, out disp); i++)

Debug log shows the Debug.Assert text found at the beginning of the method. Call stack is:

DWP2.CrestWaterDataProvider:GetWaterHeight Crest.CollProviderCache:SampleHeight Crest.GPUReadbackDisps:SampleHeight Crest.GPUReadbackDisps:ComputeUndisplacedPosition

Adding a check under the assert removes the NullReferenceException error:

`
Debug.Assert(i_samplingData._tag != null, "Invalid sampling data - LOD to sample from was unspecified.");

        var lodData = i_samplingData._tag as PerLodData;

        if (lodData == null)
        {
            undisplacedWorldPos = i_worldPos;
            return false;
        }`

But obviously the Debug.Assert is still there, which spams the Debug Log window (999+ in a second or two).

Versions Built-in render pipeline, Unity 2019.2.9f1, Crest-8.0.0 unity package.

To Reproduce Create Crest ocean according to quick-start guide. From DWP2, drop in the "TorpedoBoat" prefab. Add a constant relative force of -320000 in the Z direction. Attach a movable camera script and move camera as boat moves panning out then back in. After the boat moves from one LOD to another, the boat will fall through the ocean.

Platform

• Editor
• Windows 10 Pro 1903

Hardware AMD Ryzen 7 2700X, 32GB RAM, AMD Radeon RX580 8GB software version 19.9.2, not overclocked.

[huwb]

Hi,

Sorry you ran into this. I've seen scattered reports of this, but never got a good repro.

On the latest version, we changed out this system for a new one which is faster and simpler and completely sidesteps this issue. In the future we will completely remove the displacement readback system.

If you're comfortable to could you try latest from master, either cloning using git or using the green 'download' button? Otherwise i can make a new release, probably on the weekend.

[controlaware]

I'll try pulling down the repo. Is this a breaking change or does the API remain the same? Hopefully I wouldn't have to re-write the DWP2 water collision system. If it is the same interface then it should be drop-in-and-go and I'll give it a try.

Thanks!

[huwb]

The API changes - theres a new Query() function and it takes an array of points to query the heights. This allows queries to be batched, and brings the system in line with other query backends like burst jobs which require data in batches.

What is the interface with DWP2 - do you have an array of positions to sample?

If so, the new change simplifies things. If you only have one position at a time, and can't easily batch them, then let me know.

On Wed, 30 Oct 2019 at 12:43, ControlAware LLC notifications@github.com wrote:

I'll try pulling down the repo. Is this a breaking change or does the API remain the same? Hopefully I wouldn't have to re-write the DWP2 water collision system. If it is the same interface then it should be drop-in-and-go and I'll give it a try.

Thanks!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crest-ocean/crest/issues/358?email_source=notifications&email_token=AAHFO7I3BP5JY5RMHD2IOMDQRF6QFA5CNFSM4JGS6QH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECUASMA#issuecomment-547883312, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHFO7P7WTPC2NG4XHN77JTQRF6QFANCNFSM4JGS6QHQ .

[controlaware]

The DWP queries the heights like this:

        public override float GetWaterHeight(Vector3 pos)
        {
            if (!_initialized) return 0;

            float r = 0.2f;
            rect = new Rect(new Vector2(pos.x - r, pos.z - r), 2f * r * Vector2.one);
            collProvider.GetSamplingData(ref rect, 2f * r, samplingData);        
            if (OceanRenderer.Instance.CollisionProvider.SampleHeight(ref pos, samplingData, out height))
            {
                return height;
            }
            return OceanRenderer.Instance.SeaLevel;
        }

Without digging into the third-party code, I'm not sure what generates the pos that it is looking for. I would assume that DWP could benefit from passing in the position of the normals on the underwater portion of the boat, but I don't know what kind of (major) change that would be in the DWP code.

[huwb]

Hm ok, thats tricky. I mailed the authors of DWP to explain why we're moving to batched queries and to get their thoughts. I'll also keep thinking about it. I'll post back with updates.

[huwb]

I haven't heard from the authors yet.

I made an attempt at an adaptor - an object that can act as a proxy between one-at-a-time queries and the underlying system that works with batches. #362 has the changes and instructions on its use. basically, one can create the adaptor object wherever the DWP queries are getting handled and call GetWaterHeight on the adaptor, which will proxy the queries through to the query system.

Could you let me know what you think?

[controlaware]

Thank you, I'll give it a try tomorrow and let you know.

[huwb]

@furkandinc kindly tracked down the issue and we merged in his fix in #366

In the (near) future the DWP2 authors will connect directly to the compute-height-queries. The adaptor I made in #362 is no longer necessary.