Skip to content

Commit

Permalink
UUM-74682: Fix LightBaking not working properly on AMD
Browse files Browse the repository at this point in the history
See [UUM-74682](https://jira.unity3d.com/browse/UUM-74682)
See slack conversation [here](https://unity.slack.com/archives/C05T20SDVNH/p1724775737786409) (it is private so please ask a lighting dev to add you if you want to read it).

Several probe baking tests are consistently failing on AMD, they work fine on NVidia. It turns out that AMD is sensitive to the stride in the `GraphicsBuffers` that are bound to a compute shader. If you use a stride that does not match the access pattern of the structured buffer in HLSL (say `float` or `float3`) the buffer access will be incorrect. 
The solution was to change `IDeviceContext.CreateBuffer` API to take stride as an input. The API was `IDeviceContext.CreateBuffer(ulong size)` before and now it is `IDeviceContext.CreateBuffer(ulong count, ulong stride)`.

This is a breaking change to an API that was also introduced in Unity6, but without it we will have functionality that does not work on AMD. We found this issue recently as we have added a local Yamato runner in Copenhagen that runs baking tests on a machine with an AMD GPU (these do not exist in our Yamato test farm). 

This fixes:
* [UUM-74682](https://jira.unity3d.com/browse/UUM-74682)
* [UUM-66562](https://jira.unity3d.com/browse/UUM-66562)
* [UUM-73375](https://jira.unity3d.com/browse/UUM-73375)
* [GFXFEAT-573](https://jira.unity3d.com/browse/GFXFEAT-573)

Also fixed the `PostProcessingGPU` which were incorrectly handling the lifetime of the `IDeviceContext` causing the test to fail on a second run.

Finally fixed three cases where buffers created with `IDeviceContext` were not disposed of correctly. When this happens with the OpenCL context the test itself and any subsequently ran test can become unstable.
  • Loading branch information
pigselated authored and Evergreen committed Sep 6, 2024
1 parent 97e9cf1 commit d7493d7
Showing 1 changed file with 10 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -367,29 +367,28 @@ public static BakeContext New(InputExtraction.BakeInput input, NativeArray<Vecto
private void CreateBuffers(int probeCount)
{
// Allocate shared position and light index buffer for all jobs
var positionsBytes = (ulong)(sizeOfFloat * 3 * probeCount);
positionsBufferID = ctx.CreateBuffer(positionsBytes);
positionsBufferID = ctx.CreateBuffer((ulong)probeCount, (ulong)(3 * sizeOfFloat));

int batchSize = Mathf.Min(k_MaxProbeCountPerBatch, probeCount);
var shBytes = (ulong)(sizeSHL2RGB * batchSize);
var validityBytes = (ulong)(sizeOfFloat * batchSize);

directRadianceBufferId = ctx.CreateBuffer(shBytes);
indirectRadianceBufferId = ctx.CreateBuffer(shBytes);
validityBufferId = ctx.CreateBuffer(validityBytes);
directRadianceBufferId = ctx.CreateBuffer((ulong)(batchSize * SHL2RGBElements), (ulong)sizeOfFloat);
indirectRadianceBufferId = ctx.CreateBuffer((ulong)(batchSize * SHL2RGBElements), (ulong)sizeOfFloat);
validityBufferId = ctx.CreateBuffer((ulong)batchSize, (ulong)sizeOfFloat);

windowedDirectSHBufferId = ctx.CreateBuffer(shBytes);
boostedIndirectSHBufferId = ctx.CreateBuffer(shBytes);
combinedSHBufferId = ctx.CreateBuffer(shBytes);
irradianceBufferId = ctx.CreateBuffer(shBytes);
windowedDirectSHBufferId = ctx.CreateBuffer((ulong)(batchSize * SHL2RGBElements), (ulong)sizeOfFloat);
boostedIndirectSHBufferId = ctx.CreateBuffer((ulong)(batchSize * SHL2RGBElements), (ulong)sizeOfFloat);
combinedSHBufferId = ctx.CreateBuffer((ulong)(batchSize * SHL2RGBElements), (ulong)sizeOfFloat);
irradianceBufferId = ctx.CreateBuffer((ulong)(batchSize * SHL2RGBElements), (ulong)sizeOfFloat);

if (bakeProbeOcclusion)
{
var lightIndicesBytes = (ulong)(sizeOfFloat * maxOcclusionLightsPerProbe * probeCount);
perProbeLightIndicesId = ctx.CreateBuffer(lightIndicesBytes);
perProbeLightIndicesId = ctx.CreateBuffer((ulong)(maxOcclusionLightsPerProbe * probeCount), (ulong)sizeOfFloat);

var occlusionBytes = (ulong)(sizeOfFloat * maxOcclusionLightsPerProbe * batchSize);
occlusionBufferId = ctx.CreateBuffer(occlusionBytes);
occlusionBufferId = ctx.CreateBuffer((ulong)(maxOcclusionLightsPerProbe * batchSize), (ulong)sizeOfFloat);
}

allocatedBuffers = true;
Expand Down

0 comments on commit d7493d7

Please sign in to comment.