Skip to content

[Backport 2021.1] Fix constant buffer being stomped on by shadows during async tasks #4375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 29, 2021
Merged

[Backport 2021.1] Fix constant buffer being stomped on by shadows during async tasks #4375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 132 additions & 70 deletions com.unity.render-pipelines.core/Runtime/Common/ConstantBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class ConstantBuffer
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public static void PushGlobal<CBType>(CommandBuffer cmd, in CBType data, int shaderId) where CBType : struct
{
var cb = TypedConstantBuffer<CBType>.instance;
var cb = ConstantBufferSingleton<CBType>.instance;

cb.UpdateData(cmd, data);
cb.SetGlobal(cmd, shaderId);
Expand All @@ -35,7 +35,7 @@ public static void PushGlobal<CBType>(CommandBuffer cmd, in CBType data, int sha
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public static void Push<CBType>(CommandBuffer cmd, in CBType data, ComputeShader cs, int shaderId) where CBType : struct
{
var cb = TypedConstantBuffer<CBType>.instance;
var cb = ConstantBufferSingleton<CBType>.instance;

cb.UpdateData(cmd, data);
cb.Set(cmd, cs, shaderId);
Expand All @@ -51,7 +51,7 @@ public static void Push<CBType>(CommandBuffer cmd, in CBType data, ComputeShader
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public static void Push<CBType>(CommandBuffer cmd, in CBType data, Material mat, int shaderId) where CBType : struct
{
var cb = TypedConstantBuffer<CBType>.instance;
var cb = ConstantBufferSingleton<CBType>.instance;

cb.UpdateData(cmd, data);
cb.Set(mat, shaderId);
Expand All @@ -65,7 +65,7 @@ public static void Push<CBType>(CommandBuffer cmd, in CBType data, Material mat,
/// <param name="data">Input data of the constant buffer.</param>
public static void UpdateData<CBType>(CommandBuffer cmd, in CBType data) where CBType : struct
{
var cb = TypedConstantBuffer<CBType>.instance;
var cb = ConstantBufferSingleton<CBType>.instance;

cb.UpdateData(cmd, data);
}
Expand All @@ -78,7 +78,7 @@ public static void UpdateData<CBType>(CommandBuffer cmd, in CBType data) where C
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public static void SetGlobal<CBType>(CommandBuffer cmd, int shaderId) where CBType : struct
{
var cb = TypedConstantBuffer<CBType>.instance;
var cb = ConstantBufferSingleton<CBType>.instance;

cb.SetGlobal(cmd, shaderId);
}
Expand All @@ -92,7 +92,7 @@ public static void SetGlobal<CBType>(CommandBuffer cmd, int shaderId) where CBTy
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public static void Set<CBType>(CommandBuffer cmd, ComputeShader cs, int shaderId) where CBType : struct
{
var cb = TypedConstantBuffer<CBType>.instance;
var cb = ConstantBufferSingleton<CBType>.instance;

cb.Set(cmd, cs, shaderId);
}
Expand All @@ -105,13 +105,13 @@ public static void Set<CBType>(CommandBuffer cmd, ComputeShader cs, int shaderId
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public static void Set<CBType>(Material mat, int shaderId) where CBType : struct
{
var cb = TypedConstantBuffer<CBType>.instance;
var cb = ConstantBufferSingleton<CBType>.instance;

cb.Set(mat, shaderId);
}

/// <summary>
/// Release all currently allocated constant buffers.
/// Release all currently allocated singleton constant buffers.
/// This needs to be called before shutting down the application.
/// </summary>
public static void ReleaseAll()
Expand All @@ -122,86 +122,148 @@ public static void ReleaseAll()
m_RegisteredConstantBuffers.Clear();
}

internal abstract class ConstantBufferBase
{
public abstract void Release();
}

internal static void Register(ConstantBufferBase cb)
{
m_RegisteredConstantBuffers.Add(cb);
}
}

class TypedConstantBuffer<CBType> : ConstantBufferBase where CBType : struct
{
// Used to track all global bindings used by this CB type.
HashSet<int> m_GlobalBindings = new HashSet<int>();
// Array is required by the ComputeBuffer SetData API
CBType[] m_Data = new CBType[1];
/// <summary>
/// The base class of Constant Buffer.
/// </summary>
public abstract class ConstantBufferBase
{
/// <summary>
/// Release the constant buffer.
/// </summary>
public abstract void Release();
}

static TypedConstantBuffer<CBType> s_Instance = null;
internal static TypedConstantBuffer<CBType> instance
{
get
{
if (s_Instance == null)
s_Instance = new TypedConstantBuffer<CBType>();
return s_Instance;
}
set
{
s_Instance = value;
}
}
ComputeBuffer m_GPUConstantBuffer = null;

TypedConstantBuffer()
{
m_GPUConstantBuffer = new ComputeBuffer(1, UnsafeUtility.SizeOf<CBType>(), ComputeBufferType.Constant);
ConstantBuffer.Register(this);
}
/// <summary>
/// An instance of a constant buffer.
/// </summary>
/// <typeparam name="CBType">The type of structure representing the constant buffer data.</typeparam>
public class ConstantBuffer<CBType> : ConstantBufferBase where CBType : struct
{
// Used to track all global bindings used by this CB type.
HashSet<int> m_GlobalBindings = new HashSet<int>();
// Array is required by the ComputeBuffer SetData API
CBType[] m_Data = new CBType[1];

public void UpdateData(CommandBuffer cmd, in CBType data)
{
m_Data[0] = data;

ComputeBuffer m_GPUConstantBuffer = null;

/// <summary>
/// Constant Buffer constructor.
/// </summary>
public ConstantBuffer()
{
m_GPUConstantBuffer = new ComputeBuffer(1, UnsafeUtility.SizeOf<CBType>(), ComputeBufferType.Constant);
}

/// <summary>
/// Update the GPU data of the constant buffer.
/// </summary>
/// <param name="cmd">Command Buffer used to execute the graphic commands.</param>
/// <param name="data">Input data of the constant buffer.</param>
public void UpdateData(CommandBuffer cmd, in CBType data)
{
m_Data[0] = data;
#if UNITY_2021_1_OR_NEWER
cmd.SetBufferData(m_GPUConstantBuffer, m_Data);
cmd.SetBufferData(m_GPUConstantBuffer, m_Data);
#else
cmd.SetComputeBufferData(m_GPUConstantBuffer, m_Data);
cmd.SetComputeBufferData(m_GPUConstantBuffer, m_Data);
#endif
}
}

public void SetGlobal(CommandBuffer cmd, int shaderId)
{
m_GlobalBindings.Add(shaderId);
cmd.SetGlobalConstantBuffer(m_GPUConstantBuffer, shaderId, 0, m_GPUConstantBuffer.stride);
}
/// <summary>
/// Bind the constant buffer globally.
/// </summary>
/// <param name="cmd">Command Buffer used to execute the graphic commands.</param>
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public void SetGlobal(CommandBuffer cmd, int shaderId)
{
m_GlobalBindings.Add(shaderId);
cmd.SetGlobalConstantBuffer(m_GPUConstantBuffer, shaderId, 0, m_GPUConstantBuffer.stride);
}

public void Set(CommandBuffer cmd, ComputeShader cs, int shaderId)
{
cmd.SetComputeConstantBufferParam(cs, shaderId, m_GPUConstantBuffer, 0, m_GPUConstantBuffer.stride);
}
/// <summary>
/// Bind the constant buffer to a compute shader.
/// </summary>
/// <param name="cmd">Command Buffer used to execute the graphic commands.</param>
/// <param name="cs">Compute shader to which the constant buffer should be bound.</param>
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public void Set(CommandBuffer cmd, ComputeShader cs, int shaderId)
{
cmd.SetComputeConstantBufferParam(cs, shaderId, m_GPUConstantBuffer, 0, m_GPUConstantBuffer.stride);
}

/// <summary>
/// Bind the constant buffer to a material.
/// </summary>
/// <param name="mat">Material to which the constant buffer should be bound.</param>
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public void Set(Material mat, int shaderId)
{
// This isn't done via command buffer because as long as the buffer itself is not destroyed,
// the binding stays valid. Only the commit of data needs to go through the command buffer.
// We do it here anyway for now to simplify user API.
mat.SetConstantBuffer(shaderId, m_GPUConstantBuffer, 0, m_GPUConstantBuffer.stride);
}

/// <summary>
/// Update the GPU data of the constant buffer and bind it globally.
/// </summary>
/// <param name="cmd">Command Buffer used to execute the graphic commands.</param>
/// <param name="data">Input data of the constant buffer.</param>
/// <param name="shaderId">Shader porperty id to bind the constant buffer to.</param>
public void PushGlobal(CommandBuffer cmd, in CBType data, int shaderId)
{
UpdateData(cmd, data);
SetGlobal(cmd, shaderId);
}

/// <summary>
/// Release the constant buffers.
/// </summary>
public override void Release()
{
// Depending on the device, globally bound buffers can leave stale "valid" shader ids pointing to a destroyed buffer.
// In DX11 it does not cause issues but on Vulkan this will result in skipped drawcalls (even if the buffer is not actually accessed in the shader).
// To avoid this kind of issues, it's good practice to "unbind" all globally bound buffers upon destruction.
foreach (int shaderId in m_GlobalBindings)
Shader.SetGlobalConstantBuffer(shaderId, (ComputeBuffer)null, 0, 0);
m_GlobalBindings.Clear();

public void Set(Material mat, int shaderId)
CoreUtils.SafeRelease(m_GPUConstantBuffer);
}
}

class ConstantBufferSingleton<CBType> : ConstantBuffer<CBType> where CBType : struct
{
static ConstantBufferSingleton<CBType> s_Instance = null;
internal static ConstantBufferSingleton<CBType> instance
{
get
{
// This isn't done via command buffer because as long as the buffer itself is not destroyed,
// the binding stays valid. Only the commit of data needs to go through the command buffer.
// We do it here anyway for now to simplify user API.
mat.SetConstantBuffer(shaderId, m_GPUConstantBuffer, 0, m_GPUConstantBuffer.stride);
if (s_Instance == null)
{
s_Instance = new ConstantBufferSingleton<CBType>();
ConstantBuffer.Register(s_Instance);
}
return s_Instance;
}

public override void Release()
set
{
// Depending on the device, globally bound buffers can leave stale "valid" shader ids pointing to a destroyed buffer.
// In DX11 it does not cause issues but on Vulkan this will result in skipped drawcalls (even if the buffer is not actually accessed in the shader).
// To avoid this kind of issues, it's good practice to "unbind" all globally bound buffers upon destruction.
foreach (int shaderId in m_GlobalBindings)
Shader.SetGlobalConstantBuffer(shaderId, (ComputeBuffer)null, 0, 0);
m_GlobalBindings.Clear();

CoreUtils.SafeRelease(m_GPUConstantBuffer);
s_Instance = null;
s_Instance = value;
}
}

public override void Release()
{
base.Release();
s_Instance = null;
}
}
}
1 change: 1 addition & 0 deletions com.unity.render-pipelines.high-definition/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Fixed usage of Panini Projection with floating point HDRP and Post Processing color buffers.
- Fixed a NaN generating in Area light code.
- Fixed CustomPassUtils scaling issues when used with RTHandles allocated from a RenderTexture.
- Fixed issue with constant buffer being stomped on when async tasks run concurrently to shadows.

### Changed
- Removed the material pass probe volumes evaluation mode.
Expand Down
6 changes: 3 additions & 3 deletions com.unity.render-pipelines.high-definition/Runtime/Lighting/Shadow/HDCachedShadowAtlas.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ public HDCachedShadowAtlas(ShadowMapType type)
m_ShadowType = type;
}

public override void InitAtlas(RenderPipelineResources renderPipelineResources, RenderGraph renderGraph, bool useSharedTexture, int width, int height, int atlasShaderID, Material clearMaterial, int maxShadowRequests, HDShadowInitParameters initParams, BlurAlgorithm blurAlgorithm = BlurAlgorithm.None, FilterMode filterMode = FilterMode.Bilinear, DepthBits depthBufferBits = DepthBits.Depth16, RenderTextureFormat format = RenderTextureFormat.Shadowmap, string name = "")
public override void InitAtlas(HDShadowAtlasInitParameters atlasInitParams)
{
base.InitAtlas(renderPipelineResources, renderGraph, useSharedTexture, width, height, atlasShaderID, clearMaterial, maxShadowRequests, initParams, blurAlgorithm, filterMode, depthBufferBits, format, name);
base.InitAtlas(atlasInitParams);
m_IsACacheForShadows = true;

m_AtlasResolutionInSlots = HDUtils.DivRoundUp(width, m_MinSlotSize);
Expand All @@ -91,7 +91,7 @@ public override void InitAtlas(RenderPipelineResources renderPipelineResources,

// Note: If changing the characteristics of the atlas via HDRP asset, the lights OnEnable will not be called again so we are missing them, however we can explicitly
// put them back up for placement. If this is the first Init of the atlas, the lines below do nothing.
DefragmentAtlasAndReRender(initParams);
DefragmentAtlasAndReRender(atlasInitParams.initParams);
m_CanTryPlacement = true;
m_NeedOptimalPacking = true;
}
Expand Down
14 changes: 6 additions & 8 deletions com.unity.render-pipelines.high-definition/Runtime/Lighting/Shadow/HDCachedShadowManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,18 +274,16 @@ private HDCachedShadowManager()
areaShadowAtlas = new HDCachedShadowAtlas(ShadowMapType.AreaLightAtlas);
}

internal void InitPunctualShadowAtlas(RenderPipelineResources renderPipelineResources, RenderGraph renderGraph, bool useSharedTexture, int width, int height, int atlasShaderID, Material clearMaterial, int maxShadowRequests, HDShadowInitParameters initParams,
HDShadowAtlas.BlurAlgorithm blurAlgorithm = HDShadowAtlas.BlurAlgorithm.None, FilterMode filterMode = FilterMode.Bilinear, DepthBits depthBufferBits = DepthBits.Depth16, RenderTextureFormat format = RenderTextureFormat.Shadowmap, string name = "")
internal void InitPunctualShadowAtlas(HDShadowAtlas.HDShadowAtlasInitParameters atlasInitParams)
{
m_InitParams = initParams;
punctualShadowAtlas.InitAtlas(renderPipelineResources, renderGraph, useSharedTexture, width, height, atlasShaderID, clearMaterial, maxShadowRequests, initParams, blurAlgorithm, filterMode, depthBufferBits, format, name);
m_InitParams = atlasInitParams.initParams;
punctualShadowAtlas.InitAtlas(atlasInitParams);
}

internal void InitAreaLightShadowAtlas(RenderPipelineResources renderPipelineResources, RenderGraph renderGraph, bool useSharedTexture, int width, int height, int atlasShaderID, Material clearMaterial, int maxShadowRequests, HDShadowInitParameters initParams,
HDShadowAtlas.BlurAlgorithm blurAlgorithm = HDShadowAtlas.BlurAlgorithm.None, FilterMode filterMode = FilterMode.Bilinear, DepthBits depthBufferBits = DepthBits.Depth16, RenderTextureFormat format = RenderTextureFormat.Shadowmap, string name = "")
internal void InitAreaLightShadowAtlas(HDShadowAtlas.HDShadowAtlasInitParameters atlasInitParams)
{
m_InitParams = initParams;
areaShadowAtlas.InitAtlas(renderPipelineResources, renderGraph, useSharedTexture, width, height, atlasShaderID, clearMaterial, maxShadowRequests, initParams, blurAlgorithm, filterMode, depthBufferBits, format, name);
m_InitParams = atlasInitParams.initParams;
areaShadowAtlas.InitAtlas(atlasInitParams);
}

internal void RegisterLight(HDAdditionalLightData lightData)
Expand Down
6 changes: 3 additions & 3 deletions com.unity.render-pipelines.high-definition/Runtime/Lighting/Shadow/HDDynamicShadowAtlas.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ partial class HDDynamicShadowAtlas : HDShadowAtlas
float m_RcpScaleFactor = 1;
HDShadowResolutionRequest[] m_SortedRequestsCache;

public HDDynamicShadowAtlas(RenderPipelineResources renderPipelineResources, RenderGraph renderGraph, bool useSharedTexture, int width, int height, int atlasShaderID, Material clearMaterial, int maxShadowRequests, HDShadowInitParameters initParams, BlurAlgorithm blurAlgorithm = BlurAlgorithm.None, FilterMode filterMode = FilterMode.Bilinear, DepthBits depthBufferBits = DepthBits.Depth16, RenderTextureFormat format = RenderTextureFormat.Shadowmap, string name = "")
: base(renderPipelineResources, renderGraph, useSharedTexture, width, height, atlasShaderID, clearMaterial, maxShadowRequests, initParams, blurAlgorithm, filterMode, depthBufferBits, format, name)
public HDDynamicShadowAtlas(HDShadowAtlasInitParameters atlaInitParams)
: base(atlaInitParams)
{
m_SortedRequestsCache = new HDShadowResolutionRequest[Mathf.CeilToInt(maxShadowRequests)];
m_SortedRequestsCache = new HDShadowResolutionRequest[Mathf.CeilToInt(atlaInitParams.maxShadowRequests)];
}

internal void ReserveResolution(HDShadowResolutionRequest shadowRequest)
Expand Down
Loading