Skip to content

Commit

Permalink
Improve robustness of string array parsing
Browse files Browse the repository at this point in the history
- Add support for parsing arrays of strings with internal spaces (e.g. "Item A, Item B, Item C")
- Leverage the new parsing logic in getUIProperties.
- Update the interface of LamaSSS to enable enum parsing.
- Extend unit tests to validate the new logic.
  • Loading branch information
jstone-lucasfilm committed May 23, 2023
1 parent f942671 commit 7b229d1
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 40 deletions.
4 changes: 2 additions & 2 deletions libraries/bxdf/lama/lama_sss.mtlx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
doc="Diffuse Mean Free Path, expressed for each color channel in mm. Indicates on average how much the light travels under the surface before being scattered. The higher the value, the softer the result will be. If null, the computation simplifies to a Lambertian lobe." />
<input name="sssScale" type="float" value="1.0" uiname="Scale" uifolder="SSS"
doc="Multiplies the radius, to adjust its scale to the scene at hand. If null, the computation simplifies to a Lambertian lobe." />
<input name="sssMode" type="integer" enum="Path-traced Davis,Path-traced exponential,Diffusion Burley,Diffusion Burley (mean free path)" enumvalues="0,1,2,3" value="0" uiname="Mode" uifolder="Main"
<input name="sssMode" type="integer" uniform="true" enum="Path-traced Davis,Path-traced exponential,Diffusion Burley,Diffusion Burley (mean free path)" enumvalues="0,1,2,3" value="0" uiname="Mode" uifolder="Main"
doc="Selects what method should be used to compute sub-surface scattering. Proposes two path-traced variants, and a more traditional approximate diffusion model." />
<input name="sssIOR" type="float" value="1.0" uimin="1.0" uimax="2.0" uiname="IOR" uifolder="SSS"
doc="Index of refraction use to trigger cases of total internal reflections, when the paths are reaching the surface after having travelled under it. Can be used to avoid excessive glow in highly curved regions (corners, creases, ...)." />
Expand All @@ -25,7 +25,7 @@
doc="When enabled, ignores internal geometry and jumps to the last surface." />
<input name="sssUnitLength" type="float" value="0.00328" uiname="Unit Length" uifolder="SSS"
doc="Specifies what unit length the scene is using. It is a multiplier on the mean free path or diffuse mean free path which is expressed in mm. The default value of 0.00328 converts between feet and mm." />
<input name="mode" type="integer" enum="Reflection,Transmission,Reflection(with direct illumination)" enumvalues="0,1,2" value="0" uiname="Mode" uifolder="Advanced"
<input name="mode" type="integer" uniform="true" enum="Reflection,Transmission,Reflection(with direct illumination)" enumvalues="0,1,2" value="0" uiname="Mode" uifolder="Advanced"
doc="If the subsurface is enabled, Reflection: should be used when both the camera and the light are outside of the object. Reflection(with direct illumination): should be used when both the camera and the light are outside of the object. This mode also computes the direct illumination at the sss ray exit point. Transmission: should be used when the light is inside the object while the camera is outside. " />
<input name="albedoInversionMethod" type="integer" enum="Pixar,Chiang" enumvalues="0,1" value="0" uiname="Albedo Inversion Method" uifolder="Advanced"
doc="Decides which albedo inversion methods is used. Pixar: Does the Pixar Path Traced SSS default albedo inversion. Chiang: Does Chiang's albedo inversion (with no dmfp remapping). The look is closer to Arnold Standard Surface randomwalk." />
Expand Down
7 changes: 5 additions & 2 deletions source/MaterialXCore/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,13 @@ template <class T> void stringToData(const string& str, enable_if_mx_matrix_t<T>

template <class T> void stringToData(const string& str, enable_if_std_vector_t<T>& data)
{
for (const string& token : splitString(str, ARRAY_VALID_SEPARATORS))
// This code path parses an array of arbitrary substrings, so we split the string
// in a fashion that preserves substrings with internal spaces.
const string COMMA_SEPARATOR = ",";
for (const string& token : splitString(str, COMMA_SEPARATOR))
{
typename T::value_type val;
stringToData(token, val);
stringToData(trimSpaces(token), val);
data.push_back(val);
}
}
Expand Down
40 changes: 6 additions & 34 deletions source/MaterialXRender/Util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,46 +127,18 @@ unsigned int getUIProperties(InputPtr input, const string& target, UIProperties&

if (input->getIsUniform())
{
string enumString = input->getAttribute(ValueElement::ENUM_ATTRIBUTE);
if (!enumString.empty())
uiProperties.enumeration = input->getTypedAttribute<StringVec>(ValueElement::ENUM_ATTRIBUTE);
if (!uiProperties.enumeration.empty())
{
uiProperties.enumeration = splitString(enumString, ",");
if (!uiProperties.enumeration.empty())
propertyCount++;
propertyCount++;
}

const string& enumerationValues = input->getAttribute(ValueElement::ENUM_VALUES_ATTRIBUTE);
StringVec enumerationValues = input->getTypedAttribute<StringVec>(ValueElement::ENUM_VALUES_ATTRIBUTE);
if (!enumerationValues.empty())
{
const string& elemType = input->getType();
const TypeDesc* typeDesc = TypeDesc::get(elemType);
if (typeDesc->isScalar() || typeDesc->isFloat2() || typeDesc->isFloat3() ||
typeDesc->isFloat4())
{
StringVec stringValues = splitString(enumerationValues, ",");
string valueString;
size_t elementCount = typeDesc->getSize();
elementCount--;
size_t count = 0;
for (const string& val : stringValues)
{
if (count == elementCount)
{
valueString += val;
uiProperties.enumerationValues.push_back(Value::createValueFromStrings(valueString, elemType));
valueString.clear();
count = 0;
}
else
{
valueString += val + ",";
count++;
}
}
}
else
for (const string& val : enumerationValues)
{
uiProperties.enumerationValues.push_back(Value::createValue(enumerationValues));
uiProperties.enumerationValues.push_back(Value::createValueFromStrings(val, input->getType()));
}
if (uiProperties.enumeration.size() != uiProperties.enumerationValues.size())
{
Expand Down
4 changes: 2 additions & 2 deletions source/MaterialXTest/MaterialXCore/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ TEST_CASE("Typed values", "[value]")
mx::BoolVec{true, true, true});
testTypedValue(mx::FloatVec{1.0f, 2.0f, 3.0f},
mx::FloatVec{4.0f, 5.0f, 6.0f});
testTypedValue(mx::StringVec{"one", "two", "three"},
mx::StringVec{"four", "five", "six"});
testTypedValue(mx::StringVec{"Item A", "Item B", "Item C"},
mx::StringVec{"Item D", "Item E", "Item F"});

// Alias types
testTypedValue<long>(1l, 2l);
Expand Down

0 comments on commit 7b229d1

Please sign in to comment.