Skip to content

Commit 4be2d31

Browse files
PaulDemeulenaereEvergreen
PaulDemeulenaere
authored and
Evergreen
committed
[VFX] Support CustomType & GraphicsBuffer in CustomHLSL
Initially code from [hackweek](https://drive.google.com/file/d/1g8j6nYsJwYIe80wRt4tBNlVbEayeGqom/view?usp=drive_link) project but made it clean because the improvement worth it and it fixes a couple of issue. Fix some usage in CustomHLSL which is now allowing: - `StructuredBuffer<CustomType>`: this is about UUM-64315 - `RWStructuredBuffer<float>`: Prevent usage of global buffer for this common case - `StructuredBuffer<uint3>`: Even if we don't have `uint3` in VFX, we are allowing this kind of declaration in CustomHLSL - `RWByteAddressBuffer`, `AppendStructuredBuffer`, `ConsumeStructuredBuffer`, `Buffer<>`, `RWBuffer<>`: I covered all common type of HLSL 🎁 Bonus, fix regression at https://github.cds.internal.unity3d.com/unity/unity/pull/45766/commits/ce1a39b236bcddaebfbc3cadb8c9e8ccdc6eaa0c which is simply a revert from https://github.cds.internal.unity3d.com/unity/unity/commit/517c85322917cc0ff86a75e2c5887d5685e2db04 ## Behind the curtain There was an illegal change of expression within custom hlsl, see this code in trunk (BuildExpression modifies parent expression): https://github.cds.internal.unity3d.com/unity/unity/blob/eb1023001e78a637400bfa08cb0f8f4b287c381a/Packages/com.unity.visualeffectgraph/Editor/Models/Operators/Implementations/CustomHLSL.cs#L246 I changed the approach to use an intermediate expression which link the source graphicsBuffer to a usage. The resolution of this issue has been done with this commit https://github.cds.internal.unity3d.com/unity/unity/pull/42668/commits/5f4581f13ee481f24aa27b3be8ccf1d5ac2d84ab Here a sum up of the situation: ![image](https://media.github.cds.internal.unity3d.com/user/42/files/1f6e36a5-3725-4152-9185-f4cc874f010b) Let's take the example of this graph: ![image](https://media.github.cds.internal.unity3d.com/user/42/files/fc4da21b-a69a-4b46-a9c9-c93d889b2ba3) It will generates an expression graph like this: ![image](https://media.github.cds.internal.unity3d.com/user/42/files/bb629785-c551-413d-a0f7-c648086d2887) _N.B.: In reality, if GraphicsBufferUsage are identical, expressions `1.` & `2.` are the same reference._ We are now going to track `VFXExpressionGraphicsBufferUsage` in compilation to associate every GraphicsBuffer with their future declaration. The same approach is use in CustomHLSL block, operator but also with SampleBuffer. ## Known Limitation - There is an issue regarding include in custom HLSL => UUM-64570, there is a repro in `026_CellularAutomaton` (which a workaround) - Diverging usage of buffer can be marked as illegal when it's technically possible: ![image](https://media.github.cds.internal.unity3d.com/user/42/files/16eb1b4a-7dc6-4671-9383-44478694a680) _N.B.: I think we can considered this limitation as a bug, we are gathering usage globally while it should be done only per context._ , this is an issue which can be fixed with UUM-66620 (this also an issue where usage should be computed per context) - We aren't supporting `RWTexture2D<float>` and other, it would require the integration of texture usage pattern, I would like to explore this in the future.
1 parent 351b3ea commit 4be2d31

40 files changed

+20563
-164
lines changed

‎Packages/com.unity.visualeffectgraph/Editor/Compiler/VFXCodeGenerator.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -466,9 +466,9 @@ internal static void BuildParameterBuffer(VFXTaskCompiledData taskData, IEnumera
466466
var parameterBuffer = new VFXShaderWriter();
467467
needsGraphValueStruct = parameterBuffer.WriteGraphValuesStruct(taskData.uniformMapper);
468468
parameterBuffer.WriteLine();
469-
parameterBuffer.WriteBufferTypeDeclaration(taskData.graphicsBufferUsage.Values);
469+
parameterBuffer.WriteBufferTypeDeclaration(taskData.bufferUsage.Values);
470470
parameterBuffer.WriteLine();
471-
parameterBuffer.WriteBuffer(taskData.uniformMapper, taskData.graphicsBufferUsage);
471+
parameterBuffer.WriteBuffer(taskData.uniformMapper, taskData.bufferUsage);
472472
parameterBuffer.WriteLine();
473473
parameterBuffer.WriteTexture(taskData.uniformMapper, filteredOutTextures);
474474
parameterBufferContent = parameterBuffer.ToString();
@@ -697,15 +697,15 @@ static private StringBuilder Build(
697697
var allSourceAttributes = contextData.GetAttributes().Where(a => (contextData.IsSourceAttributeUsed(a.attrib, context)));
698698

699699
var globalDeclaration = new VFXShaderWriter();
700-
globalDeclaration.WriteBufferTypeDeclaration(taskData.graphicsBufferUsage.Values);
700+
globalDeclaration.WriteBufferTypeDeclaration(taskData.bufferUsage.Values);
701701
globalDeclaration.WriteLine();
702702
var particleData = (contextData as VFXDataParticle);
703703
var systemUniformMapper = particleData.systemUniformMapper;
704704
taskData.uniformMapper.OverrideUniformsNamesWithOther(systemUniformMapper);
705705
var needsGraphValueStruct = globalDeclaration.WriteGraphValuesStruct(taskData.uniformMapper);
706706
globalDeclaration.WriteLine();
707707

708-
globalDeclaration.WriteBuffer(taskData.uniformMapper, taskData.graphicsBufferUsage);
708+
globalDeclaration.WriteBuffer(taskData.uniformMapper, taskData.bufferUsage);
709709
globalDeclaration.WriteLine();
710710
globalDeclaration.WriteTexture(taskData.uniformMapper);
711711
globalDeclaration.WriteAttributeStruct(allCurrentAttributes.Select(a => a.attrib), "VFXAttributes");
@@ -978,7 +978,7 @@ private static void BuildBlock(VFXTaskCompiledData taskData, VFXShaderWriter blo
978978
if (!blockDeclared.Contains(methodName))
979979
{
980980
blockDeclared.Add(methodName);
981-
blockFunction.WriteBlockFunction(taskData.gpuMapper,
981+
blockFunction.WriteBlockFunction(taskData,
982982
methodName,
983983
block.source,
984984
parameters,

‎Packages/com.unity.visualeffectgraph/Editor/Compiler/VFXExpressionGraph.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,17 @@ private void CompileExpressionContext(IEnumerable<VFXContext> contexts,
8282
foreach (var exp in expressionsToReduced.Values)
8383
AddExpressionDataRecursively(m_ExpressionsData, exp);
8484

85-
var graphicsBufferUsageType = m_GraphicsBufferUsageType
85+
var bufferUsage = m_BufferUsage
8686
.Concat(expressionContext.GraphicsBufferUsageType)
8787
.GroupBy(o => o.Key).ToArray();
8888

89-
m_GraphicsBufferUsageType.Clear();
90-
foreach (var expression in graphicsBufferUsageType)
89+
m_BufferUsage.Clear();
90+
foreach (var expression in bufferUsage)
9191
{
9292
var types = expression.Select(o => o.Value);
9393
if (types.Count() != 1)
9494
throw new InvalidOperationException("Diverging type usage for GraphicsBuffer : " + types.Select(o => o.ToString()).Aggregate((a, b) => a + b));
95-
m_GraphicsBufferUsageType.Add(expression.Key, types.First());
95+
m_BufferUsage.Add(expression.Key, types.First());
9696
}
9797

9898
if (target == VFXDeviceTarget.GPU)
@@ -294,12 +294,12 @@ private VFXExpressionMapper BuildMapper(VFXContext context, Dictionary<VFXContex
294294

295295
public IEnumerable<VFXLayoutElementDesc> GlobalEventAttributes => m_GlobalEventAttributes;
296296

297-
public ReadOnlyDictionary<VFXExpression, Type> GraphicsBufferTypeUsage => new ReadOnlyDictionary<VFXExpression, Type>(m_GraphicsBufferUsageType);
297+
public ReadOnlyDictionary<VFXExpression, BufferUsage> BufferUsage => new ReadOnlyDictionary<VFXExpression, BufferUsage>(m_BufferUsage);
298298

299299
public IHLSLCodeHolder[] customHLSLExpressions => m_CustomHLSLExpressions;
300300

301301
private IHLSLCodeHolder[] m_CustomHLSLExpressions;
302-
private Dictionary<VFXExpression, Type> m_GraphicsBufferUsageType = new Dictionary<VFXExpression, Type>();
302+
private Dictionary<VFXExpression, BufferUsage> m_BufferUsage = new Dictionary<VFXExpression, BufferUsage>();
303303
private HashSet<VFXExpression> m_Expressions = new HashSet<VFXExpression>();
304304
private Dictionary<VFXExpression, VFXExpression> m_CPUExpressionsToReduced = new Dictionary<VFXExpression, VFXExpression>();
305305
private Dictionary<VFXExpression, VFXExpression> m_GPUExpressionsToReduced = new Dictionary<VFXExpression, VFXExpression>();

‎Packages/com.unity.visualeffectgraph/Editor/Compiler/VFXGraphCompiledData.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ struct VFXTaskCompiledData
6666
public VFXUniformMapper uniformMapper;
6767
public VFXSGInputs SGInputs;
6868
public List<uint> instancingSplitValues;
69-
public ReadOnlyDictionary<VFXExpression, Type> graphicsBufferUsage;
69+
public ReadOnlyDictionary<VFXExpression, BufferUsage> bufferUsage;
7070
public VFXMapping[] parameters;
7171
public (VFXSlot slot, VFXData data)[] linkedEventOut;
7272
public IHLSLCodeHolder[] hlslCodeHolders;
@@ -842,7 +842,7 @@ private void GenerateShaders(List<GeneratedCodeData> outGeneratedCodeData, VFXEx
842842
var contextData = compiledData.taskToCompiledData[task];
843843
contextData.gpuMapper = gpuMapper;
844844
contextData.uniformMapper = uniformMapper;
845-
contextData.graphicsBufferUsage = graph.GraphicsBufferTypeUsage;
845+
contextData.bufferUsage = graph.BufferUsage;
846846

847847
if (task.doesGenerateShader)
848848
{

‎Packages/com.unity.visualeffectgraph/Editor/Compiler/VFXShaderWriter.cs‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,11 @@ private void WriteBufferTypeDeclaration(Type type, HashSet<Type> alreadyGenerate
303303
}
304304
}
305305

306-
public void WriteBufferTypeDeclaration(IEnumerable<Type> types)
306+
public void WriteBufferTypeDeclaration(IEnumerable<BufferUsage> usages)
307307
{
308-
types = types.Select(type =>
308+
var types = usages.Select(usage =>
309309
{
310+
var type = usage.actualType;
310311
if (IsBufferBuiltinType(type))
311312
{
312313
//Resolve type which are conflicting behind the same VFXValueType (Vector4 & Color for instance)
@@ -318,26 +319,25 @@ public void WriteBufferTypeDeclaration(IEnumerable<Type> types)
318319

319320
var alreadyGeneratedStructure = new HashSet<Type>();
320321
foreach (var type in types)
322+
{
323+
if (type == typeof(void))
324+
continue;
325+
321326
WriteBufferTypeDeclaration(type, alreadyGeneratedStructure);
327+
}
322328
}
323329

324-
public void WriteBuffer(VFXUniformMapper mapper, ReadOnlyDictionary<VFXExpression, Type> usageGraphicsBuffer)
330+
public void WriteBuffer(VFXUniformMapper mapper, ReadOnlyDictionary<VFXExpression, BufferUsage> usageBuffer)
325331
{
326332
foreach (var buffer in mapper.buffers)
327333
{
328334
var name = mapper.GetName(buffer);
329-
330-
if (buffer.valueType == VFXValueType.Buffer && usageGraphicsBuffer.TryGetValue(buffer, out var type))
335+
if (buffer.valueType == VFXValueType.Buffer && usageBuffer.TryGetValue(buffer, out var type))
331336
{
332-
if (type == null)
337+
if (!type.valid)
333338
throw new NullReferenceException("Unexpected null type in graphicsBuffer usage");
334339

335-
var structureName = GetStructureName(type);
336-
WriteLineFormat("StructuredBuffer<{0}> {1};", structureName, name);
337-
}
338-
else if (buffer is VFXGraphicsBufferValue graphicsBufferValue && !string.IsNullOrEmpty(graphicsBufferValue.templateType))
339-
{
340-
WriteLineFormat("{0} {1};", VFXExpression.TypeToCode(buffer.valueType), name);
340+
WriteLineFormat("{0} {1};", GetBufferDeclaration(type), name);
341341
}
342342
else
343343
{
@@ -494,7 +494,19 @@ private string AggregateParameters(List<string> parameters)
494494
return parameters.Count == 0 ? "" : parameters.Aggregate((a, b) => a + ", " + b);
495495
}
496496

497-
private static string GetFunctionParameterType(VFXExpression exp)
497+
private static string GetBufferDeclaration(BufferUsage bufferUsage)
498+
{
499+
if (string.IsNullOrEmpty(bufferUsage.verbatimType))
500+
return bufferUsage.container.ToString();
501+
502+
var verbatimType = IsBufferBuiltinType(bufferUsage.actualType)
503+
? VFXExpression.TypeToCode(VFXExpression.GetVFXValueTypeFromType(bufferUsage.actualType))
504+
: bufferUsage.verbatimType;
505+
506+
return $"{bufferUsage.container}<{verbatimType}>";
507+
}
508+
509+
private static string GetFunctionParameterType(VFXExpression exp, ReadOnlyDictionary<VFXExpression, BufferUsage> usages)
498510
{
499511
switch (exp.valueType)
500512
{
@@ -505,8 +517,9 @@ private static string GetFunctionParameterType(VFXExpression exp)
505517
case VFXValueType.TextureCubeArray: return "VFXSamplerCubeArray";
506518
case VFXValueType.CameraBuffer: return "VFXSamplerCameraBuffer";
507519
case VFXValueType.Buffer:
508-
var bufferExpression = (VFXGraphicsBufferValue)exp;
509-
return string.IsNullOrEmpty(bufferExpression.templateType) ? "ByteAddressBuffer" : $"StructuredBuffer<{bufferExpression.templateType}>";
520+
if (!usages.TryGetValue(exp, out var usage))
521+
throw new KeyNotFoundException("Cannot find appropriate usage for " + exp);
522+
return GetBufferDeclaration(usage);
510523
default:
511524
return VFXExpression.TypeToCode(exp.valueType);
512525
}
@@ -544,13 +557,13 @@ public struct FunctionParameter
544557
public VFXAttributeMode mode;
545558
}
546559

547-
public void WriteBlockFunction(VFXExpressionMapper mapper, string functionName, string source, IEnumerable<FunctionParameter> parameters, string commentMethod)
560+
public void WriteBlockFunction(VFXTaskCompiledData taskData, string functionName, string source, IEnumerable<FunctionParameter> parameters, string commentMethod)
548561
{
549562
var parametersCode = new List<string>();
550563
foreach (var parameter in parameters)
551564
{
552565
var inputModifier = GetInputModifier(parameter.mode);
553-
var parameterType = GetFunctionParameterType(parameter.expression);
566+
var parameterType = GetFunctionParameterType(parameter.expression, taskData.bufferUsage);
554567
parametersCode.Add(string.Format("{0}{1} {2}", inputModifier, parameterType, parameter.name));
555568
}
556569

‎Packages/com.unity.visualeffectgraph/Editor/Compiler/VFXUniformMapper.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ private void CollectAndAddUniforms(VFXExpression exp, IEnumerable<VFXExpressionM
6565
{
6666
foreach (var data in datas)
6767
{
68-
m_CurrentUniformIndex++;
6968
m_NameCounts.TryGetValue(data.name, out uint count);
7069
m_NameCounts[data.name] = count + 1u;
7170
string name = data.id == -1 && (!VFXExpression.IsUniform(exp.valueType) || !m_NeedsNameSuffixes) ? data.name : $"{data.name}_{VFXCodeGeneratorHelper.GeneratePrefix(count)}";

‎Packages/com.unity.visualeffectgraph/Editor/Core/VFXLibrary.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,22 @@ static class VFXLibrary
179179
public static IEnumerable<VFXModelDescriptor<VFXOperator>> GetOperators() { LoadIfNeeded(); return VFXViewPreference.displayExperimentalOperator ? m_OperatorDescs : m_OperatorDescs.Where(o => !o.infoAttribute.experimental); }
180180
public static IEnumerable<VFXModelDescriptor<VFXSlot>> GetSlots() { LoadSlotsIfNeeded(); return m_SlotDescs.Values; }
181181
public static IEnumerable<Type> GetSlotsType() { LoadSlotsIfNeeded(); return m_SlotDescs.Keys; }
182+
183+
public static IEnumerable<Type> GetGraphicsBufferType()
184+
{
185+
foreach (var type in VFXLibrary.GetSlotsType())
186+
{
187+
if (VFXExpression.IsUniform(VFXExpression.GetVFXValueTypeFromType(type)))
188+
yield return type;
189+
else
190+
{
191+
var typeAttribute = VFXLibrary.GetAttributeFromSlotType(type);
192+
if (typeAttribute != null && typeAttribute.usages.HasFlag(VFXTypeAttribute.Usage.GraphicsBuffer))
193+
yield return type;
194+
}
195+
}
196+
}
197+
182198
public static bool IsSpaceableSlotType(Type type) { LoadSlotsIfNeeded(); return m_SlotSpaceable.Contains(type); }
183199
public static VFXTypeAttribute GetAttributeFromSlotType(Type type)
184200
{

‎Packages/com.unity.visualeffectgraph/Editor/Expressions/VFXExpressionAbstract.cs‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,21 +167,6 @@ public static Type TypeToType(VFXValueType type)
167167
throw new NotImplementedException(type.ToString());
168168
}
169169

170-
public static Type StringToType(string type)
171-
{
172-
switch (type)
173-
{
174-
case "float": return typeof(float);
175-
case "float2": return typeof(Vector2);
176-
case "float3": return typeof(Vector3);
177-
case "float4": return typeof(Vector4);
178-
case "int": return typeof(int);
179-
case "uint": return typeof(uint);
180-
}
181-
182-
throw new NotSupportedException($"Type not supported: {type}");
183-
}
184-
185170
public static bool IsTypeValidOnGPU(VFXValueType type)
186171
{
187172
switch (type)

‎Packages/com.unity.visualeffectgraph/Editor/Expressions/VFXExpressionBuffer.cs‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,15 @@ protected override VFXExpression Evaluate(VFXExpression[] constParents)
4545
#pragma warning disable 0659
4646
class VFXExpressionSampleBuffer : VFXExpression
4747
{
48-
public VFXExpressionSampleBuffer() : this(null, VFXValueType.None, string.Empty, VFXValue<GraphicsBuffer>.Default, VFXValue<uint>.Default, VFXValue<uint>.Default, VFXValue<uint>.Default)
48+
public VFXExpressionSampleBuffer() : this(VFXValueType.None, string.Empty, VFXValue<GraphicsBuffer>.Default, VFXValue<uint>.Default, VFXValue<uint>.Default, VFXValue<uint>.Default)
4949
{
5050
}
5151

52-
private Type m_SampledType;
5352
private VFXValueType m_FieldType;
5453
private string m_FieldPath;
5554

56-
public Type GetSampledType()
55+
public VFXExpressionSampleBuffer(VFXValueType fieldType, string path, VFXExpression graphicsBuffer, VFXExpression index, VFXExpression stride, VFXExpression count) : base(Flags.InvalidOnCPU, graphicsBuffer, index, stride, count)
5756
{
58-
return m_SampledType;
59-
}
60-
61-
public VFXExpressionSampleBuffer(Type sampledType, VFXValueType fieldType, string path, VFXExpression graphicsBuffer, VFXExpression index, VFXExpression stride, VFXExpression count) : base(Flags.InvalidOnCPU, graphicsBuffer, index, stride, count)
62-
{
63-
m_SampledType = sampledType;
6457
m_FieldType = fieldType;
6558
m_FieldPath = path;
6659
}
@@ -74,13 +67,12 @@ public override bool Equals(object obj)
7467
if (other == null)
7568
return false;
7669

77-
return m_SampledType.Equals(other.m_SampledType) && m_FieldPath.Equals(other.m_FieldPath) && m_FieldType.Equals(other.m_FieldType);
70+
return m_FieldPath.Equals(other.m_FieldPath) && m_FieldType.Equals(other.m_FieldType);
7871
}
7972

8073
protected override int GetInnerHashCode()
8174
{
8275
int hash = base.GetInnerHashCode();
83-
hash = (hash * 397) ^ m_SampledType.GetHashCode();
8476
hash = (hash * 397) ^ m_FieldPath.GetHashCode();
8577
hash = (hash * 397) ^ m_FieldType.GetHashCode();
8678
return hash;
@@ -89,7 +81,6 @@ protected override int GetInnerHashCode()
8981
protected override VFXExpression Reduce(VFXExpression[] reducedParents)
9082
{
9183
var newExpression = (VFXExpressionSampleBuffer)base.Reduce(reducedParents);
92-
newExpression.m_SampledType = m_SampledType;
9384
newExpression.m_FieldPath = m_FieldPath;
9485
newExpression.m_FieldType = m_FieldType;
9586
return newExpression;

0 commit comments

Comments
 (0)