[UsdImaging] GetInstanceCategories should get categories for all inst…

…ances Prior version did not work for nested instancing. It only fetched categories for the immediate instancer and, when nested, would prepend one or more empty category lists, confounding indexing. This change uses the existing visitor (_RunForAllInstancesToDraw) to collect category lists for the full instance hierarchy. Fixes #2002 (Internal change: 2264225)
PixarAnimationStudios · Feb 28, 2023 · fd09106 · fd09106
1 parent 6248c47
commit fd09106
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 10 deletions.
diff --git a/pxr/imaging/hd/unitTestNullRenderDelegate.cpp b/pxr/imaging/hd/unitTestNullRenderDelegate.cpp
@@ -25,6 +25,7 @@
 #include "pxr/imaging/hd/bufferArray.h"
 #include "pxr/imaging/hd/camera.h"
 #include "pxr/imaging/hd/coordSys.h"
+#include "pxr/imaging/hd/light.h"
 #include "pxr/imaging/hd/material.h"
 #include "pxr/imaging/hd/mesh.h"
 #include "pxr/imaging/hd/basisCurves.h"
@@ -235,6 +236,29 @@ class Hd_NullMaterial final : public HdMaterial {
     Hd_NullMaterial &operator =(const Hd_NullMaterial &) = delete;
 };
 
+class Hd_NullLight final : public HdLight {
+public:
+    Hd_NullLight(SdfPath const& id) : HdLight(id) {}
+    virtual ~Hd_NullLight() = default;
+
+    virtual void Sync(HdSceneDelegate *sceneDelegate,
+                      HdRenderParam   *renderParam,
+                      HdDirtyBits     *dirtyBits) override
+    {
+        *dirtyBits = HdLight::Clean;
+    }
+
+    virtual HdDirtyBits GetInitialDirtyBitsMask() const override
+    {
+        return HdLight::AllDirty;
+    }
+
+private:
+    Hd_NullLight()                                 = delete;
+    Hd_NullLight(const Hd_NullLight &)             = delete;
+    Hd_NullLight &operator =(const Hd_NullLight &) = delete;
+};
+
 class Hd_NullCoordSys final : public HdCoordSys {
 public:
     Hd_NullCoordSys(SdfPath const& id) : HdCoordSys(id) {}
@@ -290,6 +314,7 @@ const TfTokenVector Hd_UnitTestNullRenderDelegate::SUPPORTED_SPRIM_TYPES =
 {
     HdPrimTypeTokens->camera,
     HdPrimTypeTokens->coordSys,
+    HdPrimTypeTokens->domeLight,
     HdPrimTypeTokens->material
 };
 
@@ -369,6 +394,8 @@ Hd_UnitTestNullRenderDelegate::CreateSprim(TfToken const& typeId,
 {
     if (typeId == HdPrimTypeTokens->material) {
         return new Hd_NullMaterial(sprimId);
+    } else if (typeId == HdPrimTypeTokens->domeLight) {
+        return new Hd_NullLight(SdfPath::EmptyPath());
     } else if (typeId == HdPrimTypeTokens->coordSys) {
         return new Hd_NullCoordSys(sprimId);
     } else if (typeId == HdPrimTypeTokens->camera) {
@@ -384,6 +411,8 @@ Hd_UnitTestNullRenderDelegate::CreateFallbackSprim(TfToken const& typeId)
 {
     if (typeId == HdPrimTypeTokens->material) {
         return new Hd_NullMaterial(SdfPath::EmptyPath());
+    } else if (typeId == HdPrimTypeTokens->domeLight) {
+        return new Hd_NullLight(SdfPath::EmptyPath());
     } else if (typeId == HdPrimTypeTokens->coordSys) {
         return new Hd_NullCoordSys(SdfPath::EmptyPath());
     } else if (typeId == HdPrimTypeTokens->camera) {

diff --git a/pxr/usdImaging/usdImaging/instanceAdapter.cpp b/pxr/usdImaging/usdImaging/instanceAdapter.cpp
@@ -1541,21 +1541,60 @@ UsdImagingInstanceAdapter::MarkVisibilityDirty(UsdPrim const& prim,
     }
 }
 
+struct UsdImagingInstanceAdapter::_GetInstanceCategoriesFn
+{
+    _GetInstanceCategoriesFn(
+        const UsdImagingInstanceAdapter* adapter,
+        const UsdImaging_CollectionCache* cc, 
+        std::vector<VtTokenArray>* result) : 
+        _adapter(adapter),
+        _cc(cc),
+        _result(result)
+    { }
+
+    void Initialize(size_t numInstances)
+    { 
+        _result->resize(numInstances);
+    }
+
+    bool operator()(const std::vector<UsdPrim>& ctx, size_t idx)
+    {
+        // We must query the collections cache using the instance's stage path, 
+        // not its proxy path. _GetStagePath() reconstructs the stage path
+        // from the instancing context.)
+        const SdfPath& path = _GetStagePath(ctx);
+        if (path.IsEmpty()) {
+            return false;
+        }
+        _result->at(idx) = _cc->ComputeCollectionsContainingPath(path);
+        return true;
+    }
+
+    SdfPath _GetStagePath(const std::vector<UsdPrim>& ctx)
+    {
+        SdfPathVector chain;
+        chain.reserve(ctx.size());
+        for (const UsdPrim& prim : ctx) {
+            chain.push_back(prim.GetPath());
+        }
+        return _adapter->_GetPrimPathFromInstancerChain(chain);
+    }
+
+    const UsdImagingInstanceAdapter* _adapter;
+    const UsdImaging_CollectionCache* _cc;
+    std::vector<VtTokenArray>* _result;
+};
+
 /*virtual*/
 std::vector<VtArray<TfToken>>
 UsdImagingInstanceAdapter::GetInstanceCategories(UsdPrim const& prim) 
 {
     HD_TRACE_FUNCTION();
-    std::vector<VtArray<TfToken>> categories;
-    if (const _InstancerData* instancerData = 
-        TfMapLookupPtr(_instancerData, prim.GetPath())) {
-        UsdImaging_CollectionCache& cc = _GetCollectionCache();
-        categories.reserve(instancerData->instancePaths.size());
-        for (SdfPath const& p: instancerData->instancePaths) {
-            categories.push_back(cc.ComputeCollectionsContainingPath(p));
-        }
-    }
-    return categories;
+    const UsdImaging_CollectionCache& cc = _GetCollectionCache();
+    std::vector<VtTokenArray> result;
+    _GetInstanceCategoriesFn catsFn(this, &cc, &result);
+    _RunForAllInstancesToDraw(prim, &catsFn);
+    return result;
 }
 
 /*virtual*/

diff --git a/pxr/usdImaging/usdImaging/instanceAdapter.h b/pxr/usdImaging/usdImaging/instanceAdapter.h
@@ -460,6 +460,7 @@ class UsdImagingInstanceAdapter : public UsdImagingPrimAdapter
 
     struct _PopulateInstanceSelectionFn;
     struct _GetScenePrimPathsFn;
+    struct _GetInstanceCategoriesFn;
 
     // Helper functions for dealing with "actual" instances to be drawn.
     //