From 32def62ac2e484b0d1c1fab390e10312540c56f9 Mon Sep 17 00:00:00 2001 From: Matt Johnson Date: Wed, 19 Feb 2020 14:39:22 -0800 Subject: [PATCH 1/5] migrate UBOBindingsSaver class to px_vp20/utils This workaround was originally extracted from the Pixar batch renderer and turned into a more convenient and reusable RAII-style class. Since the workaround it provides is needed in multiple places, it would be good to have it defined publicly to take advantage of that reuse. --- lib/render/mayaToHydra/renderOverride.cpp | 43 +---------------------- lib/render/px_vp20/utils.cpp | 22 ++++++++++++ lib/render/px_vp20/utils.h | 36 +++++++++++++++++++ 3 files changed, 59 insertions(+), 42 deletions(-) diff --git a/lib/render/mayaToHydra/renderOverride.cpp b/lib/render/mayaToHydra/renderOverride.cpp index ef803e390f..11cad80519 100644 --- a/lib/render/mayaToHydra/renderOverride.cpp +++ b/lib/render/mayaToHydra/renderOverride.cpp @@ -47,6 +47,7 @@ PXR_NAMESPACE_CLOSE_SCOPE #include #include +#include "mayaUsd/render/px_vp20/utils.h" #include "../../usd/hdMaya/delegates/delegateRegistry.h" #include "../../usd/hdMaya/delegates/sceneDelegate.h" @@ -107,48 +108,6 @@ class UfeSelectionObserver : public UFE_NS::Observer { #endif // WANT_UFE_BUILD -/// Simple RAII class to save uniform buffer bindings, to deal with a maya -/// issue. -/// -/// As originally explained by Pixar in their usdmaya plugin: -/// -/// XXX: When Maya is using OpenGL Core Profile as the rendering engine (in -/// either compatibility or strict mode), batch renders like those done in -/// the "Render View" window or through the ogsRender command do not -/// properly track uniform buffer binding state. This was causing issues -/// where the first batch render performed would look correct, but then all -/// subsequent renders done in that Maya session would be completely black -/// (no alpha), even if the frame contained only Maya-native geometry or if -/// a new scene was created/opened. -/// -/// To avoid this problem, we need to save and restore Maya's bindings -/// across Hydra calls. We try not to bog down performance by saving and -/// restoring *all* GL_MAX_UNIFORM_BUFFER_BINDINGS possible bindings, so -/// instead we only do just enough to avoid issues. Empirically, the -/// problematic binding has been the material binding at index 4. -class UBOBindingsSaver { -public: - static constexpr size_t UNIFORM_BINDINGS_TO_SAVE = 5u; - - UBOBindingsSaver() { - for (size_t i = 0u; i < _uniformBufferBindings.size(); ++i) { - glGetIntegeri_v( - GL_UNIFORM_BUFFER_BINDING, (GLuint)i, - &_uniformBufferBindings[i]); - } - } - - ~UBOBindingsSaver() { - for (size_t i = 0u; i < _uniformBufferBindings.size(); ++i) { - glBindBufferBase( - GL_UNIFORM_BUFFER, (GLuint)i, _uniformBufferBindings[i]); - } - } - -private: - std::array _uniformBufferBindings; -}; - } // namespace MtohRenderOverride::MtohRenderOverride(const MtohRendererDescription& desc) diff --git a/lib/render/px_vp20/utils.cpp b/lib/render/px_vp20/utils.cpp index 9dcef5240e..d7e6f27c45 100644 --- a/lib/render/px_vp20/utils.cpp +++ b/lib/render/px_vp20/utils.cpp @@ -27,6 +27,7 @@ #include "pxr/base/gf/vec3f.h" #include "pxr/base/gf/vec4f.h" #include "pxr/base/tf/stringUtils.h" +#include "pxr/imaging/garch/gl.h" #include "pxr/imaging/glf/simpleLight.h" #include "pxr/imaging/glf/simpleLightingContext.h" #include "pxr/imaging/glf/simpleMaterial.h" @@ -1081,4 +1082,25 @@ px_vp20Utils::OutputDisplayStatusToStream( } +UBOBindingsSaver::UBOBindingsSaver() +{ + for (size_t i = 0u; i < _uniformBufferBindings.size(); ++i) { + glGetIntegeri_v( + GL_UNIFORM_BUFFER_BINDING, + static_cast(i), + &_uniformBufferBindings[i]); + } +} + +UBOBindingsSaver::~UBOBindingsSaver() +{ + for (size_t i = 0u; i < _uniformBufferBindings.size(); ++i) { + glBindBufferBase( + GL_UNIFORM_BUFFER, + static_cast(i), + _uniformBufferBindings[i]); + } +} + + PXR_NAMESPACE_CLOSE_SCOPE diff --git a/lib/render/px_vp20/utils.h b/lib/render/px_vp20/utils.h index f783f20846..f76c8a5d76 100644 --- a/lib/render/px_vp20/utils.h +++ b/lib/render/px_vp20/utils.h @@ -24,6 +24,7 @@ #include "pxr/base/gf/matrix4d.h" #include "pxr/base/gf/matrix4f.h" #include "pxr/base/gf/vec4f.h" +#include "pxr/imaging/garch/gl.h" #include "pxr/imaging/glf/simpleLightingContext.h" #include @@ -33,6 +34,7 @@ #include #include +#include #include @@ -115,6 +117,40 @@ class px_vp20Utils }; +/// Simple RAII class to save uniform buffer bindings, to deal with a Maya +/// issue. +/// +/// XXX: When Maya is using OpenGL Core Profile as the rendering engine (in +/// either compatibility or strict mode), batch renders like those done in the +/// "Render View" window or through the ogsRender command do not properly track +/// uniform buffer binding state. This was causing issues where the first batch +/// render performed would look correct, but then all subsequent renders done +/// in that Maya session would be completely black (no alpha), even if the +/// frame contained only Maya-native geometry or if a new scene was +/// created/opened. +/// +/// To avoid this problem, this object can be used to save and restore Maya's +/// uniform buffer bindings across Hydra/OpenGL calls. We try not to bog down +/// performance by saving and restoring *all* GL_MAX_UNIFORM_BUFFER_BINDINGS +/// possible bindings, so instead we only do just enough to avoid issues. +/// Empirically, the problematic binding has been the material binding at +/// index 4. +class UBOBindingsSaver +{ + public: + static constexpr size_t UNIFORM_BINDINGS_TO_SAVE = 5u; + + MAYAUSD_CORE_PUBLIC + UBOBindingsSaver(); + + MAYAUSD_CORE_PUBLIC + ~UBOBindingsSaver(); + + private: + std::array _uniformBufferBindings; +}; + + PXR_NAMESPACE_CLOSE_SCOPE From 35a6a0f50ca8f360e08ee10d8819f67366d7be02 Mon Sep 17 00:00:00 2001 From: Matt Johnson Date: Wed, 19 Feb 2020 14:43:49 -0800 Subject: [PATCH 2/5] use UBOBindingsSaver when rendering in Pixar's batch renderer With UBOBindingsSaver now available from px_vp20/utils, this change uses that class to wrap the Hydra Execute() call made during rendering. --- lib/render/pxrUsdMayaGL/batchRenderer.cpp | 33 +++-------------------- 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/lib/render/pxrUsdMayaGL/batchRenderer.cpp b/lib/render/pxrUsdMayaGL/batchRenderer.cpp index 859d9d0b91..4be02c6927 100644 --- a/lib/render/pxrUsdMayaGL/batchRenderer.cpp +++ b/lib/render/pxrUsdMayaGL/batchRenderer.cpp @@ -22,8 +22,8 @@ #include "./debugCodes.h" #include "./userData.h" -#include "../px_vp20/utils.h" -#include "../px_vp20/utils_legacy.h" +#include "mayaUsd/render/px_vp20/utils.h" +#include "mayaUsd/render/px_vp20/utils_legacy.h" #include "pxr/base/gf/matrix4d.h" #include "pxr/base/gf/vec2i.h" @@ -1294,27 +1294,7 @@ UsdMayaGLBatchRenderer::_Render( GL_DEPTH_BUFFER_BIT | GL_VIEWPORT_BIT); - // XXX: When Maya is using OpenGL Core Profile as the rendering engine (in - // either compatibility or strict mode), batch renders like those done in - // the "Render View" window or through the ogsRender command do not - // properly track uniform buffer binding state. This was causing issues - // where the first batch render performed would look correct, but then all - // subsequent renders done in that Maya session would be completely black - // (no alpha), even if the frame contained only Maya-native geometry or if - // a new scene was created/opened. - // - // To avoid this problem, we need to save and restore Maya's bindings - // across Hydra calls. We try not to bog down performance by saving and - // restoring *all* GL_MAX_UNIFORM_BUFFER_BINDINGS possible bindings, so - // instead we only do just enough to avoid issues. Empirically, the - // problematic binding has been the material binding at index 4. - static constexpr size_t _UNIFORM_BINDINGS_TO_SAVE = 5u; - std::vector uniformBufferBindings(_UNIFORM_BINDINGS_TO_SAVE, 0); - for (size_t i = 0u; i < uniformBufferBindings.size(); ++i) { - glGetIntegeri_v(GL_UNIFORM_BUFFER_BINDING, - (GLuint)i, - &uniformBufferBindings[i]); - } + UBOBindingsSaver bindingsSaver; // hydra orients all geometry during topological processing so that // front faces have ccw winding. We disable culling because culling @@ -1374,13 +1354,6 @@ UsdMayaGLBatchRenderer::_Render( glDisable(GL_FRAMEBUFFER_SRGB_EXT); - // XXX: Restore Maya's uniform buffer binding state. See above for details. - for (size_t i = 0u; i < uniformBufferBindings.size(); ++i) { - glBindBufferBase(GL_UNIFORM_BUFFER, - (GLuint)i, - uniformBufferBindings[i]); - } - glPopAttrib(); // GL_LIGHTING_BIT | GL_ENABLE_BIT | GL_POLYGON_BIT | // GL_DEPTH_BUFFER_BIT | GL_VIEWPORT_BIT } From b0b7205231cc79b1d50396abb862bd7f2ba56824 Mon Sep 17 00:00:00 2001 From: Matt Johnson Date: Wed, 19 Feb 2020 14:46:21 -0800 Subject: [PATCH 3/5] save and restore uniform buffer bindings during batch renderer selection Uniform buffer bindings need to be saved and restored across Hydra Execute() calls as a workaround to the issue described in the documentation for the UBOBindingsSaver class. There are two such calls in the Pixar batch renderer, one during drawing, and another in the private _TestIntersection() method which is used when computing selections and during live surface workflows. The former was already protected by the workaround, but the latter was not, so this change applies the workaround there as well. --- lib/render/pxrUsdMayaGL/batchRenderer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/render/pxrUsdMayaGL/batchRenderer.cpp b/lib/render/pxrUsdMayaGL/batchRenderer.cpp index 4be02c6927..732df068b5 100644 --- a/lib/render/pxrUsdMayaGL/batchRenderer.cpp +++ b/lib/render/pxrUsdMayaGL/batchRenderer.cpp @@ -993,6 +993,8 @@ UsdMayaGLBatchRenderer::TestIntersectionCustomPrimFilter( // Differs from viewport implementations in that it doesn't rely on // _ComputeSelection being called first. + UBOBindingsSaver bindingsSaver; + return _TestIntersection(primFilter.collection, primFilter.renderTags, viewMatrix, projectionMatrix, @@ -1196,6 +1198,8 @@ UsdMayaGLBatchRenderer::_ComputeSelection( _selectResults.clear(); + UBOBindingsSaver bindingsSaver; + for (const PxrMayaHdPrimFilter& primFilter : primFilters) { TF_DEBUG(PXRUSDMAYAGL_BATCHED_SELECTION).Msg( " --- Intersection Testing with collection: %s\n", From 80bf6f8af763dfb9fba3fe45cd5c48eb63347197 Mon Sep 17 00:00:00 2001 From: Matt Johnson Date: Wed, 19 Feb 2020 17:36:39 -0800 Subject: [PATCH 4/5] rename UBOBindingsSaver class to GLUniformBufferBindingsSaver --- lib/render/mayaToHydra/renderOverride.cpp | 2 +- lib/render/px_vp20/utils.cpp | 4 ++-- lib/render/px_vp20/utils.h | 6 +++--- lib/render/pxrUsdMayaGL/batchRenderer.cpp | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/render/mayaToHydra/renderOverride.cpp b/lib/render/mayaToHydra/renderOverride.cpp index 11cad80519..9a1789c608 100644 --- a/lib/render/mayaToHydra/renderOverride.cpp +++ b/lib/render/mayaToHydra/renderOverride.cpp @@ -379,7 +379,7 @@ MStatus MtohRenderOverride::Render(const MHWRender::MDrawContext& drawContext) { _InitHydraResources(); } - UBOBindingsSaver bindingsSaver; + GLUniformBufferBindingsSaver bindingsSaver; _SelectionChanged(); diff --git a/lib/render/px_vp20/utils.cpp b/lib/render/px_vp20/utils.cpp index d7e6f27c45..e0c2a6f84b 100644 --- a/lib/render/px_vp20/utils.cpp +++ b/lib/render/px_vp20/utils.cpp @@ -1082,7 +1082,7 @@ px_vp20Utils::OutputDisplayStatusToStream( } -UBOBindingsSaver::UBOBindingsSaver() +GLUniformBufferBindingsSaver::GLUniformBufferBindingsSaver() { for (size_t i = 0u; i < _uniformBufferBindings.size(); ++i) { glGetIntegeri_v( @@ -1092,7 +1092,7 @@ UBOBindingsSaver::UBOBindingsSaver() } } -UBOBindingsSaver::~UBOBindingsSaver() +GLUniformBufferBindingsSaver::~GLUniformBufferBindingsSaver() { for (size_t i = 0u; i < _uniformBufferBindings.size(); ++i) { glBindBufferBase( diff --git a/lib/render/px_vp20/utils.h b/lib/render/px_vp20/utils.h index f76c8a5d76..372b8ecff2 100644 --- a/lib/render/px_vp20/utils.h +++ b/lib/render/px_vp20/utils.h @@ -135,16 +135,16 @@ class px_vp20Utils /// possible bindings, so instead we only do just enough to avoid issues. /// Empirically, the problematic binding has been the material binding at /// index 4. -class UBOBindingsSaver +class GLUniformBufferBindingsSaver { public: static constexpr size_t UNIFORM_BINDINGS_TO_SAVE = 5u; MAYAUSD_CORE_PUBLIC - UBOBindingsSaver(); + GLUniformBufferBindingsSaver(); MAYAUSD_CORE_PUBLIC - ~UBOBindingsSaver(); + ~GLUniformBufferBindingsSaver(); private: std::array _uniformBufferBindings; diff --git a/lib/render/pxrUsdMayaGL/batchRenderer.cpp b/lib/render/pxrUsdMayaGL/batchRenderer.cpp index 732df068b5..f12f9fe87b 100644 --- a/lib/render/pxrUsdMayaGL/batchRenderer.cpp +++ b/lib/render/pxrUsdMayaGL/batchRenderer.cpp @@ -993,7 +993,7 @@ UsdMayaGLBatchRenderer::TestIntersectionCustomPrimFilter( // Differs from viewport implementations in that it doesn't rely on // _ComputeSelection being called first. - UBOBindingsSaver bindingsSaver; + GLUniformBufferBindingsSaver bindingsSaver; return _TestIntersection(primFilter.collection, primFilter.renderTags, @@ -1198,7 +1198,7 @@ UsdMayaGLBatchRenderer::_ComputeSelection( _selectResults.clear(); - UBOBindingsSaver bindingsSaver; + GLUniformBufferBindingsSaver bindingsSaver; for (const PxrMayaHdPrimFilter& primFilter : primFilters) { TF_DEBUG(PXRUSDMAYAGL_BATCHED_SELECTION).Msg( @@ -1298,7 +1298,7 @@ UsdMayaGLBatchRenderer::_Render( GL_DEPTH_BUFFER_BIT | GL_VIEWPORT_BIT); - UBOBindingsSaver bindingsSaver; + GLUniformBufferBindingsSaver bindingsSaver; // hydra orients all geometry during topological processing so that // front faces have ccw winding. We disable culling because culling From 5c448b5bdb5e23bfe3aaba7a230c49cadc723e74 Mon Sep 17 00:00:00 2001 From: Matt Johnson Date: Thu, 20 Feb 2020 11:03:33 -0800 Subject: [PATCH 5/5] make UNIFORM_BINDINGS_TO_SAVE private to the class --- lib/render/px_vp20/utils.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/render/px_vp20/utils.h b/lib/render/px_vp20/utils.h index 372b8ecff2..75d213e9fa 100644 --- a/lib/render/px_vp20/utils.h +++ b/lib/render/px_vp20/utils.h @@ -138,8 +138,6 @@ class px_vp20Utils class GLUniformBufferBindingsSaver { public: - static constexpr size_t UNIFORM_BINDINGS_TO_SAVE = 5u; - MAYAUSD_CORE_PUBLIC GLUniformBufferBindingsSaver(); @@ -147,6 +145,8 @@ class GLUniformBufferBindingsSaver ~GLUniformBufferBindingsSaver(); private: + static constexpr size_t UNIFORM_BINDINGS_TO_SAVE = 5u; + std::array _uniformBufferBindings; };