Skip to content

Commit c6e110e

Browse files
sunyabpixar-oss
authored andcommitted
pcp: Fix simplified specializes regression with variants
In certain cases where a specializes arc was introduced inside a variant, the process of adding implied specializes arcs up to the root of the prim index would be cut short causing valid opinions to be omitted. This worked correctly prior to the introduction of simplied specializes. For example, consider the case where the variant node at /A/B{x=y} introduces a specializes arc to /A/C. That arc would be immediately propagated to the root of the graph and have other composition tasks enqueued -- particularly the implied class task. Pcp would try to add the corresponding implied arc to the parent of the variant node at /A/B. The computed implied specializes path would be /A/C, so an inert placeholder node would be added at the path and would then be immediately propagated to the root. However, since there was already a propagated specializes arc at /A/C, no node would be added and no tasks enqueued, which would cause the implied class process to stop. This change detects this situation and ensures an implied class task is always enqueued to ensure the process continues. While this fixes the reported regression, it introduces a separate regression in an internal test. This second regression involves a much more complicated composition structure that we think is less likely, so I'm landing this fix first to address the more common case for the 25.08 release. This change will be reverted afterwards while work on a more complete fix is underway. See #3750 (Internal change: 2374321)
1 parent 841d909 commit c6e110e

File tree

4 files changed

+170
-2
lines changed

4 files changed

+170
-2
lines changed

pxr/usd/pcp/CMakeLists.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,11 @@ pxr_install_test_dir(
592592
DEST testPcpMuseum_SpecializesAndVariants3
593593
)
594594

595+
pxr_install_test_dir(
596+
SRC testenv/testPcpMuseum_SpecializesAndVariants4.testenv
597+
DEST testPcpMuseum_SpecializesAndVariants4
598+
)
599+
595600
pxr_install_test_dir(
596601
SRC testenv/testPcpMuseum_SubrootInheritsAndVariants.testenv
597602
DEST testPcpMuseum_SubrootInheritsAndVariants
@@ -1431,6 +1436,13 @@ pxr_register_test(testPcpMuseum_SpecializesAndVariants3
14311436
DIFF_COMPARE compositionResults_SpecializesAndVariants3.txt
14321437
)
14331438

1439+
pxr_register_test(testPcpMuseum_SpecializesAndVariants4
1440+
PYTHON
1441+
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testPcpCompositionResults --usd SpecializesAndVariants4/root.usda"
1442+
STDOUT_REDIRECT compositionResults_SpecializesAndVariants4.txt
1443+
DIFF_COMPARE compositionResults_SpecializesAndVariants4.txt
1444+
)
1445+
14341446
pxr_register_test(testPcpMuseum_SubrootInheritsAndVariants
14351447
PYTHON
14361448
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testPcpCompositionResults --usd SubrootInheritsAndVariants/root.usda"

pxr/usd/pcp/primIndex.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3267,11 +3267,29 @@ _AddClassBasedArc(
32673267
// EvalImpliedSpecializes task.
32683268
//
32693269
// If we're _not_ in a recursive prim indexing call we can
3270-
// immediately do the propagation to avoid going the task
3270+
// immediately do the propagation to avoid paying the task
32713271
// overhead. See also _FindSpecializesToPropagateToRoot.
32723272
if (!indexer->previousFrame && placeholder &&
32733273
!_IsRelocatesPlaceholderImpliedArc(placeholder)) {
3274-
return _PropagateNodeToRoot(placeholder, indexer);
3274+
3275+
PcpNodeRef propagatedNode =
3276+
_PropagateNodeToRoot(placeholder, indexer);
3277+
3278+
// If a new node was created to propagate the placeholder
3279+
// to the root, tasks will have been enqueued to continue
3280+
// implying the class to the root of the prim index.
3281+
//
3282+
// If a pre-existing node was found instead (i.e., the
3283+
// returned node's origin isn't the placeholder), we need
3284+
// to manually enqueue tasks on the placeholder to continue
3285+
// that process.
3286+
if (propagatedNode &&
3287+
propagatedNode.GetOriginNode() != placeholder) {
3288+
indexer->AddTasksForNode(
3289+
placeholder, Task::EvalImpliedClasses);
3290+
}
3291+
3292+
return propagatedNode;
32753293
}
32763294

32773295
return placeholder;
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#usda 1.0
2+
(
3+
"""
4+
Museum case showing combinations of specializes and variants.
5+
6+
In this example, when composing /A/render, a specializes arc to
7+
/_class_/defaultImplementation is introduced within a variant.
8+
That specializes arc is propagated to the root of the graph and
9+
must also add implied specializes arcs to its parent nodes;
10+
in particular, there should be an implied specializes arc added
11+
for /A/defaultImplementation.
12+
"""
13+
)
14+
15+
class "_class_"
16+
{
17+
class "defaultImplementation"
18+
{
19+
uniform string test = "value from /_class_/defaultImplementation"
20+
}
21+
22+
def "render" (
23+
variants = {
24+
string implementation = "default"
25+
}
26+
prepend variantSets = "implementation"
27+
)
28+
{
29+
variantSet "implementation" = {
30+
"default" (
31+
prepend specializes = </_class_/defaultImplementation>
32+
) {
33+
34+
}
35+
}
36+
}
37+
}
38+
39+
def "A" (
40+
prepend specializes = </_class_>
41+
)
42+
{
43+
over "defaultImplementation"
44+
{
45+
uniform string test = "value from /A/defaultImplementation"
46+
}
47+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
Loading @SpecializesAndVariants4/root.usda@
2+
3+
------------------------------------------------------------------------
4+
Layer Stack:
5+
root.usda
6+
7+
------------------------------------------------------------------------
8+
Results for composing </_class_>
9+
10+
Prim Stack:
11+
root.usda /_class_
12+
13+
Child names:
14+
['defaultImplementation', 'render']
15+
16+
------------------------------------------------------------------------
17+
Results for composing </_class_/defaultImplementation>
18+
19+
Prim Stack:
20+
root.usda /_class_/defaultImplementation
21+
22+
Property names:
23+
['test']
24+
25+
Property stacks:
26+
/_class_/defaultImplementation.test:
27+
root.usda /_class_/defaultImplementation.test
28+
29+
------------------------------------------------------------------------
30+
Results for composing </_class_/render>
31+
32+
Prim Stack:
33+
root.usda /_class_/render
34+
root.usda /_class_/render{implementation=default}
35+
root.usda /_class_/defaultImplementation
36+
37+
Variant Selections:
38+
{implementation = default}
39+
40+
Property names:
41+
['test']
42+
43+
Property stacks:
44+
/_class_/render.test:
45+
root.usda /_class_/defaultImplementation.test
46+
47+
------------------------------------------------------------------------
48+
Results for composing </A>
49+
50+
Prim Stack:
51+
root.usda /A
52+
root.usda /_class_
53+
54+
Child names:
55+
['defaultImplementation', 'render']
56+
57+
------------------------------------------------------------------------
58+
Results for composing </A/defaultImplementation>
59+
60+
Prim Stack:
61+
root.usda /A/defaultImplementation
62+
root.usda /_class_/defaultImplementation
63+
64+
Property names:
65+
['test']
66+
67+
Property stacks:
68+
/A/defaultImplementation.test:
69+
root.usda /A/defaultImplementation.test
70+
root.usda /_class_/defaultImplementation.test
71+
72+
------------------------------------------------------------------------
73+
Results for composing </A/render>
74+
75+
Prim Stack:
76+
root.usda /_class_/render
77+
root.usda /_class_/render{implementation=default}
78+
root.usda /A/defaultImplementation
79+
root.usda /_class_/defaultImplementation
80+
81+
Variant Selections:
82+
{implementation = default}
83+
84+
Property names:
85+
['test']
86+
87+
Property stacks:
88+
/A/render.test:
89+
root.usda /A/defaultImplementation.test
90+
root.usda /_class_/defaultImplementation.test
91+

0 commit comments

Comments
 (0)