Merge pull request #1859 from jonny-apple/jon/dev/cleanup

[HdSt] Remove excess parameters, change vega fix to member, support caps in pso

(Internal change: 2237160)

pxr/imaging/hdSt/pipelineDrawBatch.cpp

@@ -47,7 +47,6 @@
 
 #include "pxr/imaging/hgi/blitCmds.h"
 #include "pxr/imaging/hgi/blitCmdsOps.h"
-#include "pxr/imaging/hgi/graphicsCmds.h"
 #include "pxr/imaging/hgi/graphicsPipeline.h"
 #include "pxr/imaging/hgi/resourceBindings.h"
 
@@ -1284,9 +1283,9 @@ HdSt_PipelineDrawBatch::ExecuteDraw(
     _BindVertexBuffersForDrawing(gfxCmds, state);
 
     if (drawIndirect) {
-        _ExecuteDrawIndirect(gfxCmds, _dispatchBuffer, state.indexBar);
+        _ExecuteDrawIndirect(gfxCmds, state.indexBar);
     } else {
-        _ExecuteDrawImmediate(gfxCmds, _dispatchBuffer, state.indexBar);
+        _ExecuteDrawImmediate(gfxCmds, state.indexBar);
     }
 
     hgi->DestroyResourceBindings(&resourceBindings);
@@ -1298,20 +1297,19 @@ HdSt_PipelineDrawBatch::ExecuteDraw(
 void
 HdSt_PipelineDrawBatch::_ExecuteDrawIndirect(
     HgiGraphicsCmds * gfxCmds,
-    HdStDispatchBufferSharedPtr const & dispatchBuffer,
     HdStBufferArrayRangeSharedPtr const & indexBar)
 {
     TRACE_FUNCTION();
 
-    HdStBufferResourceSharedPtr paramBuffer = dispatchBuffer->
+    HdStBufferResourceSharedPtr paramBuffer = _dispatchBuffer->
         GetBufferArrayRange()->GetResource(HdTokens->drawDispatch);
     if (!TF_VERIFY(paramBuffer)) return;
 
     if (!_useDrawIndexed) {
         gfxCmds->DrawIndirect(
             paramBuffer->GetHandle(),
             paramBuffer->GetOffset(),
-            dispatchBuffer->GetCount(),
+            _dispatchBuffer->GetCount(),
             paramBuffer->GetStride());
     } else {
         HdStBufferResourceSharedPtr indexBuffer =
@@ -1322,7 +1320,7 @@ HdSt_PipelineDrawBatch::_ExecuteDrawIndirect(
             indexBuffer->GetHandle(),
             paramBuffer->GetHandle(),
             paramBuffer->GetOffset(),
-            dispatchBuffer->GetCount(),
+            _dispatchBuffer->GetCount(),
             paramBuffer->GetStride(),
             _drawCommandBuffer,
             _patchBaseVertexByteOffset);
@@ -1332,13 +1330,12 @@ HdSt_PipelineDrawBatch::_ExecuteDrawIndirect(
 void
 HdSt_PipelineDrawBatch::_ExecuteDrawImmediate(
     HgiGraphicsCmds * gfxCmds,
-    HdStDispatchBufferSharedPtr const & dispatchBuffer,
     HdStBufferArrayRangeSharedPtr const & indexBar)
 {
     TRACE_FUNCTION();
 
-    uint32_t const drawCount = dispatchBuffer->GetCount();
-    uint32_t const strideUInt32 = dispatchBuffer->GetCommandNumUints();
+    uint32_t const drawCount = _dispatchBuffer->GetCount();
+    uint32_t const strideUInt32 = _dispatchBuffer->GetCommandNumUints();
 
     if (!_useDrawIndexed) {
         for (uint32_t i = 0; i < drawCount; ++i) {

pxr/imaging/hdSt/pipelineDrawBatch.h

@@ -132,12 +132,10 @@ class HdSt_PipelineDrawBatch : public HdSt_DrawBatch
 
     void _ExecuteDrawIndirect(
                 HgiGraphicsCmds * gfxCmds,
-                HdStDispatchBufferSharedPtr const & dispatchBuffer,
                 HdStBufferArrayRangeSharedPtr const & indexBar);
 
     void _ExecuteDrawImmediate(
                 HgiGraphicsCmds * gfxCmds,
-                HdStDispatchBufferSharedPtr const & dispatchBuffer,
                 HdStBufferArrayRangeSharedPtr const & indexBar);
 
     void _ExecuteFrustumCull(

pxr/imaging/hgiMetal/graphicsCmds.h

@@ -143,6 +143,7 @@ class HgiMetalGraphicsCmds final : public HgiGraphicsCmds
 
     void _CreateArgumentBuffer();
     void _SyncArgumentBuffer();
+    void _VegaIndirectFix();
 
     // We implement multi-draw indirect commands on Metal by encoding
     // separate draw commands for each draw.

pxr/imaging/hgiMetal/graphicsCmds.mm

@@ -39,21 +39,6 @@
 
 PXR_NAMESPACE_OPEN_SCOPE
 
-static void
-_VegaIndirectFix(HgiMetal* hgi,
-                 id<MTLRenderCommandEncoder> encoder,
-                 MTLPrimitiveType primType)
-{
-    if (hgi->GetCapabilities()->requiresIndirectDrawFix) {
-        // Fix for Vega in macOS before 12.0.  There is state leakage between
-        // indirect draw of different prim types which results in a GPU crash.
-        // Flush with a null draw through the direct path.
-        [encoder drawPrimitives:primType
-                vertexStart:0
-                vertexCount:0];
-    }
-}
-
 HgiMetalGraphicsCmds::CachedEncoderState::CachedEncoderState()
 {
     numVertexBufferBindings = MAX_METAL_VERTEX_BUFFER_BINDINGS;
@@ -288,6 +273,27 @@
     _enableParallelEncoder = enable;
 }
 
+void
+HgiMetalGraphicsCmds::_VegaIndirectFix()
+{
+    if (!_primitiveTypeChanged) {
+        return;
+    }
+
+    if (!_hgi->GetCapabilities()->requiresIndirectDrawFix) {
+        return;
+    }
+    // Fix for Vega in macOS before 12.0.  There is state leakage between
+    // indirect draw of different prim types which results in a GPU crash.
+    // Flush with a null draw through the direct path.
+    id<MTLRenderCommandEncoder> encoder = _GetEncoder();
+    MTLPrimitiveType mtlType =
+        HgiMetalConversions::GetPrimitiveType(_primitiveType);
+    [encoder drawPrimitives:mtlType
+            vertexStart:0
+            vertexCount:0];
+}
+
 uint32_t
 HgiMetalGraphicsCmds::_GetNumEncoders()
 {
@@ -416,11 +422,10 @@
     if (_argumentBuffer) {
         if (_argumentBuffer.storageMode != MTLStorageModeShared &&
             [_argumentBuffer respondsToSelector:@selector(didModifyRange:)]) {
-            NSRange range = NSMakeRange(0, _argumentBuffer.length);
 
             ARCH_PRAGMA_PUSH
             ARCH_PRAGMA_INSTANCE_METHOD_NOT_FOUND
-            [_argumentBuffer didModifyRange:range];
+            [_argumentBuffer didModifyRange:{0, _argumentBuffer.length}];
             ARCH_PRAGMA_POP
         }
         _argumentBuffer = nil;
@@ -434,7 +439,7 @@
     double y = vp[1];
     double w = vp[2];
     double h = vp[3];
-    
+
     // Viewport is inverted in the y. Along with the front face winding order
     // being inverted.
     // This combination allows us to emulate the OpenGL coordinate space on
@@ -628,7 +633,7 @@
 
     MTLPrimitiveType type=HgiMetalConversions::GetPrimitiveType(_primitiveType);
     id<MTLRenderCommandEncoder> encoder = _GetEncoder();
-    
+
     _SetVertexBufferStepFunctionOffsets(encoder, baseInstance);
 
     if (_primitiveType == HgiPrimitiveTypePatchList) {
@@ -664,13 +669,13 @@
     uint32_t drawCount,
     uint32_t stride)
 {
-    _SyncArgumentBuffer();
-
-    HgiMetalBuffer* drawBuf =
-        static_cast<HgiMetalBuffer*>(drawParameterBuffer.Get());
+    MTLPrimitiveType mtlType =
+        HgiMetalConversions::GetPrimitiveType(_primitiveType);
+    id<MTLBuffer> drawBufferId =
+        static_cast<HgiMetalBuffer*>(drawParameterBuffer.Get())->GetBufferId();
+    const HgiCapabilities *capabilities = _hgi->GetCapabilities();
 
-    MTLPrimitiveType type=HgiMetalConversions::GetPrimitiveType(_primitiveType);
-
+    _SyncArgumentBuffer();
     static const uint32_t _drawCallsPerThread = 256;
     const uint32_t numEncoders = std::min(
                                  std::max(drawCount / _drawCallsPerThread, 1U),
@@ -703,23 +708,22 @@
                         [encoder drawPatches:controlPointCount
                             patchIndexBuffer:NULL
                       patchIndexBufferOffset:0
-                              indirectBuffer:drawBuf->GetBufferId()
+                              indirectBuffer:drawBufferId
                         indirectBufferOffset:bufferOffset];
                     }
                 }
                 else {
-                    if (_primitiveTypeChanged) {
-                        _VegaIndirectFix(_hgi, encoder, type);
-                    }
+                    _VegaIndirectFix();
+
                     for (uint32_t offset = encoderOffset;
                          offset < encoderOffset + encoderCount;
                          ++offset) {
                         _SetVertexBufferStepFunctionOffsets(encoder, offset);
                         const uint32_t bufferOffset = drawBufferByteOffset
                                                     + (offset * stride);
 
-                        [encoder drawPrimitives:type
-                                 indirectBuffer:drawBuf->GetBufferId()
+                        [encoder drawPrimitives:mtlType
+                                 indirectBuffer:drawBufferId
                            indirectBufferOffset:bufferOffset];
                     }
                 }
@@ -741,7 +745,8 @@
 
     HgiMetalBuffer* indexBuf = static_cast<HgiMetalBuffer*>(indexBuffer.Get());
 
-    MTLPrimitiveType type=HgiMetalConversions::GetPrimitiveType(_primitiveType);
+    MTLPrimitiveType mtlType =
+        HgiMetalConversions::GetPrimitiveType(_primitiveType);
 
     id<MTLRenderCommandEncoder> encoder = _GetEncoder();
 
@@ -762,7 +767,9 @@
                       instanceCount:instanceCount
                        baseInstance:baseInstance];
     } else {
-        [encoder drawIndexedPrimitives:type
+        _VegaIndirectFix();
+
+        [encoder drawIndexedPrimitives:mtlType
                             indexCount:indexCount
                              indexType:MTLIndexTypeUInt32
                            indexBuffer:indexBuf->GetBufferId()
@@ -785,16 +792,14 @@
     std::vector<uint32_t> const& drawParameterBufferUInt32,
     uint32_t patchBaseVertexByteOffset)
 {
-    _SyncArgumentBuffer();
-
-    id<MTLBuffer> indexBufferId =
-        static_cast<HgiMetalBuffer*>(indexBuffer.Get())->GetBufferId();
+    MTLPrimitiveType mtlType =
+        HgiMetalConversions::GetPrimitiveType(_primitiveType);
     id<MTLBuffer> drawBufferId =
         static_cast<HgiMetalBuffer*>(drawParameterBuffer.Get())->GetBufferId();
+    id<MTLBuffer> indexBufferId =
+        static_cast<HgiMetalBuffer*>(indexBuffer.Get())->GetBufferId();
 
-    MTLPrimitiveType type =
-        HgiMetalConversions::GetPrimitiveType(_primitiveType);
-
+    _SyncArgumentBuffer();
     static const uint32_t _drawCallsPerThread = 256;
     const uint32_t numEncoders = std::min(
                                  std::max(drawCount / _drawCallsPerThread, 1U),
@@ -846,9 +851,8 @@
                     }
                 }
                 else {
-                    if (_primitiveTypeChanged) {
-                        _VegaIndirectFix(_hgi, encoder, type);
-                    }
+                    _VegaIndirectFix();
+
                     for (uint32_t offset = encoderOffset;
                          offset < encoderOffset + encoderCount;
                          ++offset) {
@@ -857,7 +861,7 @@
                         const uint32_t bufferOffset = drawBufferByteOffset
                                                     + (offset * stride);
 
-                        [encoder drawIndexedPrimitives:type
+                        [encoder drawIndexedPrimitives:mtlType
                                              indexType:MTLIndexTypeUInt32
                                            indexBuffer:indexBufferId
                                      indexBufferOffset:0

pxr/imaging/hgiMetal/graphicsPipeline.h

@@ -30,9 +30,6 @@
 
 #include "pxr/imaging/hgiMetal/api.h"
 
-#include <vector>
-
-
 PXR_NAMESPACE_OPEN_SCOPE
 
 class HgiMetal;
@@ -60,8 +57,8 @@ class HgiMetalGraphicsPipeline final : public HgiGraphicsPipeline
     HgiMetalGraphicsPipeline(const HgiMetalGraphicsPipeline&) = delete;
 
     void _CreateVertexDescriptor();
-    void _CreateDepthStencilState(id<MTLDevice> device);
-    void _CreateRenderPipelineState(id<MTLDevice> device);
+    void _CreateDepthStencilState(HgiMetal *hgi);
+    void _CreateRenderPipelineState(HgiMetal *hgi);
 
     MTLVertexDescriptor *_vertexDescriptor;
     id<MTLDepthStencilState> _depthStencilState;

pxr/imaging/hgiMetal/graphicsPipeline.mm

@@ -46,8 +46,8 @@
     , _constantTessFactors(nil)
 {
     _CreateVertexDescriptor();
-    _CreateDepthStencilState(hgi->GetPrimaryDevice());
-    _CreateRenderPipelineState(hgi->GetPrimaryDevice());
+    _CreateDepthStencilState(hgi);
+    _CreateRenderPipelineState(hgi);
 }
 
 HgiMetalGraphicsPipeline::~HgiMetalGraphicsPipeline()
@@ -103,9 +103,7 @@
         }
 
         // Describe each vertex attribute in the vertex buffer
-        for (size_t loc = 0; loc<vas.size(); loc++) {
-            HgiVertexAttributeDesc const& va = vas[loc];
-
+        for (HgiVertexAttributeDesc const& va : vas) {
             uint32_t idx = va.shaderBindLocation;
             _vertexDescriptor.attributes[idx].format =
                 HgiMetalConversions::GetVertexFormat(va.format);
@@ -118,7 +116,7 @@
 }
 
 void
-HgiMetalGraphicsPipeline::_CreateRenderPipelineState(id<MTLDevice> device)
+HgiMetalGraphicsPipeline::_CreateRenderPipelineState(HgiMetal *hgi)
 {
     MTLRenderPipelineDescriptor *stateDesc =
         [[MTLRenderPipelineDescriptor alloc] init];
@@ -228,6 +226,7 @@
     stateDesc.vertexDescriptor = _vertexDescriptor;
 
     NSError *error = NULL;
+    id<MTLDevice> device = hgi->GetPrimaryDevice();
     _renderPipelineState = [device
         newRenderPipelineStateWithDescriptor:stateDesc
         error:&error];
@@ -261,7 +260,7 @@
 }
 
 void
-HgiMetalGraphicsPipeline::_CreateDepthStencilState(id<MTLDevice> device)
+HgiMetalGraphicsPipeline::_CreateDepthStencilState(HgiMetal *hgi)
 {
     MTLDepthStencilDescriptor *depthStencilStateDescriptor =
         [[MTLDepthStencilDescriptor alloc] init];
@@ -295,6 +294,7 @@
             _CreateStencilDescriptor(_descriptor.depthState.stencilBack);
     }
 
+    id<MTLDevice> device = hgi->GetPrimaryDevice();
     _depthStencilState = [device
         newDepthStencilStateWithDescriptor:depthStencilStateDescriptor];
     [depthStencilStateDescriptor release];