Ancestral unloaded payload discovery code results in overly broad resync notices

[nvmkuruc]

Description of Issue

UsdStage::LoadAndUnload takes a set of paths to load and unload on the stage and then expands it to honor the contract of loading ancestral payloads (and properly handle instancing).

In trying to identify ancestral payloads, it uses the following check--

prim && prim.IsLoaded() && p != curPath

However, for active prims without any ancestral payloads prim.IsLoaded() trivially returns true. Therefore, for a payloaded prim without any ancestral payloads, this discovery path will always return that prim's immediate parent. In practice, this means for the simple case of a grouping scope containing a dozen or so payloaded assets, loading one triggers a resync on the parent (implying the siblings and their descendants should be resynced).

To generate resync notices with a better scoped granularity, this code should be updated to either return all ancestral prims with payload composition arcs that are not loaded or the most ancestral prim with a payload that is unloaded.

Steps to Reproduce

1. Create the following ascii layer

#usda 1.0

over Scope "prim1" {
}

def Scope "root" {
    def Scope "subroot" {
        def "prim1" (payload = </prim1>){}
    }
}

2. usdview --norender --unloaded <LAYER>.usda
3. Enable USD_CHANGES and USD_PAYLOAD debug flags
4. Select /root/subroot/prim1
5. Load
6. The terminal reports /root/subroot as requiring a resync when /root/subroot/prim1 would be expected.

System Information (OS, Hardware)

Ubuntu 22.04

Package Versions

Python 3.10
Boost 1.78
TBB 2020.3

Build Flags

--usd-imaging --usdview

[tallytalwar]

Filed as internal issue #USD-8023

[gitamohr]

Nice catch, thanks @nvmkuruc ! I think this is a logic bug in that code. It wants to find the most ancestral unloaded path (i.e. walking unloaded parents, stopping when it finds a loaded parent) but it conflates the check for redundantly reinserting the path p with the stop condition. That is, this bit:

UsdPrim prim = GetPrimAtPath(parentPath);
if (prim && prim.IsLoaded() && p != curPath) {
    finalLoadSet.insert(curPath);
    break;
}

Should instead be:

UsdPrim prim = GetPrimAtPath(parentPath);
if (prim && prim.IsLoaded()) {
    if (p != curPath) {
        finalLoadSet.insert(curPath);
    }
    break;
}

That is, we always stop the search when we find a loaded ancestor and we just don't redundantly reinsert curPath if it's the same as p. This passes your test, which I've added to the suite. I've also added more code comments explaining what's going on here.

Please let me know if this makes sense to you or if I've missed something.

[nvmkuruc]

I think your proposed solution works.

Seeing both your and my proposed solution, I wonder if the conditional exist outside of the loop. So the logic is "Do you have an unloaded parent? If so, find the load / unload boundary." Rather than have that be embedded in the loop.

Happy to have it fixed either way.

[nvmkuruc]

@gitamohr @jesschimein I think this can be closed. This was fixed by usd: Fix a bug where UsdStage::LoadAndUnload() could overinvalidate & · PixarAnimationStudios/OpenUSD@91fd738 (github.com)

[jesschimein]

Thanks for flagging, Matt!