Skip to content

Commit fa8e2b2

Browse files
tallytalwarpixar-oss
authored andcommitted
Deprecate negative offset scale during composition
Cummulative negative layer offset scale on a composed layer doesn't make sense, as it reverses the direction of time, and can lead to incorrect and non-intuitive results specially with various spline evaluation behaviors and with time samples with soon to be implemented preValue support. Introduced PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE environment variable, which currently defaults to true, allowing the use of negative layer offset, however will issue a warning. A false value, will result in a coding error. (Internal change: 2357711)
1 parent e104b33 commit fa8e2b2

File tree

5 files changed

+135
-3
lines changed

5 files changed

+135
-3
lines changed

pxr/usd/pcp/CMakeLists.txt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ pxr_test_scripts(
119119
testenv/testPcpResolvedPathChange.py
120120
testenv/testPcpStreamingLayerReload.py
121121
testenv/testPcpCompositionResults.py
122+
testenv/testPcpNegativeOffsetScale.py
122123
)
123124

124125
pxr_build_test_shared_lib(TestPcpDynamicFileFormatPlugin
@@ -2162,3 +2163,17 @@ pxr_register_test(testPcpLayerRelocatesEditBuilder
21622163
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testPcpLayerRelocatesEditBuilder"
21632164
EXPECTED_RETURN_CODE 0
21642165
)
2166+
2167+
pxr_register_test(testPcpNegativeOffsetScaleAllowed
2168+
PYTHON
2169+
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testPcpNegativeOffsetScale"
2170+
EXPECTED_RETURN_CODE 0
2171+
)
2172+
2173+
pxr_register_test(testPcpNegativeOffsetScale
2174+
PYTHON
2175+
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testPcpNegativeOffsetScale"
2176+
EXPECTED_RETURN_CODE 0
2177+
ENV
2178+
PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE=0
2179+
)

pxr/usd/pcp/layerStack.cpp

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ TF_DEFINE_ENV_SETTING(
5252
"non-USD caches/layer stacks; the legacy behavior cannot be enabled in USD "
5353
"mode");
5454

55+
TF_DEFINE_ENV_SETTING(
56+
PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE, true,
57+
"Enables the use of negative layer offset scale. This behavior is "
58+
"deprecated and a warning will be issued if this setting is enabled, "
59+
"otherwise a coding error will be issued. Negative layer offset scale on a "
60+
"composed property doesn't make sense, as it reverses the direction of "
61+
"time, and can lead to incorrect and non intuitive results.");
62+
63+
5564
struct Pcp_SublayerInfo {
5665
Pcp_SublayerInfo() = default;
5766
Pcp_SublayerInfo(const SdfLayerRefPtr& layer_, const SdfLayerOffset& offset_,
@@ -1649,6 +1658,14 @@ PcpLayerStack::_Compute(const std::string &fileFormatTarget,
16491658
}
16501659
}
16511660

1661+
bool
1662+
PcpNegativeLayerOffsetScaleAllowed()
1663+
{
1664+
static bool allowed =
1665+
TfGetEnvSetting(PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE);
1666+
return allowed;
1667+
}
1668+
16521669
SdfLayerTreeHandle
16531670
PcpLayerStack::_BuildLayerStack(
16541671
const SdfLayerHandle & layer,
@@ -1786,8 +1803,20 @@ PcpLayerStack::_BuildLayerStack(
17861803

17871804
// Check sublayer offset.
17881805
SdfLayerOffset sublayerOffset = sublayerOffsets[i];
1789-
if (!sublayerOffset.IsValid()
1790-
|| !sublayerOffset.GetInverse().IsValid()) {
1806+
1807+
const bool isNegativeScale = sublayerOffset.GetScale() < 0.0;
1808+
const bool negativeScaleAllowed = PcpNegativeLayerOffsetScaleAllowed();
1809+
1810+
if (isNegativeScale && negativeScaleAllowed) {
1811+
// Report warning.
1812+
TF_WARN("Layer @%s@ has a negative offset scale. Negative scale "
1813+
"offsets are deprecated.",
1814+
layer->GetIdentifier().c_str());
1815+
}
1816+
1817+
if ((isNegativeScale && !negativeScaleAllowed) ||
1818+
!sublayerOffset.IsValid() ||
1819+
!sublayerOffset.GetInverse().IsValid()) {
17911820
// Report error, but continue with an identity layer offset.
17921821
PcpErrorInvalidSublayerOffsetPtr err =
17931822
PcpErrorInvalidSublayerOffset::New();

pxr/usd/pcp/layerStack.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,16 @@ std::ostream& operator<<(std::ostream&, const PcpLayerStackPtr&);
338338
PCP_API
339339
std::ostream& operator<<(std::ostream&, const PcpLayerStackRefPtr&);
340340

341+
/// Returns true if negative layer offsets and scales are allowed.
342+
///
343+
/// Negative layer offset scales are deprecated and a warning will be issued
344+
/// when commulative scale during composition is negative with
345+
/// PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE is set to true (default right now).
346+
/// If PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE is set to false, a coding error
347+
/// will be issued when a negative scale is encountered.
348+
PCP_API
349+
bool PcpNegativeLayerOffsetScaleAllowed();
350+
341351
/// Checks if the source and target paths constitute a valid relocates. This
342352
/// validation is not context specific, i.e. if this returns false, the
343353
/// combination of source and target paths is always invalid for any attempted

pxr/usd/pcp/primIndex.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2123,8 +2123,20 @@ _EvalRefOrPayloadArcs(PcpNodeRef node,
21232123
fail = true;
21242124
}
21252125

2126+
const bool isNegativeScale = layerOffset.GetScale() < 0.0;
2127+
const bool negativeScaleAllowed = PcpNegativeLayerOffsetScaleAllowed();
2128+
2129+
if (isNegativeScale && negativeScaleAllowed) {
2130+
TF_WARN("Found negative scale in layer offset for %s to @%s@<%s>. "
2131+
"Negative offset scale is deprecated.",
2132+
ARC_TYPE == PcpArcTypePayload ? "payload" : "reference",
2133+
info.authoredAssetPath.c_str(),
2134+
refOrPayload.GetPrimPath().GetText());
2135+
}
2136+
21262137
// Validate layer offset in original reference or payload.
2127-
if (!layerOffset.IsValid() ||
2138+
if ((isNegativeScale && !negativeScaleAllowed) ||
2139+
!layerOffset.IsValid() ||
21282140
!layerOffset.GetInverse().IsValid()) {
21292141
PcpErrorInvalidReferenceOffsetPtr err =
21302142
PcpErrorInvalidReferenceOffset::New();
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
#!/pxrpythonsubst
2+
#
3+
# Copyright 2025 Pixar
4+
#
5+
# Licensed under the terms set forth in the LICENSE.txt file available at
6+
7+
import unittest
8+
9+
from pxr import Pcp, Sdf, Tf
10+
11+
def _ComposeLayersWithNegativeOffsetScale():
12+
refLayer = Sdf.Layer.CreateAnonymous("ref")
13+
refLayer.ImportFromString('''
14+
#sdf 1.4.32
15+
16+
def "prim" {
17+
double attr.timeSamples = {
18+
0: 1.0,
19+
1: 2.0,
20+
}
21+
}
22+
'''.strip())
23+
24+
subLayer = Sdf.Layer.CreateAnonymous("sub")
25+
subLayer.ImportFromString(f'''
26+
#sdf 1.4.32
27+
(
28+
subLayers = [
29+
@{refLayer.identifier}@ (scale = -1)
30+
]
31+
)
32+
'''.strip())
33+
34+
rootLayer = Sdf.Layer.CreateAnonymous()
35+
rootLayer.ImportFromString(f'''
36+
#sdf 1.4.32
37+
38+
def "prim" (
39+
references = @{subLayer.identifier}@</prim> (scale = -1)
40+
)
41+
{{
42+
}}
43+
'''.strip())
44+
45+
pcpCache = Pcp.Cache(Pcp.LayerStackIdentifier(rootLayer))
46+
_, errs = pcpCache.ComputePrimIndex("/prim")
47+
return errs
48+
49+
class TestPcpNegativeLayerOffsetScale(unittest.TestCase):
50+
51+
# Following will not result in a composition error
52+
@unittest.skipIf(not Tf.GetEnvSetting("PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE"),
53+
"Allow negative layer offset scale, no composition error")
54+
def test_NegativeLayerOffsetScaleAllowed(self):
55+
errs = _ComposeLayersWithNegativeOffsetScale()
56+
self.assertEqual(len(errs), 0)
57+
58+
# Following will result in a composition error
59+
@unittest.skipIf(Tf.GetEnvSetting("PCP_ALLOW_NEGATIVE_LAYER_OFFSET_SCALE"),
60+
"Do not allow negative layer offset scale, composition error")
61+
def test_NegativeLayerOffsetScaleNotAllowed(self):
62+
errs = _ComposeLayersWithNegativeOffsetScale()
63+
self.assertEqual(len(errs), 2)
64+
65+
if __name__ == "__main__":
66+
unittest.main()

0 commit comments

Comments
 (0)