Skip to content

Commit

Permalink
Better handling of layer resolved path changes
Browse files Browse the repository at this point in the history 
Calling PcpCache::Reload or ReloadReferences after
a layer's resolved path had changed would cause any
prims depending on asset paths relative to that
resolved path to be recomposed. However, directly
reloading the affected layer via SdfLayer::Reload did
not trigger this recomposition. It does now.

SdfLayer now registers a change when its resolved
path has changed. Pcp uses this to detect when it
needs to recompose affected prims as part of its
normal change processing. This means the PcpCache
methods above no longer need to handle this case
specially and can be reverted to their original
behavior.

(Internal change: 1733305)
  • Loading branch information
@sunyab @pixar-oss
sunyab authored and pixar-oss committed Apr 6, 2017
1 parent 9438cf2 commit d0a29f8
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 127 deletions.
76 changes: 2 additions & 74 deletions pxr/usd/lib/pcp/cache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -985,56 +985,6 @@ PcpCache::Apply(const PcpCacheChanges& changes, PcpLifeboat* lifeboat)
_includedPayloads.insert(newIncludes.begin(), newIncludes.end());
}

// Scan given prim indexes and layer stacks and register significant
// changes for anything that needs to be recomputed due to asset path
// changes. This is potentially expensive, but the alternative
// is to store extra information when computing prim indexes, which
// would impact composition/load times and memory usage.
template <class PrimIndexIter, class LayerStackIter>
static void
_RegisterChangesForAssetPathChanges(
PcpCache* cache, PcpChanges* changes,
PrimIndexIter beginPrimIndex, PrimIndexIter endPrimIndex,
LayerStackIter beginLayerStack, LayerStackIter endLayerStack)
{
tbb::concurrent_vector<SdfPath> pathsToResync;
tbb::concurrent_vector<PcpLayerStackPtr> layerStacksToResync;

WorkDispatcher dispatcher;
dispatcher.Run(
[&pathsToResync, beginPrimIndex, endPrimIndex]() {
WorkParallelForEach(
beginPrimIndex, endPrimIndex,
[&pathsToResync](const std::pair<SdfPath, PcpPrimIndex>& e) {
if (e.second.IsValid() &&
Pcp_NeedToRecomputeDueToAssetPathChange(e.second)) {
pathsToResync.push_back(e.first);
}
});
});

dispatcher.Run(
[&layerStacksToResync, beginLayerStack, endLayerStack]() {
WorkParallelForEach(
beginLayerStack, endLayerStack,
[&layerStacksToResync](const PcpLayerStackPtr& layerStack) {
if (Pcp_NeedToRecomputeDueToAssetPathChange(layerStack)) {
layerStacksToResync.push_back(layerStack);
}
});
});

dispatcher.Wait();

for (const SdfPath& path : pathsToResync) {
changes->DidChangeSignificantly(cache, path);
}

for (const PcpLayerStackPtr& layerStack : layerStacksToResync) {
changes->DidChangeLayerStack(cache, layerStack);
}
}

void
PcpCache::Reload(PcpChanges* changes)
{
Expand Down Expand Up @@ -1078,27 +1028,14 @@ PcpCache::Reload(PcpChanges* changes)
}

// Reload every layer we've reached except the session layers (which we
// never want to reload from disk). Hold notifications until the end
// of this function so that clients don't alter the state of the PcpCache
// before the subsequent code is run.
SdfChangeBlock changeBlock;
// never want to reload from disk).
SdfLayerHandleSet layersToReload = GetUsedLayers();

for (const SdfLayerHandle &layer : _layerStack->GetSessionLayers()) {
layersToReload.erase(layer);
}

SdfLayer::ReloadLayers(layersToReload);

// Find prim indexes and layer stacks that need to be recomputed
// due to asset path changes resulting from the reload above,
// but that aren't due to scene description changes. This ensures
// that we reload anything that may be affected by external state
// changes that aren't visible to Pcp but affect asset path resolution.
_RegisterChangesForAssetPathChanges(
this, changes,
_primIndexCache.begin(), _primIndexCache.end(),
allLayerStacks.begin(), allLayerStacks.end());
}

void
Expand Down Expand Up @@ -1146,10 +1083,7 @@ PcpCache::ReloadReferences(PcpChanges* changes, const SdfPath& primPath)
}

// Reload every layer used by prims at or under primPath, except for
// local layers. Hold notifications until the end of this function so
// that clients don't alter the state of the PcpCache before the
// subsequent code is run.
SdfChangeBlock changeBlock;
// local layers.
SdfLayerHandleSet layersToReload;
for (const PcpLayerStackPtr& layerStack: layerStacksAtOrUnderPrim) {
for (const SdfLayerHandle& layer: layerStack->GetLayers()) {
Expand All @@ -1160,12 +1094,6 @@ PcpCache::ReloadReferences(PcpChanges* changes, const SdfPath& primPath)
}

SdfLayer::ReloadLayers(layersToReload);

// See comments in Reload.
_RegisterChangesForAssetPathChanges(
this, changes,
range.first, range.second,
layerStacksAtOrUnderPrim.begin(), layerStacksAtOrUnderPrim.end());
}

void
Expand Down
119 changes: 74 additions & 45 deletions pxr/usd/lib/pcp/changes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,11 @@ PcpChanges::DidChange(const std::vector<PcpCache*>& caches,
const SdfLayerChangeListMap& changes)
{
// LayerStack changes
static const int LayerStackLayersChange = 1;
static const int LayerStackOffsetsChange = 2;
static const int LayerStackRelocatesChange = 4;
static const int LayerStackSignificantChange = 8;
static const int LayerStackLayersChange = 1;
static const int LayerStackOffsetsChange = 2;
static const int LayerStackRelocatesChange = 4;
static const int LayerStackSignificantChange = 8;
static const int LayerStackResolvedPathChange = 16;
typedef int LayerStackChangeBitmask;
typedef std::map<PcpLayerStackPtr, LayerStackChangeBitmask>
LayerStackChangeMap;
Expand Down Expand Up @@ -395,7 +396,7 @@ PcpChanges::DidChange(const std::vector<PcpCache*>& caches,
PcpLayerStackPtrVector stacks =
cache->FindAllLayerStacksUsingLayer(layer);
if (!stacks.empty()) {
cacheLayerStacks.push_back(std::make_pair( cache, stacks) );
cacheLayerStacks.emplace_back(cache, std::move(stacks));
}
}
if (cacheLayerStacks.empty()) {
Expand Down Expand Up @@ -553,6 +554,10 @@ PcpChanges::DidChange(const std::vector<PcpCache*>& caches,
}
break;
}

if (entry.flags.didChangeResolvedPath) {
layerStackChangeMask |= LayerStackResolvedPathChange;
}
}

// Handle changes that require a prim graph change.
Expand Down Expand Up @@ -864,17 +869,27 @@ PcpChanges::DidChange(const std::vector<PcpCache*>& caches,

// Process layer stack changes. This will handle both blowing
// caches (as needed) for the layer stack contents and offsets,
// as well as analyzing relocation changes in the layer stack.
TF_FOR_ALL(layerStackChange, layerStackChangesMap) {
_DidChangeLayerStack(
layerStackChange->first,
layerStackChange->second & LayerStackLayersChange,
layerStackChange->second & LayerStackOffsetsChange,
layerStackChange->second & LayerStackSignificantChange);
if (layerStackChange->second & LayerStackRelocatesChange) {
_DidChangeLayerStackRelocations(caches, layerStackChange->first,
debugSummary);
// as well as analyzing relocation changes in the layer stack.
for (const auto& entry : layerStackChangesMap) {
const PcpLayerStackPtr& layerStack = entry.first;
LayerStackChangeBitmask layerStackChanges = entry.second;

if (layerStackChanges & LayerStackResolvedPathChange) {
_DidChangeLayerStackResolvedPath(caches, layerStack, debugSummary);
if (Pcp_NeedToRecomputeDueToAssetPathChange(layerStack)) {
layerStackChanges |= LayerStackSignificantChange;
}
}

if (layerStackChanges & LayerStackRelocatesChange) {
_DidChangeLayerStackRelocations(caches, layerStack, debugSummary);
}

_DidChangeLayerStack(
layerStack,
layerStackChanges & LayerStackLayersChange,
layerStackChanges & LayerStackOffsetsChange,
layerStackChanges & LayerStackSignificantChange);
}

if (debugSummary && !debugSummary->empty()) {
Expand Down Expand Up @@ -1084,36 +1099,6 @@ PcpChanges::DidChangeLayerOffsets(PcpCache* cache)
}
}

void
PcpChanges::DidChangeLayerStack(PcpCache* cache,
const PcpLayerStackPtr& layerStack)
{
_DidChangeLayerStack(
layerStack,
/* requiresLayerStackChange = */ true,
/* requiresLayerStackOffsetChange = */ true,
/* requiresSignificantChange = */ true);

// Since this layer stack will be recomputed, need to register
// significant changes to all prim indexes that use it so that
// they pick up the new contents.

const PcpDependencyVector deps = cache->FindSiteDependencies(
layerStack, SdfPath::AbsoluteRootPath(),
PcpDependencyTypeAnyIncludingVirtual,
/* recurseOnSite */ true,
/* recurseOnIndex */ true,
/* filter */ true);

for (const PcpDependency& dep : deps) {
if (!dep.indexPath.IsAbsoluteRootOrPrimPath()) {
// Filter to only prims; see comment above re: properties.
continue;
}
DidChangeSignificantly(cache, dep.indexPath);
}
}

void
PcpChanges::DidChangeSignificantly(PcpCache* cache, const SdfPath& path)
{
Expand Down Expand Up @@ -1820,6 +1805,50 @@ PcpChanges::_DidChangeLayerStackRelocations(
}
}

void
PcpChanges::_DidChangeLayerStackResolvedPath(
const std::vector<PcpCache*>& caches,
const PcpLayerStackPtr& layerStack,
std::string* debugSummary)
{
for (PcpCache* cache : caches) {
PcpDependencyVector deps =
cache->FindSiteDependencies(
layerStack, SdfPath::AbsoluteRootPath(),
PcpDependencyTypeAnyIncludingVirtual,
/* recurseOnSite */ true,
/* recurseOnIndex */ false,
/* filterForExisting */ true);

auto noResyncNeeded = [cache](const PcpDependency& dep) {
if (!dep.indexPath.IsPrimPath()) {
return false;
}
const PcpPrimIndex* primIndex = cache->FindPrimIndex(dep.indexPath);
return (TF_VERIFY(primIndex) &&
!Pcp_NeedToRecomputeDueToAssetPathChange(*primIndex));
};

deps.erase(
std::remove_if(deps.begin(), deps.end(), noResyncNeeded),
deps.end());
if (deps.empty()) {
continue;
}

PCP_APPEND_DEBUG(
" Resync following in @%s@ significant due to layer "
"resolved path change:\n",
cache->GetLayerStackIdentifier().rootLayer->
GetIdentifier().c_str());

for (const PcpDependency& dep : deps) {
PCP_APPEND_DEBUG(" <%s>\n", dep.indexPath.GetText());
DidChangeSignificantly(cache, dep.indexPath);
}
}
}

void
PcpChanges::_DidChangeSpecStackInternal(PcpCache* cache, const SdfPath& path)
{
Expand Down
12 changes: 6 additions & 6 deletions pxr/usd/lib/pcp/changes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,6 @@ class PcpChanges {
PCP_API
void DidChangeLayerOffsets(PcpCache* cache);

/// The layer stack \p layerStack changed significantly, requiring
/// the layer stack to be recomputed.
PCP_API
void DidChangeLayerStack(PcpCache* cache,
const PcpLayerStackPtr& layerStack);

/// The object at \p path changed significantly enough to require
/// recomputing the entire prim or property index. A significant change
/// implies changes to every namespace descendant's index, specs, and
Expand Down Expand Up @@ -413,6 +407,12 @@ class PcpChanges {
const PcpLayerStackPtr & layerStack,
std::string* debugSummary );

// Register changes to any prim indexes in \p caches that are affected
// by a change to a layer's resolved path used by \p layerStack.
void _DidChangeLayerStackResolvedPath(const std::vector<PcpCache*>& caches,
const PcpLayerStackPtr& layerStack,
std::string* debugSummary);

// The spec stack for the prim or property index at \p path must be
// recomputed due to a change that affects only the internal representation
// of the stack and not its contents.
Expand Down
8 changes: 8 additions & 0 deletions pxr/usd/lib/sdf/changeList.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ std::ostream& operator<<(std::ostream &os, const SdfChangeList &cl)
os << " didRename\n";
if (entry.flags.didChangeIdentifier)
os << " didChangeIdentifier\n";
if (entry.flags.didChangeResolvedPath)
os << " didChangeResolvedPath\n";
if (entry.flags.didReplaceContent)
os << " didReplaceContent\n";
if (entry.flags.didReloadContent)
Expand Down Expand Up @@ -156,6 +158,12 @@ SdfChangeList::DidChangeLayerIdentifier(const std::string &oldIdentifier)
}
}

void
SdfChangeList::DidChangeLayerResolvedPath()
{
GetEntry(SdfPath::AbsoluteRootPath()).flags.didChangeResolvedPath = true;
}

void
SdfChangeList::DidChangeSublayerPaths( const std::string &subLayerPath,
SubLayerChangeType changeType )
Expand Down
2 changes: 2 additions & 0 deletions pxr/usd/lib/sdf/changeList.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class SdfChangeList

void DidReplaceLayerContent();
void DidReloadLayerContent();
void DidChangeLayerResolvedPath();
void DidChangeLayerIdentifier(const std::string &oldIdentifier);
void DidChangeSublayerPaths(const std::string &subLayerPath,
SubLayerChangeType changeType);
Expand Down Expand Up @@ -127,6 +128,7 @@ class SdfChangeList

// SdfLayer
bool didChangeIdentifier:1;
bool didChangeResolvedPath:1;
bool didReplaceContent:1;
bool didReloadContent:1;

Expand Down
8 changes: 8 additions & 0 deletions pxr/usd/lib/sdf/changeManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,14 @@ Sdf_ChangeManager::DidChangeLayerIdentifier(const SdfLayerHandle &layer,
_data.local().changes[layer].DidChangeLayerIdentifier(oldIdentifier);
}

void
Sdf_ChangeManager::DidChangeLayerResolvedPath(const SdfLayerHandle &layer)
{
if (!layer->_ShouldNotify())
return;
_data.local().changes[layer].DidChangeLayerResolvedPath();
}

static bool
_IsOrderChangeOnly(const VtValue & oldVal, const VtValue & newVal )
{
Expand Down
1 change: 1 addition & 0 deletions pxr/usd/lib/sdf/changeManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class Sdf_ChangeManager : boost::noncopyable {
void DidReloadLayerContent(const SdfLayerHandle &layer);
void DidChangeLayerIdentifier(const SdfLayerHandle &layer,
const std::string &oldIdentifier);
void DidChangeLayerResolvedPath(const SdfLayerHandle &layer);
void DidChangeField(const SdfLayerHandle &layer,
const SdfPath & path, const TfToken &field,
const VtValue & oldValue, const VtValue & newValue );
Expand Down
16 changes: 14 additions & 2 deletions pxr/usd/lib/sdf/layer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,11 @@ SdfLayer::_Reload(bool force)
}

_assetModificationTime.Swap(timestamp);

if (realPath != oldRealPath) {
Sdf_ChangeManager::Get().DidChangeLayerResolvedPath(
SdfLayerHandle(this));
}
}

_MarkCurrentStateAsClean();
Expand Down Expand Up @@ -1236,6 +1241,7 @@ SdfLayer::_InitializeFromIdentifier(
// must occur prior to updating the layer registry, as the new layer
// information is used to recompute registry indices.
string oldIdentifier = _assetInfo->identifier;
string oldRealPath = _assetInfo->realPath;
_assetInfo.swap(newInfo);

// Update layer state delegate.
Expand All @@ -1249,9 +1255,15 @@ SdfLayer::_InitializeFromIdentifier(
// Only send a notice if the identifier has changed (this notice causes
// mass invalidation. See http://bug/33217). If the old identifier was
// empty, this is a newly constructed layer, so don't send the notice.
if (!oldIdentifier.empty() && (oldIdentifier != GetIdentifier())) {
if (!oldIdentifier.empty()) {
SdfChangeBlock block;
Sdf_ChangeManager::Get().DidChangeLayerIdentifier(self, oldIdentifier);
if (oldIdentifier != GetIdentifier()) {
Sdf_ChangeManager::Get().DidChangeLayerIdentifier(
self, oldIdentifier);
}
if (oldRealPath != GetRealPath()) {
Sdf_ChangeManager::Get().DidChangeLayerResolvedPath(self);
}
}
}

Expand Down

0 comments on commit d0a29f8

Please sign in to comment.