Skip to content

Commit

Permalink
[VFX] Custom hlsl parsing issues
Browse files Browse the repository at this point in the history
Jira: UUM-79389

### First issue
When this HLSL function is used, the `position` attribute is not recognized
```hlsl
void Test(inout VFXAttributes attributes, in float value, in float altSize)
{
    if (attributes.position.y >= value)
    {
        attributes.size = altSize;
    }
}
```
In this simple case it does no harm, but in more complex cases it can break the shader compilation.

### Second issue
The same function, but with a different formatting is not properly parsed, leading to function's parameters not being identified.
```hlsl
void Test(inout VFXAttributes attributes
   , in float value
   , in float altSize
)
{
    if (attributes.position.y >= value)
    {
        attributes.size = altSize;
    }
}
```
  • Loading branch information
julienamsellem authored and Evergreen committed Sep 19, 2024
1 parent f9439a5 commit a806f0b
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public HLSLMissingIncludeFile(string filePath)
class HLSLFunctionParameter
{
// Match inout/in/out accessor then any whitespace then the parameter type then optionally a template type any whitespace and then the parameter name
static readonly Regex s_ParametersParser = new Regex(@"(?<access>(inout|in|out)\b)?\s*(?<type>\w+)(?:[<](?<template>\w+)[>])?\s*(?<parameter>\w+)(?:,\s*)?", RegexOptions.Compiled);
static readonly Regex s_ParametersParser = new Regex(@"(?<access>(inout|in|out)\b)?\s*(?<type>\w+)(?:[<](?<template>\w+)[>])?\s*(?<parameter>\w+)(?:\s*,\s*)?", RegexOptions.Compiled);

readonly string m_RawCode;

Expand All @@ -200,7 +200,7 @@ public static IEnumerable<HLSLFunctionParameter> Parse(string hlsl, Dictionary<s
match.Groups["type"].Value,
match.Groups["template"].Value,
match.Groups["parameter"].Value,
doc.TryGetValue(match.Groups["parameter"].Value, out var tooltip) ? tooltip : null);
doc.GetValueOrDefault(match.Groups["parameter"].Value));
}
}

Expand Down Expand Up @@ -248,7 +248,7 @@ public static IEnumerable<HLSLFunctionParameter> Parse(string hlsl, Dictionary<s
class HLSLFunction
{
// Match all attributes not followed by a single '=' character and also catch ++ and -- prefix (for read+write case)
static readonly string s_AttributeReadPattern = @"(?<op>\+\+|\-\-)?{0}.(?<name>\w+\b)(?!.*(?:[^=]=[^=]))";
static readonly string s_AttributeReadPattern = @"(?<op>\+{{2}}|-{{2}})?{0}.(?<name>\w+\b)(?!(?:\.\w+)?\s*[^=<>+-]=[^=])";
// Match all attributes followed by an assigment operator (=, ++, --, +=, -= ...) even if there's multiple assignments on the same line
private static readonly string s_AttributeWritePattern = @"{0}.(?<name>\w+)(?:\.\w)?\s*(?<op>[\/\-+*%&\|\^]?=[^=]|(\+\+|\-\-|<<=|>>=))";
// Simply match lines where a 'return' statement is used
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,36 @@ public void Check_CustomHLSL_Block_Attribute_Read_Access()
Assert.IsTrue(hlslBlock.attributes.All(x => x.mode == VFXAttributeMode.Read));
}

[Test, Description("Covers issue UUM-79389")]
public void Check_CustomHLSL_Block_Attribute_Read_SubField_Access()
{
// Arrange
var hlslCode =
"void Test(inout VFXAttributes attributes, in float value, in float altSize)\n" +
"{\n" +
" if (attributes.position.y >= value)\n" +
" {\n" +
" attributes.size = altSize;\n" +
" }\n" +
"}";

var hlslBlock = CreateCustomHLSLBlock();
hlslBlock.SetSettingValue("m_HLSLCode", hlslCode);

// Act
hlslBlock.Invalidate(VFXModel.InvalidationCause.kSettingChanged);

// Assert
Assert.AreEqual(2, hlslBlock.attributes.Count());
var sizeAttribute = hlslBlock.attributes.First();
Assert.AreEqual(sizeAttribute.attrib.name, "size");
Assert.AreEqual(sizeAttribute.mode, VFXAttributeMode.Write);

var positionAttribute = hlslBlock.attributes.Last();
Assert.AreEqual(positionAttribute.attrib.name, "position");
Assert.AreEqual(positionAttribute.mode, VFXAttributeMode.Read);
}

[Test]
public void Check_CustomHLSL_Block_Attribute_Write_Access()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,30 @@ public void HLSL_Variadic_Attributes()
Assert.AreEqual(1, errors.Length, "Missing error feedback when using variadic attribute in custom hlsl code");
Assert.IsInstanceOf<HLSLVFXAttributeIsVariadic>(errors[0]);
}

[Test, Description("Covers issue UUM-79389")]
public void HLSL_Parameters_On_Different_Lines()
{
// Arrange
var hlslCode =
"void CustomHLSL(inout VFXAttributes attributes" +
" , in float3 offset" +
" , in float speedFactor)" + "\n" +
"{" + "\n" +
" attributes.position += offset;" + "\n" +
" attributes.velocity *= speedFactor;" + "\n" +
"}";

// Act
var function = HLSLFunction.Parse(this.attributesManager, hlslCode).Single();

// Assert
Assert.AreEqual(3, function.inputs.Count);
Assert.AreEqual(new [] { "attributes", "offset", "speedFactor" }, function.inputs.Select(x => x.name));
Assert.AreEqual(new [] { "VFXAttributes", "float3", "float" }, function.inputs.Select(x => x.rawType));
Assert.AreEqual(new [] { HLSLAccess.INOUT, HLSLAccess.IN, HLSLAccess.IN }, function.inputs.Select(x => x.access));

}
}
}
#endif

0 comments on commit a806f0b

Please sign in to comment.