Skip to content

Commit

Permalink
Maintain UsdResolveInfo validity across insignificant
Browse files Browse the repository at this point in the history
layer stack changes

UsdResolveInfo previously held on to an index into a
layer stack to indicate the layer containing an attribute
value. It now holds on to a SdfLayerHandle directly to
ensure that additions/removals of empty layers to the
layer stack don't invalidate the object. This means
UsdAttributeQuery objects (which rely on UsdResolveInfo)
will still be valid across such changes.

Fixes #1500

(Internal change: 2161341)
  • Loading branch information
sunyab authored and pixar-oss committed Apr 23, 2021
1 parent 3232d10 commit f8d2081
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 17 deletions.
7 changes: 7 additions & 0 deletions pxr/usd/usd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ pxr_test_scripts(
testenv/testUsdAppliedAPISchemas.py
testenv/testUsdAttributeBlocking.py
testenv/testUsdAttributeConnections.py
testenv/testUsdAttributeQuery.py
testenv/testUsdBugs.py
testenv/testUsdBug119633.py
testenv/testUsdBug141491.py
Expand Down Expand Up @@ -623,6 +624,12 @@ pxr_register_test(testUsdAttributeConnections
EXPECTED_RETURN_CODE 0
)

pxr_register_test(testUsdAttributeQuery
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdAttributeQuery"
EXPECTED_RETURN_CODE 0
)

pxr_register_test(testUsdValueClips
PYTHON
COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdValueClips"
Expand Down
7 changes: 3 additions & 4 deletions pxr/usd/usd/resolveInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ class UsdResolveInfo
public:
UsdResolveInfo()
: _source(UsdResolveInfoSourceNone)
, _layerIndex(std::numeric_limits<size_t>::max())
, _valueIsBlocked(false)
{
}
Expand Down Expand Up @@ -145,12 +144,12 @@ class UsdResolveInfo
/// values.
SdfPath _primPathInLayerStack;

/// The index of the layer in \p layerStack that provides the
/// strongest time sample or default opinion.
/// The layer in \p layerStack that provides the strongest time sample or
/// default opinion.
///
/// This is valid only if \p source is either
/// \p UsdResolveInfoSourceDefault or \p UsdResolveInfoTimeSamples.
size_t _layerIndex;
SdfLayerHandle _layer;

/// If \p source is \p UsdResolveInfoTimeSamples, the time
/// offset that maps time in the strongest resolved layer
Expand Down
20 changes: 7 additions & 13 deletions pxr/usd/usd/stage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7106,8 +7106,7 @@ class UsdStage_ResolveInfoAccess
{
const SdfPath specPath =
info._primPathInLayerStack.AppendProperty(attr.GetName());
const SdfLayerRefPtr& layer =
info._layerStack->GetLayers()[info._layerIndex];
const SdfLayerHandle& layer = info._layer;
const double localTime =
info._layerToStageOffset.GetInverse() * time.GetValue();

Expand Down Expand Up @@ -7229,8 +7228,7 @@ UsdStage::_GetLayerWithStrongestValue(

if (resolveInfo._source == UsdResolveInfoSourceTimeSamples ||
resolveInfo._source == UsdResolveInfoSourceDefault) {
resultLayer =
resolveInfo._layerStack->GetLayers()[resolveInfo._layerIndex];
resultLayer = resolveInfo._layer;
}
else if (resolveInfo._source == UsdResolveInfoSourceValueClips) {
const Usd_ClipSetRefPtr& clipSet = extraResolveInfo.clipSet;
Expand Down Expand Up @@ -7418,7 +7416,7 @@ struct UsdStage::_ResolveInfoResolver

if (_resolveInfo->_source != UsdResolveInfoSourceNone) {
_resolveInfo->_layerStack = nodeLayers;
_resolveInfo->_layerIndex = layerStackPosition;
_resolveInfo->_layer = layer;
_resolveInfo->_primPathInLayerStack = node.GetPath();
_resolveInfo->_layerToStageOffset = layerToStageOffset;
_resolveInfo->_node = node;
Expand Down Expand Up @@ -7602,8 +7600,7 @@ UsdStage::_GetValueFromResolveInfoImpl(const UsdResolveInfo &info,
else if (info._source == UsdResolveInfoSourceDefault) {
const SdfPath specPath =
info._primPathInLayerStack.AppendProperty(attr.GetName());
const SdfLayerHandle& layer =
info._layerStack->GetLayers()[info._layerIndex];
const SdfLayerHandle& layer = info._layer;

TF_DEBUG(USD_VALUE_RESOLUTION).Msg(
"RESOLVE: reading field %s:%s from @%s@, with t = %.3f"
Expand Down Expand Up @@ -7744,8 +7741,7 @@ UsdStage::_GetTimeSamplesInIntervalFromResolveInfo(
if (info._source == UsdResolveInfoSourceTimeSamples) {
const SdfPath specPath =
info._primPathInLayerStack.AppendProperty(attr.GetName());
const SdfLayerRefPtr& layer =
info._layerStack->GetLayers()[info._layerIndex];
const SdfLayerHandle& layer = info._layer;

const std::set<double> samples =
layer->ListTimeSamplesForPath(specPath);
Expand Down Expand Up @@ -7818,8 +7814,7 @@ UsdStage::_GetNumTimeSamplesFromResolveInfo(const UsdResolveInfo &info,
if (info._source == UsdResolveInfoSourceTimeSamples) {
const SdfPath specPath =
info._primPathInLayerStack.AppendProperty(attr.GetName());
const SdfLayerRefPtr& layer =
info._layerStack->GetLayers()[info._layerIndex];
const SdfLayerHandle& layer = info._layer;

return layer->GetNumTimeSamplesForPath(specPath);
}
Expand Down Expand Up @@ -7899,8 +7894,7 @@ UsdStage::_GetBracketingTimeSamplesFromResolveInfo(const UsdResolveInfo &info,
if (info._source == UsdResolveInfoSourceTimeSamples) {
const SdfPath specPath =
info._primPathInLayerStack.AppendProperty(attr.GetName());
const SdfLayerRefPtr& layer =
info._layerStack->GetLayers()[info._layerIndex];
const SdfLayerHandle& layer = info._layer;
const double layerTime =
info._layerToStageOffset.GetInverse() * desiredTime;

Expand Down
104 changes: 104 additions & 0 deletions pxr/usd/usd/testenv/testUsdAttributeQuery.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
#!/pxrpythonsubst
#
# Copyright 2021 Pixar
#
# Licensed under the Apache License, Version 2.0 (the "Apache License")
# with the following modification; you may not use this file except in
# compliance with the Apache License and the following modification to it:
# Section 6. Trademarks. is deleted and replaced with:
#
# 6. Trademarks. This License does not grant permission to use the trade
# names, trademarks, service marks, or product names of the Licensor
# and its affiliates, except as required to comply with Section 4(c) of
# the License and to reproduce the content of the NOTICE file.
#
# You may obtain a copy of the Apache License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the Apache License with the above modification is
# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the Apache License for the specific
# language governing permissions and limitations under the Apache License.

import contextlib
import unittest

from pxr import Usd, Sdf, Tf

@contextlib.contextmanager
def StageChangeListener(stage):
class _Listener(object):
def __init__(self, stage):
self._listener = Tf.Notice.Register(
Usd.Notice.ObjectsChanged, self._HandleNotice, stage)
self.resyncedPrimPaths = []
self.changedInfoPaths = []

def _HandleNotice(self, notice, sender):
self.resyncedPrimPaths = notice.GetResyncedPaths()
self.changedInfoPaths = notice.GetChangedInfoOnlyPaths()

l = _Listener(stage)
yield l

class TestUsdAttributeQuery(unittest.TestCase):
def test_NoInvalidationForInsignificantChange(self):
"""Test that insignificant layer stack changes do not invalidate
an attribute query."""

sublayer = Sdf.Layer.CreateAnonymous("source.usda")
sublayer.ImportFromString('''
#usda 1.0
def "Prim"
{
double attr = 1.0
double attr.timeSamples = {
0.0: 2.0
}
}
'''
.strip())

emptySublayerBefore = Sdf.Layer.CreateAnonymous("empty_before.usda")
emptySublayerAfter = Sdf.Layer.CreateAnonymous("empty_after.usda")

rootLayer = Sdf.Layer.CreateAnonymous("root.usda")
rootLayer.subLayerPaths.insert(0, sublayer.identifier)

stage = Usd.Stage.Open(rootLayer)

query = Usd.AttributeQuery(stage.GetAttributeAtPath("/Prim.attr"))
self.assertTrue(query)
self.assertEqual(query.Get(), 1.0)
self.assertEqual(query.Get(0), 2.0)

# Test that adding and removing empty sublayers before and after the
# sublayer with attribute values does not invalidate the attribute
# query.
@contextlib.contextmanager
def _Validate():
with StageChangeListener(stage) as l:
yield
self.assertEqual(l.resyncedPrimPaths, [])

self.assertTrue(query)
self.assertEqual(query.Get(), 1.0)
self.assertEqual(query.Get(0), 2.0)

with _Validate():
rootLayer.subLayerPaths.insert(0, emptySublayerBefore.identifier)

with _Validate():
rootLayer.subLayerPaths.append(emptySublayerAfter.identifier)

with _Validate():
rootLayer.subLayerPaths.remove(emptySublayerBefore.identifier)

with _Validate():
rootLayer.subLayerPaths.remove(emptySublayerAfter.identifier)

if __name__ == "__main__":
unittest.main()

0 comments on commit f8d2081

Please sign in to comment.