Ensure light collections are dirtied on creation

pxr/usdImaging/usdImaging/collectionCache.cpp

@@ -38,10 +38,29 @@ _IsQueryTrivial(UsdCollectionAPI::MembershipQuery const& query)
         ruleMap.begin()->second == UsdTokens->expandPrims;
 }
 
+void
+UsdImaging_CollectionCache::_MarkCollectionContentDirty(UsdStageWeakPtr const& stage, 
+    UsdCollectionAPI::MembershipQuery const& query)
+{
+    if(query.HasExcludes())
+    {
+        // If there are any exlusion we have to consider all previously excluded paths dirty
+        // As there is no API to query excluded paths we mark all prims in the Stage dirty
+        _allPathsDirty = true;
+    }
+    else
+    {
+        SdfPathSet linkedPaths = UsdComputeIncludedPathsFromCollection(query, stage);
+        std::merge(_dirtyPaths.begin(), _dirtyPaths.end(), linkedPaths.begin(), linkedPaths.end(),
+            std::inserter(_dirtyPaths, _dirtyPaths.begin()));
+    }
+}
+
 TfToken
 UsdImaging_CollectionCache::UpdateCollection(UsdCollectionAPI const& c)
 {
-    RemoveCollection(c);
+    const UsdStageWeakPtr& stage = c.GetPrim().GetStage();
+    RemoveCollection(stage, c.GetCollectionPath());
 
     std::lock_guard<std::mutex> lock(_mutex);
 
@@ -78,15 +97,16 @@ UsdImaging_CollectionCache::UpdateCollection(UsdCollectionAPI const& c)
     _pathsForQuery[query].insert(path);
     _idForPath[path] = id;
 
+    _MarkCollectionContentDirty(stage, query);
+
     return id;
 }
 
 void
-UsdImaging_CollectionCache::RemoveCollection(UsdCollectionAPI const& c)
+UsdImaging_CollectionCache::RemoveCollection(UsdStageWeakPtr const& stage, SdfPath const& path)
 {
     std::lock_guard<std::mutex> lock(_mutex);
 
-    SdfPath path = c.GetCollectionPath();
     auto const& pathEntry = _idForPath.find(path);
     if (pathEntry == _idForPath.end()) {
         // No pathEntry -- bail.  This can happen if the collection was
@@ -102,6 +122,9 @@ UsdImaging_CollectionCache::RemoveCollection(UsdCollectionAPI const& c)
         return;
     }
     UsdCollectionAPI::MembershipQuery const& queryRef = queryEntry->second;
+    _idForQuery.erase(queryRef);
+
+    _MarkCollectionContentDirty(stage, queryRef);
 
     auto const& pathsForQueryEntry = _pathsForQuery.find(queryRef);
     pathsForQueryEntry->second.erase(path);
@@ -113,13 +136,31 @@ UsdImaging_CollectionCache::RemoveCollection(UsdCollectionAPI const& c)
     // This also reaps the associated identifier.
     if (pathsForQueryEntry->second.empty()) {
         _pathsForQuery.erase(pathsForQueryEntry);
-        _idForQuery.erase(queryRef);
         _queryForId.erase(queryEntry);
         TF_DEBUG(USDIMAGING_COLLECTIONS)
             .Msg("UsdImaging_CollectionCache: Dropped id '%s'", id.GetText());
     }
 };
 
+SdfPathSet const&
+UsdImaging_CollectionCache::GetDirtyPaths() const
+{
+    return _dirtyPaths;
+}
+
+bool
+UsdImaging_CollectionCache::AreAllPathsDirty() const
+{
+    return _allPathsDirty;
+}
+
+void
+UsdImaging_CollectionCache::ClearDirtyPaths()
+{
+    _dirtyPaths.clear();
+    _allPathsDirty = false;
+}
+
 TfToken
 UsdImaging_CollectionCache::GetIdForCollection(UsdCollectionAPI const& c)
 {

pxr/usdImaging/usdImaging/collectionCache.h

@@ -68,7 +68,7 @@ class UsdImaging_CollectionCache : boost::noncopyable {
 
     /// Remove any cached entry for the given collection.
     /// Does nothing if no cache entry exists.
-    void RemoveCollection(UsdCollectionAPI const& collection);
+    void RemoveCollection(UsdStageWeakPtr const& stage, SdfPath const& path);
 
     /// Return the cached entry for the given collection.
     TfToken
@@ -79,6 +79,24 @@ class UsdImaging_CollectionCache : boost::noncopyable {
     VtArray<TfToken>
     ComputeCollectionsContainingPath(SdfPath const& path) const;
 
+    /// Return the cached MembershipQuery for a given id
+    void
+    GetMembershipQuery(TfToken const& id, const Query** query) const;
+
+    /// Returns true if all paths should be considered dirty
+    /// This happens when a collection containing an exclusion is updated
+    bool
+    AreAllPathsDirty() const;
+
+    /// Returns a set of dirty paths
+    /// Should only be used if AreAllPathsDirty returned false
+    SdfPathSet const&
+    GetDirtyPaths() const;
+
+    /// Clears the internal dirty flags
+    void
+    ClearDirtyPaths();
+
 private:
     // The cache boils down to tracking the correspondence of
     // collection paths, their computed queries, and the id
@@ -95,6 +113,12 @@ class UsdImaging_CollectionCache : boost::noncopyable {
     std::unordered_map<SdfPath, TfToken, SdfPath::Hash> _idForPath;
     std::unordered_map<Query, SdfPathSet, Query::Hash> _pathsForQuery;
 
+    void
+    _MarkCollectionContentDirty(UsdStageWeakPtr const& stage, UsdCollectionAPI::MembershipQuery const& query);
+
+    SdfPathSet _dirtyPaths;
+    bool _allPathsDirty = false;
+
     std::mutex _mutex;
 };
 

pxr/usdImaging/usdImaging/delegate.cpp

@@ -1082,6 +1082,39 @@ UsdImagingDelegate::ApplyPendingUpdates()
     indexProxy._UniqueifyPathsToRepopulate();
     _Populate(&indexProxy);
     _ExecuteWorkForVariabilityUpdate(&worker);
+
+    // Mark all dirty collection prims
+    if (_collectionCache.AreAllPathsDirty())
+    {
+        TRACE_SCOPE("Mark all dirty collection members");
+        TF_FOR_ALL(it, _hdPrimInfoMap) {
+            UsdImagingPrimAdapterSharedPtr& adapter = it->second.adapter;
+            TF_DEBUG(USDIMAGING_CHANGES).Msg("[Update]: invalidate collection member prim (all paths dirty) %s\n", it->first.GetText());
+            adapter->MarkDirty(it->second.usdPrim, it->first,
+                HdChangeTracker::DirtyCategories, &indexProxy);
+        }
+        _collectionCache.ClearDirtyPaths();
+    }
+    else
+    {
+        const SdfPathSet& pathsDirtiedByCollections = _collectionCache.GetDirtyPaths();
+        if(!pathsDirtiedByCollections.empty())
+        {
+            TRACE_SCOPE("Mark dirty collection members");
+            for(const SdfPath& dirtyPath : pathsDirtiedByCollections)
+            {
+                TF_DEBUG(USDIMAGING_CHANGES).Msg("[Update]: invalidate collection member prim %s\n", dirtyPath.GetText());
+                _HdPrimInfo* primInfo = _GetHdPrimInfo(dirtyPath);
+                if (primInfo && primInfo->usdPrim.IsValid() &&
+                    TF_VERIFY(primInfo->adapter, "%s", dirtyPath.GetText())) {
+                    UsdImagingPrimAdapterSharedPtr& adapter = primInfo->adapter;
+                    adapter->MarkDirty(primInfo->usdPrim, dirtyPath,
+                        HdChangeTracker::DirtyCategories, &indexProxy);
+                }
+            }
+            _collectionCache.ClearDirtyPaths();
+        }
+    }
 }
 
 void 

pxr/usdImaging/usdImaging/delegate.h

@@ -520,6 +520,12 @@ class UsdImagingDelegate : public HdSceneDelegate, public TfWeakBase {
     USDIMAGING_API
     bool IsInInvisedPaths(const SdfPath &usdPath) const;
 
+    USDIMAGING_API
+    UsdStageRefPtr GetStage() const
+    {
+        return _stage;
+    }
+
 private:
     // Internal Get and SamplePrimvar
     VtValue _Get(SdfPath const& id, TfToken const& key, VtIntArray *outIndices);

pxr/usdImaging/usdImaging/diskLightAdapter.cpp

@@ -64,6 +64,7 @@ void
 UsdImagingDiskLightAdapter::_RemovePrim(SdfPath const& cachePath,
                                          UsdImagingIndexProxy* index)
 {
+    UsdImagingLightAdapter::_RemovePrim(cachePath, index);
     index->RemoveSprim(HdPrimTypeTokens->diskLight, cachePath);
 }
 

pxr/usdImaging/usdImaging/distantLightAdapter.cpp

@@ -64,6 +64,7 @@ void
 UsdImagingDistantLightAdapter::_RemovePrim(SdfPath const& cachePath,
                                          UsdImagingIndexProxy* index)
 {
+    UsdImagingLightAdapter::_RemovePrim(cachePath, index);
     index->RemoveSprim(HdPrimTypeTokens->distantLight, cachePath);
 }
 

pxr/usdImaging/usdImaging/domeLightAdapter.cpp

@@ -64,6 +64,7 @@ void
 UsdImagingDomeLightAdapter::_RemovePrim(SdfPath const& cachePath,
                                          UsdImagingIndexProxy* index)
 {
+    UsdImagingLightAdapter::_RemovePrim(cachePath, index);
     index->RemoveSprim(HdPrimTypeTokens->domeLight, cachePath);
 }
 

pxr/usdImaging/usdImaging/lightAdapter.cpp

@@ -80,6 +80,42 @@ UsdImagingLightAdapter::_RemovePrim(SdfPath const& cachePath,
                                     UsdImagingIndexProxy* index)
 {
     index->RemoveSprim(HdPrimTypeTokens->light, cachePath);
+    UsdImaging_CollectionCache &collectionCache = _GetCollectionCache();
+    SdfPath lightLinkPath = cachePath.AppendProperty(UsdImagingTokens->collectionLightLink);
+    collectionCache.RemoveCollection(GetDelegate()->GetStage(), lightLinkPath);
+    SdfPath shadowLinkPath = cachePath.AppendProperty(UsdImagingTokens->collectionShadowLink);
+    collectionCache.RemoveCollection(GetDelegate()->GetStage(), shadowLinkPath);
+}
+
+bool
+UsdImagingLightAdapter::_UpdateCollectionsChanged(UsdPrim const& prim, SdfPath const& cachePath) const
+{
+    UsdImaging_CollectionCache &collectionCache = _GetCollectionCache();
+    auto getCollectionHash = [&collectionCache] (const UsdCollectionAPI& api) -> size_t {
+        const TfToken id = collectionCache.UpdateCollection(api);
+        const UsdImaging_CollectionCache::Query* query = nullptr;
+        collectionCache.GetMembershipQuery(id, &query);
+        return query != nullptr ? query->GetHash() : 0;
+    };
+    UsdLuxLight light(prim);
+    const size_t newLightCollectionHash = getCollectionHash(light.GetLightLinkCollectionAPI());
+    const size_t newShadowCollectionHash = getCollectionHash(light.GetShadowLinkCollectionAPI());
+    auto hashesIt = _collectionHashes.find(cachePath);
+    if(hashesIt == _collectionHashes.end()){
+        hashesIt = _collectionHashes.insert({cachePath, {0, 0}}).first;
+    }
+    HashPair& hashes = hashesIt->second;
+    if (newLightCollectionHash != hashes.lightCollectionHash || newShadowCollectionHash != hashes.shadowCollectionHash)
+    {
+
+        hashes.lightCollectionHash = newLightCollectionHash;
+        hashes.shadowCollectionHash = newShadowCollectionHash;
+        return true;
+    }
+    else
+    {
+        return false;
+    }
 }
 
 void 
@@ -127,11 +163,14 @@ UsdImagingLightAdapter::TrackVariability(UsdPrim const& prim,
 
     UsdLuxLightAPI light(prim);
     if (TF_VERIFY(light)) {
-        UsdImaging_CollectionCache &collectionCache = _GetCollectionCache();
-        collectionCache.UpdateCollection(light.GetLightLinkCollectionAPI());
-        collectionCache.UpdateCollection(light.GetShadowLinkCollectionAPI());
-        // TODO: When collections change we need to invalidate affected
-        // prims with the DirtyCollections flag.
+        if (_UpdateCollectionsChanged(prim, cachePath))
+        {
+            *timeVaryingBits |= HdLight::DirtyBits::DirtyCollection;
+        }
+        else
+        {
+            *timeVaryingBits &= ~HdLight::DirtyBits::DirtyCollection;
+        }
     }
 
     // XXX Cache primvars for lights.
@@ -177,6 +216,9 @@ UsdImagingLightAdapter::ProcessPropertyChange(UsdPrim const& prim,
     if (UsdGeomXformable::IsTransformationAffectedByAttrNamed(propertyName)) {
         return HdLight::DirtyBits::DirtyTransform;
     }
+
+    _UpdateCollectionsChanged(prim, cachePath);
+
     // "DirtyParam" is the catch-all bit for light params.
     return HdLight::DirtyBits::DirtyParams;
 }

pxr/usdImaging/usdImaging/lightAdapter.h

@@ -128,6 +128,18 @@ class UsdImagingLightAdapter : public UsdImagingPrimAdapter {
     void _RemovePrim(SdfPath const& cachePath,
                      UsdImagingIndexProxy* index) override;
 
+private:
+    /// Updates the collection cache content
+    /// Checks for collection hash change and returns true if they are different
+    bool _UpdateCollectionsChanged(UsdPrim const& prim, SdfPath const& cachePath) const;
+
+    // Cache to detect collection changes
+    struct HashPair{
+        size_t lightCollectionHash = 0;
+        size_t shadowCollectionHash = 0;
+    };
+    mutable std::unordered_map<SdfPath, HashPair, SdfPath::Hash> _collectionHashes;
+
 };
 
 

pxr/usdImaging/usdImaging/rectLightAdapter.cpp

@@ -65,6 +65,7 @@ void
 UsdImagingRectLightAdapter::_RemovePrim(SdfPath const& cachePath,
                                          UsdImagingIndexProxy* index)
 {
+    UsdImagingLightAdapter::_RemovePrim(cachePath, index);
     index->RemoveSprim(HdPrimTypeTokens->rectLight, cachePath);
 }
 

pxr/usdImaging/usdImaging/sphereLightAdapter.cpp

@@ -65,6 +65,7 @@ void
 UsdImagingSphereLightAdapter::_RemovePrim(SdfPath const& cachePath,
                                          UsdImagingIndexProxy* index)
 {
+    UsdImagingLightAdapter::_RemovePrim(cachePath, index);
     index->RemoveSprim(HdPrimTypeTokens->sphereLight, cachePath);
 }
 

pxr/usdImaging/usdImaging/tokens.h

@@ -32,6 +32,8 @@ PXR_NAMESPACE_OPEN_SCOPE
 
 
 #define USDIMAGING_TOKENS   \
+    ((collectionLightLink, "collection:lightLink")) \
+    ((collectionShadowLink, "collection:shadowLink")) \
     ((infoSource, "info:source")) \
     (faceIndexPrimvar)      \
     (faceOffsetPrimvar)     \