Merge pull request PixarAnimationStudios#484 from autodesk-forks/reso…

…lve/dev

auto sync origin/dev @727e2ad to adsk/dev

VERSIONS.md

@@ -27,7 +27,7 @@ Our test machines have the following software versions installed
 | Qt for Python | PySide2 5.14.1       | PySide6 6.3.1                | PySide2 5.14.1                 |
 | PyOpenGL      | 3.1.5                | 3.1.5                        | 3.1.5                          |
 | Embree        | 3.2.2                | 3.13.3                       | 3.2.2                          |
-| RenderMan     | 24.4                 | 24.4                         | 24.4                           |
+| RenderMan     | 24.4, 25.0           | 24.4, 25.0                   | 24.4, 25.0                     |
 | Alembic       | 1.7.10               | 1.8.3                        | 1.7.10                         |
 | OpenEXR       | 2.4.3                | 2.4.3                        | 2.5.2                          |
 | MaterialX     | 1.38.7               | 1.38.7                       | 1.38.7                         |

pxr/base/plug/registry.cpp

@@ -136,9 +136,6 @@ PlugRegistry::_RegisterPlugins(const std::vector<std::string>& pathsToPlugInfo,
     NewPluginsVec newPlugins;
     {
         Plug_TaskArena taskArena;
-        // Drop the GIL in case we have it since we're taking a lock on _mutex
-        // below, and other python threads could reenter here.
-        TF_PY_ALLOW_THREADS_IN_SCOPE();
         // XXX -- Is this mutex really needed?
         std::lock_guard<std::mutex> lock(_mutex);
         WorkWithScopedParallelism([&]() {
@@ -152,7 +149,19 @@ PlugRegistry::_RegisterPlugins(const std::vector<std::string>& pathsToPlugInfo,
                         &PlugRegistry::_RegisterPlugin<NewPluginsVec>,
                         this, std::placeholders::_1, &newPlugins),
                     &taskArena);
-            });
+        }, /*dropPythonGIL=*/false);
+        // We explicitly do not drop the GIL here because of sad stories like
+        // the following. A shared library loads and during its initialization,
+        // it wants to look up information from plugins, and thus invokes this
+        // code to do first-time plugin registration. The dynamic loader holds
+        // its own lock while it loads the shared library. If this code holds
+        // the GIL (say the library is being loaded due to a python 'import')
+        // and was to drop it during the parallelism, then other Python-based
+        // threads can take the GIL and wind up calling, dlsym() for example.
+        // This will wait on the dynamic loader's lock, but this thread will
+        // never release it since it will wait to reacquire the GIL. This causes
+        // a deadlock between the dynamic loader's lock and the Python GIL.
+        // Retaining the GIL here prevents this scenario.
     }
 
     if (!newPlugins.empty()) {
@@ -275,12 +284,6 @@ PlugPlugin::_RegisterAllPlugins()
 {
     PlugPluginPtrVector result;
 
-    // Drop the GIL here -- the _RegisterPlugins call below will drop the GIL,
-    // meaning that some python caller could arrive here and wait on the
-    // call_once while holding the GIL.  When _RegisterPlugins attempts to
-    // reacquire the GIL, we'll deadlock.
-    TF_PY_ALLOW_THREADS_IN_SCOPE();
-
     static std::once_flag once;
     std::call_once(once, [&result](){
         PlugRegistry &registry = PlugRegistry::GetInstance();

pxr/base/plug/wrapTestPlugBase.cpp

@@ -63,8 +63,3 @@ void wrap_TestPlugBase()
     wrap_TestPlugBase<_TestPlugBase3>("_TestPlugBase3");
     wrap_TestPlugBase<_TestPlugBase4>("_TestPlugBase4");
 }
-
-TF_REFPTR_CONST_VOLATILE_GET(_TestPlugBase1)
-TF_REFPTR_CONST_VOLATILE_GET(_TestPlugBase2)
-TF_REFPTR_CONST_VOLATILE_GET(_TestPlugBase3)
-TF_REFPTR_CONST_VOLATILE_GET(_TestPlugBase4)

pxr/base/tf/pyContainerConversions.h

@@ -56,7 +56,7 @@
 
 PXR_NAMESPACE_OPEN_SCOPE
 
-// Converter from vector<string> to python list.
+// Converts any iterable C++ container to a Python list.
 template <typename ContainerType>
 struct TfPySequenceToPython
 {
@@ -70,6 +70,20 @@ struct TfPySequenceToPython
     }
 };
 
+// Converts any iterable C++ container to a Python set.
+template <typename ContainerType>
+struct TfPySequenceToPythonSet
+{
+    static PyObject* convert(ContainerType const &c)
+    {
+        PyObject* result = PySet_New(nullptr);
+        for (const auto &elem : c) {
+            PySet_Add(result, boost::python::object(elem).ptr());
+        }
+        return result;
+    }
+};
+
 template <typename ContainerType>
 struct TfPyMapToPythonDict
 {

pxr/base/tf/refPtr.h

@@ -1343,25 +1343,6 @@ TfHashAppend(HashState &h, const TfRefPtr<T> &ptr)
 
 #define TF_SUPPORTS_REFPTR(T) std::is_base_of<TfRefBase, T>::value
 
-#if defined(ARCH_COMPILER_MSVC) 
-// There is a bug in the compiler which means we have to provide this
-// implementation. See here for more information:
-// https://connect.microsoft.com/VisualStudio/Feedback/Details/2852624
-
-#define TF_REFPTR_CONST_VOLATILE_GET(x)                                       \
-        namespace boost                                                       \
-        {                                                                     \
-            template<>                                                        \
-            const volatile x*                                                 \
-                get_pointer(const volatile x* p)                              \
-            {                                                                 \
-                return p;                                                     \
-            }                                                                 \
-        }
-#else
-#define TF_REFPTR_CONST_VOLATILE_GET(x)
-#endif
-
 PXR_NAMESPACE_CLOSE_SCOPE
 
 #endif // PXR_BASE_TF_REF_PTR_H

pxr/base/tf/wrapScriptModuleLoader.cpp

@@ -51,5 +51,3 @@ void wrapScriptModuleLoader() {
         .def("_LoadModulesForLibrary", &This::LoadModulesForLibrary)
         ;
 }
-
-TF_REFPTR_CONST_VOLATILE_GET(TfScriptModuleLoader)

pxr/base/tf/wrapTestTfPython.cpp

@@ -608,9 +608,3 @@ void wrapTf_TestTfPython()
         ;
 
 }
-
-TF_REFPTR_CONST_VOLATILE_GET(Tf_ClassWithVarArgInit)
-TF_REFPTR_CONST_VOLATILE_GET(Tf_TestBase)
-TF_REFPTR_CONST_VOLATILE_GET(Tf_TestDerived)
-TF_REFPTR_CONST_VOLATILE_GET(polymorphic_Tf_TestBase<class Tf_TestBase>)
-TF_REFPTR_CONST_VOLATILE_GET(polymorphic_Tf_TestDerived<class Tf_TestDerived>)

pxr/base/tf/wrapToken.cpp

@@ -83,24 +83,26 @@ void wrapToken()
     TfPyContainerConversions::from_python_sequence<
         std::set<TfToken> , 
         TfPyContainerConversions::set_policy >();
+    bp::to_python_converter<
+        std::set<TfToken>, 
+        TfPySequenceToPythonSet<std::set<TfToken> > >();
 
     TfPyContainerConversions::from_python_sequence<
         std::vector<TfToken>,
         TfPyContainerConversions::variable_capacity_policy >();
-
-    boost::python::to_python_converter<
+    bp::to_python_converter<
         std::vector<TfToken>, 
         TfPySequenceToPython<std::vector<TfToken> > >();
 
     // Tokens are represented directly as Python strings in Python.
-    bp::to_python_converter<TfToken, Tf_TokenToPythonString>();
     Tf_TokenFromPythonString();
+    bp::to_python_converter<TfToken, Tf_TokenToPythonString>();
 
+    TfPyContainerConversions::from_python_tuple_pair<
+        std::pair<TfToken, TfToken>>();
     bp::to_python_converter<
         std::pair<TfToken, TfToken>,
         TfPyContainerConversions::to_tuple<std::pair<TfToken, TfToken>>>();
-    TfPyContainerConversions::from_python_tuple_pair<
-        std::pair<TfToken, TfToken>>();
 
     // Stats.
     bp::def("DumpTokenStats", TfDumpTokenStats);

pxr/base/work/withScopedParallelism.h

@@ -107,32 +107,38 @@ PXR_NAMESPACE_OPEN_SCOPE
 /// thread that restricts the tasks it can take to the protected scope: all
 /// other worker threads continue unhindered.
 ///
-/// If Python support is enabled, this function ensures the GIL is released
-/// before invoking \p fn.  If this function released the GIL, it reacquires it
-/// before returning.
+/// If Python support is enabled and \p dropPythonGIL is true, this function
+/// ensures the GIL is released before invoking \p fn.  If this function
+/// released the GIL, it reacquires it before returning.
 ///
 template <class Fn>
 auto
-WorkWithScopedParallelism(Fn &&fn)
+WorkWithScopedParallelism(Fn &&fn, bool dropPythonGIL=true)
 {
-    TF_PY_ALLOW_THREADS_IN_SCOPE();
-    return tbb::this_task_arena::isolate(std::forward<Fn>(fn));
+    if (dropPythonGIL) {
+        TF_PY_ALLOW_THREADS_IN_SCOPE();
+        return tbb::this_task_arena::isolate(std::forward<Fn>(fn));
+    }
+    else {
+        return tbb::this_task_arena::isolate(std::forward<Fn>(fn));
+    }
 }
 
-/// Similar to WorkWithScopedParallelism, but pass a WorkDispatcher instance to
-/// \p fn for its use during the scoped parallelism.  Accordingly, \p fn must
+/// Similar to WorkWithScopedParallelism(), but pass a WorkDispatcher instance
+/// to \p fn for its use during the scoped parallelism.  Accordingly, \p fn must
 /// accept a WorkDispatcher lvalue reference argument.  After \p fn returns but
 /// before the scoped parallelism ends, call WorkDispatcher::Wait() on the
-/// dispatcher instance.
+/// dispatcher instance.  The \p dropPythonGIL argument has the same meaning as
+/// it does for WorkWithScopedParallelism().
 template <class Fn>
 auto
-WorkWithScopedDispatcher(Fn &&fn)
+WorkWithScopedDispatcher(Fn &&fn, bool dropPythonGIL=true)
 {
     return WorkWithScopedParallelism([&fn]() {
         WorkDispatcher dispatcher;
         return std::forward<Fn>(fn)(dispatcher);
         // dispatcher's destructor invokes Wait() here.
-    });
+    }, dropPythonGIL);
 }
 
 PXR_NAMESPACE_CLOSE_SCOPE

pxr/imaging/garch/wrapPlatformDebugContext.cpp

@@ -43,5 +43,3 @@ void wrapPlatformDebugContext()
         .def("makeCurrent", &This::makeCurrent)
     ;
 }
-
-TF_REFPTR_CONST_VOLATILE_GET(GarchGLPlatformDebugContext)

pxr/imaging/glf/wrapDrawTarget.cpp

@@ -73,5 +73,3 @@ void wrapDrawTarget()
 
         ;
 }
-
-TF_REFPTR_CONST_VOLATILE_GET(GlfDrawTarget)

pxr/imaging/glf/wrapTexture.cpp

@@ -62,5 +62,3 @@ void wrapTexture()
                 return_value_policy<return_by_value>()))
         ;
 }
-
-TF_REFPTR_CONST_VOLATILE_GET(GlfTexture)

pxr/imaging/hd/mergingSceneIndex.cpp

@@ -25,6 +25,8 @@
 #include "pxr/imaging/hd/overlayContainerDataSource.h"
 #include "pxr/base/tf/denseHashSet.h"
 #include "pxr/base/trace/trace.h"
+#include "pxr/base/work/dispatcher.h"
+#include <tbb/concurrent_queue.h>
 
 PXR_NAMESPACE_OPEN_SCOPE
 
@@ -33,20 +35,31 @@ HdMergingSceneIndex::HdMergingSceneIndex()
 {
 }
 
-void
-HdMergingSceneIndex::
-_FillAddedEntriesRecursively(
-    const HdSceneIndexBaseRefPtr &sceneIndex,
-    const SdfPath &primPath,
-    HdSceneIndexObserver::AddedPrimEntries * const addedEntries)
+// Concurrent queue of added entries that worker threads produce.
+using _AddedPrimEntryQueue =
+    tbb::concurrent_queue<HdSceneIndexObserver::AddedPrimEntry>;
+
+static void
+_FillAddedChildEntriesRecursively(
+    WorkDispatcher *dispatcher,
+    HdMergingSceneIndex *mergingSceneIndex,
+    const HdSceneIndexBaseRefPtr &inputSceneIndex,
+    SdfPath parentPath,
+    _AddedPrimEntryQueue *queue)
 {
-    // Old scene indices might have a prim of different type at the given path,
-    // so we need to query the merging scene index itself here.
-    const TfToken resolvedPrimType = GetPrim(primPath).primType;
-
-    addedEntries->emplace_back(primPath, resolvedPrimType);
-    for (const SdfPath &childPath : sceneIndex->GetChildPrimPaths(primPath)) {
-        _FillAddedEntriesRecursively(sceneIndex, childPath, addedEntries);
+    for (SdfPath childPath : inputSceneIndex->GetChildPrimPaths(parentPath)) {
+        // Old scene indices might have a prim of different type at the given path,
+        // so we need to query the merging scene index itself here.
+        const TfToken resolvedPrimType =
+            mergingSceneIndex->GetPrim(childPath).primType;
+
+        queue->emplace(childPath, resolvedPrimType);
+
+        dispatcher->Run([=]() {
+            _FillAddedChildEntriesRecursively(
+                dispatcher, mergingSceneIndex, 
+                inputSceneIndex, childPath, queue);
+            });
     }
 }
 
@@ -69,6 +82,8 @@ HdMergingSceneIndex::AddInputScene(
     const HdSceneIndexBaseRefPtr &inputScene,
     const SdfPath &activeInputSceneRoot)
 {
+    TRACE_FUNCTION();
+
     if (!inputScene) {
         return;
     }
@@ -102,15 +117,33 @@ HdMergingSceneIndex::AddInputScene(
         return;
     }
 
-    _FillAddedEntriesRecursively(
-        inputScene, activeInputSceneRoot, &addedEntries);
+    // Add entries for input scene
+    {
+        _AddedPrimEntryQueue queue;
+
+        // Old scene indices might have a prim of different type at the given path,
+        // so we need to query the merging scene index itself here.
+        queue.emplace(activeInputSceneRoot,
+                       GetPrim(activeInputSceneRoot).primType);
+
+        WorkDispatcher dispatcher;
+        _FillAddedChildEntriesRecursively(
+            &dispatcher, this, inputScene, activeInputSceneRoot, &queue);
+        dispatcher.Wait();
+
+        addedEntries.insert(
+            addedEntries.end(),
+            queue.unsafe_begin(), queue.unsafe_end());
+    }
 
     _SendPrimsAdded(addedEntries);
 }
 
 void
 HdMergingSceneIndex::RemoveInputScene(const HdSceneIndexBaseRefPtr &sceneIndex)
 {
+    TRACE_FUNCTION();
+
     for (_InputEntries::iterator it = _inputs.begin(); it != _inputs.end();
             ++it) {
         if (sceneIndex == it->sceneIndex) {

pxr/imaging/hd/mergingSceneIndex.h

@@ -88,12 +88,6 @@ class HdMergingSceneIndex : public HdFilteringSceneIndexBase
         const HdSceneIndexBase &sender,
         const HdSceneIndexObserver::DirtiedPrimEntries &entries);
 
-    void _FillAddedEntriesRecursively(
-        const HdSceneIndexBaseRefPtr &newSceneIndex,
-        const SdfPath &primPath,
-        HdSceneIndexObserver::AddedPrimEntries * const addedEntries);
-
-
     friend class _Observer;
 
     class _Observer : public HdSceneIndexObserver

pxr/imaging/hd/renderSettings.cpp

@@ -73,6 +73,7 @@ HdRenderSettings::HdRenderSettings(
     SdfPath const& id)
     : HdBprim(id)
     , _active(false)
+    , _settingsVersion(1)
 {
 }
 
@@ -90,6 +91,12 @@ HdRenderSettings::GetNamespacedSettings() const
     return _namespacedSettings;
 }
 
+unsigned int
+HdRenderSettings::GetSettingsVersion() const
+ {
+    return _settingsVersion;
+}
+
 const HdRenderSettings::RenderProducts&
 HdRenderSettings::GetRenderProducts() const
 {
@@ -135,6 +142,7 @@ HdRenderSettings::Sync(
             GetId(), HdRenderSettingsPrimTokens->namespacedSettings);
         if (vSettings.IsHolding<VtDictionary>()) {
             _namespacedSettings = vSettings.UncheckedGet<VtDictionary>();
+            _settingsVersion++;
         }
     }