Fix subdivision when primvars are set in parent primitives (#1983)

* Fix subdivision when primvars are set in parent primitives #1982 * Add changelog
Autodesk · Jul 15, 2024 · c3cecca · c3cecca
1 parent 0e1fd53
commit c3cecca
Show file tree

Hide file tree

Showing 7 changed files with 234 additions and 252 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 - [usd#1972](https://github.com/Autodesk/arnold-usd/issues/1972) - Delegate should only create default shaders when needed
 - [usd#1946](https://github.com/Autodesk/arnold-usd/issues/1946) - Support color space in materialx for hydra
 - [usd#1972](https://github.com/Autodesk/arnold-usd/issues/1972) - Ensure subdivision is disabled when subdiv iterations is equal to 0 in Hydra
+- [usd#1982](https://github.com/Autodesk/arnold-usd/issues/1982) - Fix subdivision when primvars are set in parent primitives
 - [usd#1979](https://github.com/Autodesk/arnold-usd/issues/1979) - Support treatAsPoint in Hydra photometric lights
 
 ### Bug fixes

diff --git a/libs/translator/reader/read_geometry.cpp b/libs/translator/reader/read_geometry.cpp
@@ -256,9 +256,6 @@ AtNode* UsdArnoldReadMesh::Read(const UsdPrim &prim, UsdArnoldReaderContext &con
 
     AtNode *node = context.CreateArnoldNode("polymesh", prim.GetPath().GetText());
     ReadMatrix(prim, node, time, context);
-    if (!HasAuthoredAttribute(prim, str::t_primvars_arnold_smoothing))
-        AiNodeSetBool(node, str::smoothing, true);
-
     // Get mesh.
     UsdGeomMesh mesh(prim);
 
@@ -397,7 +394,7 @@ AtNode* UsdArnoldReadMesh::Read(const UsdPrim &prim, UsdArnoldReaderContext &con
 
     // reset subdiv_iterations to 0, unless primvars:arnold:subdiv_iterations is 
     // explicitely set, in which case it would be applied during readArnoldParameter #1929
-    if (!HasAuthoredAttribute(prim, str::t_primvars_arnold_subdiv_iterations))
+    if (!HasConstantPrimvar(context, str::t_primvars_arnold_subdiv_iterations))
         AiNodeSetByte(node, str::subdiv_iterations, 0);
 
     MeshPrimvarsRemapper primvarsRemapper(meshOrientation);
@@ -438,7 +435,7 @@ AtNode* UsdArnoldReadMesh::Read(const UsdPrim &prim, UsdArnoldReaderContext &con
     // we get smoothed normals by default.
     // Also, we only read the builtin subdivisionScheme if the arnold
     // attribute wasn't explcitely set above, through primvars:arnold (see #679)
-    if ((!HasAuthoredAttribute(prim, str::t_primvars_arnold_subdiv_type)) &&
+    if ((!HasConstantPrimvar(context, str::t_primvars_arnold_subdiv_type)) &&
             (AiNodeGetByte(node, str::subdiv_iterations) > 0)) {
         if (subdiv == UsdGeomTokens->none)
             AiNodeSetStr(node, str::subdiv_type, str::none);
@@ -672,7 +669,7 @@ AtNode* UsdArnoldReadCube::Read(const UsdPrim &prim, UsdArnoldReaderContext &con
     float frame = time.frame;
     AtNode *node = context.CreateArnoldNode("polymesh", prim.GetPath().GetText());
     ReadMatrix(prim, node, time, context);
-    if (!HasAuthoredAttribute(prim, str::t_primvars_arnold_smoothing))
+    if (!HasConstantPrimvar(context, str::t_primvars_arnold_smoothing))
         AiNodeSetBool(node, str::smoothing, false);
 
     static const VtIntArray numVerts { 4, 4, 4, 4, 4, 4 };
@@ -725,8 +722,6 @@ AtNode* UsdArnoldReadSphere::Read(const UsdPrim &prim, UsdArnoldReaderContext &c
     float frame = time.frame;
     AtNode *node = context.CreateArnoldNode("polymesh", prim.GetPath().GetText());
     ReadMatrix(prim, node, time, context);
-    if (!HasAuthoredAttribute(prim, str::t_primvars_arnold_smoothing))
-        AiNodeSetBool(node, str::smoothing, true);
 
     static const VtIntArray numVerts{
         3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 
@@ -895,9 +890,7 @@ AtNode* UsdArnoldReadCylinder::Read(const UsdPrim &prim, UsdArnoldReaderContext
     float frame = time.frame;
     AtNode *node = context.CreateArnoldNode("polymesh", prim.GetPath().GetText());
     ReadMatrix(prim, node, time, context);
-    if (!HasAuthoredAttribute(prim, str::t_primvars_arnold_smoothing))
-        AiNodeSetBool(node, str::smoothing, true);
-
+
     static const VtIntArray numVerts{ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
                                       4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
                                       3, 3, 3, 3, 3, 3, 3, 3, 3, 3 };
@@ -963,9 +956,6 @@ AtNode* UsdArnoldReadCone::Read(const UsdPrim &prim, UsdArnoldReaderContext &con
     AtNode *node = context.CreateArnoldNode("polymesh", prim.GetPath().GetText());
     ReadMatrix(prim, node, time, context);
 
-    if (!HasAuthoredAttribute(prim, str::t_primvars_arnold_smoothing))
-        AiNodeSetBool(node, str::smoothing, true);
-
     static const VtIntArray numVerts{ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
                                       4, 4, 4, 4, 4, 4, 4, 4, 4, 4 };
     static const VtIntArray verts{
@@ -1023,9 +1013,6 @@ AtNode* UsdArnoldReadCapsule::Read(const UsdPrim &prim, UsdArnoldReaderContext &
     AtNode *node = context.CreateArnoldNode("polymesh", prim.GetPath().GetText());
     ReadMatrix(prim, node, time, context);
 
-    if (!HasAuthoredAttribute(prim, str::t_primvars_arnold_smoothing))
-        AiNodeSetBool(node, str::smoothing, true);
-
     // slices are segments around the mesh
     static constexpr int _capsuleSlices = 10;
     // stacks are segments along the spine axis

diff --git a/libs/translator/reader/utils.cpp b/libs/translator/reader/utils.cpp
@@ -52,6 +52,15 @@ TF_DEFINE_PRIVATE_TOKENS(
     ((PrimvarsArnoldDeformKeys, "primvars:arnold:deform_keys"))
 );
 
+bool HasConstantPrimvar(UsdArnoldReaderContext &context, const TfToken& name)
+{
+    const std::vector<UsdGeomPrimvar>& primvars = context.GetPrimvars();
+    for (const auto& primvar : primvars) {
+        if (primvar.GetName() == name)
+            return true;
+    }
+    return false;
+}
 
 static inline void getMatrix(const UsdPrim &prim, AtMatrix &matrix, float frame, 
     UsdArnoldReaderContext &context, bool isXformable = true)

diff --git a/libs/translator/reader/utils.h b/libs/translator/reader/utils.h
@@ -70,7 +70,7 @@ AtArray *ReadMatrix(const UsdPrim& prim, const TimeSettings& time,
 
 AtArray *ReadLocalMatrix(const UsdPrim &prim, const TimeSettings &time);
 
-
+bool HasConstantPrimvar(UsdArnoldReaderContext &context, const TfToken& name);
 
 size_t ReadTopology(
     UsdAttribute& usdAttr, AtNode* node, const char* attrName, const TimeSettings& time, UsdArnoldReaderContext &context);

diff --git a/testsuite/groups b/testsuite/groups
@@ -41,8 +41,7 @@ darwin:
 #######################
 # Tests that can be run in hydra mode
 
-hydra: test_0000 test_0001 test_0002 test_0003 test_0004 test_0005 test_0006 test_0007 test_0008 test_0009 test_0010 test_0011 test_0012 test_0014 test_0016 test_0017 test_0018 test_0019 test_0020 test_0021 test_0022 test_0023 test_0024 test_0025 test_0026 test_0027 test_0028 test_0029 test_0030 test_0031 test_0032 test_0033 test_0034 test_0037 test_0038 test_0041 test_0046 test_0048 test_0049 test_0050 test_0051 test_0052 test_0053 test_0055 test_0056 test_0057 test_0058 test_0059 test_0060 test_0062 test_0064 test_0066 test_0067 test_0068 test_0071 test_0072 test_0073 test_0074 test_0076 test_0077 test_0080 test_0081 test_0082 test_0083 test_0084 test_0086 test_0087 test_0088 test_0091 test_0093 test_0094 test_0097 test_0099 test_0104 test_0105 test_0107 test_0108 test_0110 test_0111 test_0112 test_0113 test_0114 test_0115 test_0117 test_0120 test_0121 test_0122 test_0123 test_0125 test_0126 test_0127 test_0128 test_0129 test_0130 test_0132 test_0133 test_0135 test_0139 test_0140 test_0141 test_0142 test_0143 test_0144 test_0145 test_0148 test_0149 test_0150 test_0151 test_0158 test_0159 test_0160 test_0161 test_0162 test_0163 test_0164 test_0165 test_0166 test_0168 test_0169 test_0170 test_0171 test_0172 test_0173 test_0174 test_0175 test_0177 test_0178 test_0179 test_0180 test_0183 test_0184 test_0186 test_0187 test_0188 test_0189 test_0191 test_0194 test_0195 test_0196 test_0197 test_0198 test_0200 test_0201 test_0202 test_0204 test_0205 test_0206 test_0207 test_0209 test_0210 test_0215 test_0216 test_0217 test_0218 test_0219 test_0220 test_0221 test_0222 test_0223 test_0225 test_0226 test_0227 test_0228 test_0229 test_0230 test_0231 test_0232 test_0233 test_0234 test_0238 test_0239 test_0240 test_0242 test_0243 test_0244 test_0245 test_0299 test_0316 test_0317 test_0739 test_1181 test_1204 test_1209 test_1223 test_1225 test_1237 test_1238 test_1245 test_1262 test_1282 test_1294 test_1309 test_1311 test_1313 test_1316 test_1329 test_1333 test_1334 test_1346 test_1354 test_1366 test_1383 test_1416 test_1420 test_1426 test_1427.1 test_1427.2 test_1427.3 test_1430 test_1433 test_1435 test_1438 test_1442 test_1447 test_1457 test_1476 test_1485 test_1486 test_1499 test_1521 test_1524 test_1525 test_1530 test_1535 test_1538 test_1546 test_1547.1 test_1547.2 test_1550 test_1567 test_1588 test_1590 test_1593 test_1607 test_1613 test_1625 test_1632 test_1635 test_1654 test_1657 test_1678 test_1705 test_1718 test_1726 test_1730 test_1735 test_1769 test_1772 test_1776 test_1814 test_1868 test_1873 test_1878 test_1881 test_1894 test_1906 test_1923 test_1939 test_1940 test_14028
-
+hydra: test_0000 test_0001 test_0002 test_0003 test_0004 test_0005 test_0006 test_0007 test_0008 test_0009 test_0010 test_0011 test_0012 test_0014 test_0016 test_0017 test_0018 test_0019 test_0020 test_0021 test_0022 test_0023 test_0024 test_0025 test_0026 test_0027 test_0028 test_0029 test_0030 test_0031 test_0032 test_0033 test_0034 test_0037 test_0038 test_0041 test_0046 test_0048 test_0049 test_0050 test_0051 test_0052 test_0053 test_0055 test_0056 test_0057 test_0058 test_0059 test_0060 test_0062 test_0064 test_0066 test_0067 test_0068 test_0071 test_0072 test_0073 test_0074 test_0076 test_0077 test_0080 test_0081 test_0082 test_0083 test_0084 test_0086 test_0087 test_0088 test_0091 test_0093 test_0094 test_0097 test_0099 test_0104 test_0105 test_0107 test_0108 test_0110 test_0111 test_0112 test_0113 test_0114 test_0115 test_0117 test_0120 test_0121 test_0122 test_0123 test_0125 test_0126 test_0127 test_0128 test_0129 test_0130 test_0132 test_0133 test_0135 test_0139 test_0140 test_0141 test_0142 test_0143 test_0144 test_0145 test_0148 test_0149 test_0150 test_0151 test_0158 test_0159 test_0160 test_0161 test_0162 test_0163 test_0164 test_0165 test_0166 test_0168 test_0169 test_0170 test_0171 test_0172 test_0173 test_0174 test_0175 test_0177 test_0178 test_0179 test_0180 test_0183 test_0184 test_0186 test_0187 test_0188 test_0189 test_0191 test_0194 test_0195 test_0196 test_0197 test_0198 test_0200 test_0201 test_0202 test_0204 test_0205 test_0206 test_0207 test_0209 test_0210 test_0215 test_0216 test_0217 test_0218 test_0219 test_0220 test_0221 test_0222 test_0223 test_0225 test_0226 test_0227 test_0228 test_0229 test_0230 test_0231 test_0232 test_0233 test_0234 test_0238 test_0239 test_0240 test_0242 test_0243 test_0244 test_0245 test_0299 test_0316 test_0317 test_0739 test_1181 test_1204 test_1209 test_1223 test_1225 test_1232 test_1237 test_1238 test_1245 test_1262 test_1282 test_1294 test_1309 test_1311 test_1313 test_1316 test_1329 test_1333 test_1334 test_1346 test_1354 test_1366 test_1383 test_1416 test_1420 test_1426 test_1427.1 test_1427.2 test_1427.3 test_1430 test_1433 test_1435 test_1438 test_1442 test_1447 test_1457 test_1476 test_1485 test_1486 test_1499 test_1521 test_1524 test_1525 test_1530 test_1535 test_1538 test_1546 test_1547.1 test_1547.2 test_1550 test_1567 test_1588 test_1590 test_1593 test_1607 test_1613 test_1625 test_1632 test_1635 test_1654 test_1657 test_1678 test_1705 test_1718 test_1726 test_1730 test_1735 test_1769 test_1772 test_1776 test_1814 test_1868 test_1873 test_1878 test_1881 test_1894 test_1906 test_1923 test_1939 test_1940 test_14028
 
 # Tests in this group will never be executed (you can use it to temporarily disable some tests and/or groups)
 ignore:  

diff --git a/testsuite/test_1232/ref/reference.exr b/testsuite/test_1232/ref/reference.exr