From ed4c3756d06933c57018a4212fb4caddfbb4e436 Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Tue, 23 Aug 2022 13:48:18 -0400 Subject: [PATCH] Enable primitive restart for list topologies This is supported by OpenGL, and on Vulkan with VK_EXT_primitive_topology_list_restart. On Vulkan, all drivers are known to support this even without VK_EXT_primitive_topology_list_restart. On D3D, primitive restart is only supported for strip topologies. Previously, RenderDoc specifically disabled primitive restart for non-strip topologies. In this change, that is no longer done. If the app enables primitive restart, so will RenderDoc behave accordingly. It would be the responsibility of the app to avoid primitive restart if the API doesn't allow it. --- .../python_api/renderdoc/pipelines/common.rst | 2 -- qrenderdoc/Windows/BufferViewer.cpp | 11 +++----- renderdoc/api/replay/pipestate.h | 6 ++-- renderdoc/api/replay/pipestate.inl | 11 +++++--- renderdoc/api/replay/replay_enums.h | 28 ------------------- renderdoc/driver/gl/gl_postvs.cpp | 5 ++-- renderdoc/driver/vulkan/vk_postvs.cpp | 3 +- renderdoc/driver/vulkan/vk_rendermesh.cpp | 2 +- renderdoc/replay/replay_driver.cpp | 7 ++--- util/test/rdtest/analyse.py | 8 +++--- .../tests/D3D11/D3D11_Primitive_Restart.py | 2 +- util/test/tests/GL/GL_Draw_Zoo.py | 8 +++--- util/test/tests/Iter_Test.py | 4 +-- util/test/tests/Vulkan/VK_Int8_IBuffer.py | 2 +- 14 files changed, 33 insertions(+), 66 deletions(-) diff --git a/docs/python_api/renderdoc/pipelines/common.rst b/docs/python_api/renderdoc/pipelines/common.rst index 37d757d6a..afd53a6fa 100644 --- a/docs/python_api/renderdoc/pipelines/common.rst +++ b/docs/python_api/renderdoc/pipelines/common.rst @@ -30,8 +30,6 @@ Vertex Inputs .. autofunction:: renderdoc.VertexOffset .. autofunction:: renderdoc.PatchList_Count .. autofunction:: renderdoc.PatchList_Topology -.. autofunction:: renderdoc.SupportsRestart -.. autofunction:: renderdoc.IsStrip Shader Resource Bindings ------------------------ diff --git a/qrenderdoc/Windows/BufferViewer.cpp b/qrenderdoc/Windows/BufferViewer.cpp index d79bb2b35..12ebddb6d 100644 --- a/qrenderdoc/Windows/BufferViewer.cpp +++ b/qrenderdoc/Windows/BufferViewer.cpp @@ -2825,10 +2825,9 @@ void BufferViewer::OnEventChanged(uint32_t eventId) const PipeState &pipe = m_Ctx.CurPipelineState(); - if(pipe.IsStripRestartEnabled() && action && (action->flags & ActionFlags::Indexed) && - SupportsRestart(pipe.GetPrimitiveTopology())) + if(pipe.IsRestartEnabled() && action && (action->flags & ActionFlags::Indexed)) { - bufdata->vsinConfig.primRestart = pipe.GetStripRestartIndex(); + bufdata->vsinConfig.primRestart = pipe.GetRestartIndex(); if(pipe.GetIBuffer().byteStride == 1) bufdata->vsinConfig.primRestart &= 0xff; @@ -3784,10 +3783,8 @@ void BufferViewer::UI_CalculateMeshFormats() m_VSInPosition = MeshFormat(); m_VSInSecondary = MeshFormat(); - m_VSInPosition.allowRestart = pipe.IsStripRestartEnabled() && - (action->flags & ActionFlags::Indexed) && - SupportsRestart(pipe.GetPrimitiveTopology()); - m_VSInPosition.restartIndex = pipe.GetStripRestartIndex(); + m_VSInPosition.allowRestart = pipe.IsRestartEnabled() && (action->flags & ActionFlags::Indexed); + m_VSInPosition.restartIndex = pipe.GetRestartIndex(); const BufferConfiguration &vsinConfig = m_ModelVSIn->getConfig(); diff --git a/renderdoc/api/replay/pipestate.h b/renderdoc/api/replay/pipestate.h index 49f4a696b..47c9eb7e8 100644 --- a/renderdoc/api/replay/pipestate.h +++ b/renderdoc/api/replay/pipestate.h @@ -311,14 +311,14 @@ For some APIs that don't distinguish by entry point, this may be empty. :return: A boolean indicating if primitive restart is enabled. :rtype: bool )"); - bool IsStripRestartEnabled() const; + bool IsRestartEnabled() const; DOCUMENT(R"(Retrieves the primitive restart index. -:return: The index value that represents a strip restart not a real index. +:return: The index value that represents a primitive restart not a real index. :rtype: int )"); - uint32_t GetStripRestartIndex() const; + uint32_t GetRestartIndex() const; DOCUMENT(R"(Retrieves the currently bound vertex buffers. diff --git a/renderdoc/api/replay/pipestate.inl b/renderdoc/api/replay/pipestate.inl index a0b2b0c17..cd14f58ed 100644 --- a/renderdoc/api/replay/pipestate.inl +++ b/renderdoc/api/replay/pipestate.inl @@ -503,14 +503,17 @@ BoundVBuffer PipeState::GetIBuffer() const return ret; } -bool PipeState::IsStripRestartEnabled() const +bool PipeState::IsRestartEnabled() const { if(IsCaptureLoaded()) { if(IsCaptureD3D11()) { - // D3D11 this is always enabled - return true; + // D3D11 this is always enabled for strips + const Topology topology = m_D3D11->inputAssembly.topology; + return topology == Topology::LineStrip || topology == Topology::TriangleStrip || + topology == Topology::LineStrip_Adj || topology == Topology::TriangleStrip_Adj || + topology == Topology::TriangleFan; } else if(IsCaptureD3D12()) { @@ -529,7 +532,7 @@ bool PipeState::IsStripRestartEnabled() const return false; } -uint32_t PipeState::GetStripRestartIndex() const +uint32_t PipeState::GetRestartIndex() const { if(IsCaptureLoaded()) { diff --git a/renderdoc/api/replay/replay_enums.h b/renderdoc/api/replay/replay_enums.h index ee33d3644..4b7255bfc 100644 --- a/renderdoc/api/replay/replay_enums.h +++ b/renderdoc/api/replay/replay_enums.h @@ -2239,34 +2239,6 @@ constexpr inline uint32_t PatchList_Count(Topology topology) : uint32_t(topology) - uint32_t(Topology::PatchList_1CPs) + 1; } -DOCUMENT(R"(Check whether or not this topology supports primitive restart. - -.. note:: This is almost but not quite the same as being a line/triangle strip - triangle fans - also support restart. See also :func:`IsStrip`. - -:param Topology topology: The topology to check. -:return: ``True`` if it describes a topology that allows restart, ``False`` for a list. -:rtype: bool -)"); -constexpr inline bool SupportsRestart(Topology topology) -{ - return topology == Topology::LineStrip || topology == Topology::TriangleStrip || - topology == Topology::LineStrip_Adj || topology == Topology::TriangleStrip_Adj || - topology == Topology::TriangleFan; -} - -DOCUMENT(R"(Check whether or not this is a strip-type topology. - -:param Topology topology: The topology to check. -:return: ``True`` if it describes a strip topology, ``False`` for a list. -:rtype: bool -)"); -constexpr inline bool IsStrip(Topology topology) -{ - return topology == Topology::LineStrip || topology == Topology::TriangleStrip || - topology == Topology::LineStrip_Adj || topology == Topology::TriangleStrip_Adj; -} - DOCUMENT(R"(The stage in a pipeline where a shader runs .. data:: Vertex diff --git a/renderdoc/driver/gl/gl_postvs.cpp b/renderdoc/driver/gl/gl_postvs.cpp index 53e40d802..cd088a9c2 100644 --- a/renderdoc/driver/gl/gl_postvs.cpp +++ b/renderdoc/driver/gl/gl_postvs.cpp @@ -955,9 +955,8 @@ void GLReplay::InitPostVSBuffers(uint32_t eventId) uint32_t stripRestartValue32 = 0; - if(SupportsRestart(drawParams.topo) && - (rs.Enabled[GLRenderState::eEnabled_PrimitiveRestart] || - rs.Enabled[GLRenderState::eEnabled_PrimitiveRestartFixedIndex])) + if(rs.Enabled[GLRenderState::eEnabled_PrimitiveRestart] || + rs.Enabled[GLRenderState::eEnabled_PrimitiveRestartFixedIndex]) { stripRestartValue32 = rs.Enabled[GLRenderState::eEnabled_PrimitiveRestartFixedIndex] ? ~0U diff --git a/renderdoc/driver/vulkan/vk_postvs.cpp b/renderdoc/driver/vulkan/vk_postvs.cpp index c6ede3b72..d554c0c50 100644 --- a/renderdoc/driver/vulkan/vk_postvs.cpp +++ b/renderdoc/driver/vulkan/vk_postvs.cpp @@ -1728,8 +1728,7 @@ void VulkanReplay::FetchVSOut(uint32_t eventId, VulkanRenderState &state) if(action->flags & ActionFlags::Indexed) { - const bool restart = pipeCreateInfo.pInputAssemblyState->primitiveRestartEnable && - SupportsRestart(MakePrimitiveTopology(state.primitiveTopology, 3)); + const bool restart = pipeCreateInfo.pInputAssemblyState->primitiveRestartEnable == VK_TRUE; bytebuf idxdata; rdcarray indices; uint8_t *idx8 = NULL; diff --git a/renderdoc/driver/vulkan/vk_rendermesh.cpp b/renderdoc/driver/vulkan/vk_rendermesh.cpp index ea9f3a4eb..edf91cb48 100644 --- a/renderdoc/driver/vulkan/vk_rendermesh.cpp +++ b/renderdoc/driver/vulkan/vk_rendermesh.cpp @@ -197,7 +197,7 @@ VKMeshDisplayPipelines VulkanDebugManager::CacheMeshDisplayPipelines(VkPipelineL false, }; - ia.primitiveRestartEnable = primary.allowRestart && SupportsRestart(primary.topology); + ia.primitiveRestartEnable = primary.allowRestart; VkRect2D scissor = {{0, 0}, {16384, 16384}}; diff --git a/renderdoc/replay/replay_driver.cpp b/renderdoc/replay/replay_driver.cpp index 4b2856483..e9736ee23 100644 --- a/renderdoc/replay/replay_driver.cpp +++ b/renderdoc/replay/replay_driver.cpp @@ -804,8 +804,7 @@ FloatVector HighlightCache::InterpretVertex(const byte *data, uint32_t vert, con vert = indices[vert]; - if(SupportsRestart(cfg.position.topology) && cfg.position.topology != Topology::TriangleFan && - cfg.position.allowRestart) + if(cfg.position.topology != Topology::TriangleFan && cfg.position.allowRestart) { if((cfg.position.indexByteStride == 1 && vert == 0xff) || (cfg.position.indexByteStride == 2 && vert == 0xffff) || @@ -933,7 +932,7 @@ void HighlightCache::CacheHighlightingData(uint32_t eventId, const MeshDisplay & maxIndex += add; uint32_t primRestart = 0; - if(SupportsRestart(cfg.position.topology) && cfg.position.allowRestart) + if(cfg.position.allowRestart) { if(cfg.position.indexByteStride == 1) primRestart = 0xff; @@ -1001,7 +1000,7 @@ bool HighlightCache::FetchHighlightPositions(const MeshDisplay &cfg, FloatVector activeVertex = InterpretVertex(data, idx, cfg, dataEnd, true, valid); uint32_t primRestart = 0; - if(SupportsRestart(meshtopo) && cfg.position.allowRestart) + if(cfg.position.allowRestart) { if(cfg.position.indexByteStride == 1) primRestart = 0xff; diff --git a/util/test/rdtest/analyse.py b/util/test/rdtest/analyse.py index 229870bf0..bd145b456 100644 --- a/util/test/rdtest/analyse.py +++ b/util/test/rdtest/analyse.py @@ -58,8 +58,8 @@ def open_capture(filename="", cap: rd.CaptureFile=None, opts: rd.ReplayOptions=N def fetch_indices(controller: rd.ReplayController, action: rd.ActionDescription, mesh: rd.MeshFormat, index_offset: int, first_index: int, num_indices: int): pipe = controller.GetPipelineState() - restart_idx = pipe.GetStripRestartIndex() & ((1 << (mesh.indexByteStride*8)) - 1) - restart_enabled = pipe.IsStripRestartEnabled() and rd.IsStrip(pipe.GetPrimitiveTopology()) + restart_idx = pipe.GetRestartIndex() & ((1 << (mesh.indexByteStride*8)) - 1) + restart_enabled = pipe.IsRestartEnabled() # If we have an index buffer if mesh.indexResourceId != rd.ResourceId.Null(): @@ -282,8 +282,8 @@ def decode_mesh_data(controller: rd.ReplayController, indices: List[int], displa # Calculate the strip restart index for this index width striprestart_index = None - if controller.GetPipelineState().IsStripRestartEnabled() and attrs[0].mesh.indexResourceId != rd.ResourceId.Null(): - striprestart_index = (controller.GetPipelineState().GetStripRestartIndex() & + if controller.GetPipelineState().IsRestartEnabled() and attrs[0].mesh.indexResourceId != rd.ResourceId.Null(): + striprestart_index = (controller.GetPipelineState().GetRestartIndex() & ((1 << (attrs[0].mesh.indexByteStride*8)) - 1)) for i,idx in enumerate(indices): diff --git a/util/test/tests/D3D11/D3D11_Primitive_Restart.py b/util/test/tests/D3D11/D3D11_Primitive_Restart.py index 17f413456..b8eaf839b 100644 --- a/util/test/tests/D3D11/D3D11_Primitive_Restart.py +++ b/util/test/tests/D3D11/D3D11_Primitive_Restart.py @@ -19,7 +19,7 @@ class D3D11_Primitive_Restart(rdtest.TestCase): ib = pipe.GetIBuffer() # Calculate the strip restart index for this index width - striprestart_index = pipe.GetStripRestartIndex() & ((1 << (ib.byteStride*8)) - 1) + striprestart_index = pipe.GetRestartIndex() & ((1 << (ib.byteStride*8)) - 1) # We don't check all of the output, we check a few key vertices to ensure they match up postvs_ref = { diff --git a/util/test/tests/GL/GL_Draw_Zoo.py b/util/test/tests/GL/GL_Draw_Zoo.py index a606144ba..1b421da0e 100644 --- a/util/test/tests/GL/GL_Draw_Zoo.py +++ b/util/test/tests/GL/GL_Draw_Zoo.py @@ -22,8 +22,8 @@ class GL_Draw_Zoo(rdtest.Draw_Zoo): ib = pipe.GetIBuffer() - self.check(pipe.IsStripRestartEnabled()) - self.check((pipe.GetStripRestartIndex() & ((1 << (ib.byteStride*8)) - 1)) == 0xffff) + self.check(pipe.IsRestartEnabled()) + self.check((pipe.GetRestartIndex() & ((1 << (ib.byteStride*8)) - 1)) == 0xffff) action = self.find_action("GL_PRIMITIVE_RESTART_FIXED_INDEX") @@ -37,8 +37,8 @@ class GL_Draw_Zoo(rdtest.Draw_Zoo): ib = pipe.GetIBuffer() - self.check(pipe.IsStripRestartEnabled()) - self.check((pipe.GetStripRestartIndex() & ((1 << (ib.byteStride*8)) - 1)) == 0xffff) + self.check(pipe.IsRestartEnabled()) + self.check((pipe.GetRestartIndex() & ((1 << (ib.byteStride*8)) - 1)) == 0xffff) action = self.find_action("GL_ClearDepth") self.check(action is not None) diff --git a/util/test/tests/Iter_Test.py b/util/test/tests/Iter_Test.py index 0bf4fb2f3..1a642b082 100644 --- a/util/test/tests/Iter_Test.py +++ b/util/test/tests/Iter_Test.py @@ -95,9 +95,9 @@ class Iter_Test(rdtest.TestCase): idx = indices[0] - striprestart_index = pipe.GetStripRestartIndex() & ((1 << (ib.byteStride*8)) - 1) + striprestart_index = pipe.GetRestartIndex() & ((1 << (ib.byteStride*8)) - 1) - if pipe.IsStripRestartEnabled() and idx == striprestart_index: + if pipe.IsRestartEnabled() and idx == striprestart_index: return rdtest.log.print("Debugging vtx %d idx %d (inst %d)" % (vtx, idx, inst)) diff --git a/util/test/tests/Vulkan/VK_Int8_IBuffer.py b/util/test/tests/Vulkan/VK_Int8_IBuffer.py index 2ae06c707..40ef2847f 100644 --- a/util/test/tests/Vulkan/VK_Int8_IBuffer.py +++ b/util/test/tests/Vulkan/VK_Int8_IBuffer.py @@ -19,7 +19,7 @@ class VK_Int8_IBuffer(rdtest.TestCase): ib = pipe.GetIBuffer() # Calculate the strip restart index for this index width - striprestart_index = pipe.GetStripRestartIndex() & ((1 << (ib.byteStride*8)) - 1) + striprestart_index = pipe.GetRestartIndex() & ((1 << (ib.byteStride*8)) - 1) # We don't check all of the output, we check a few key vertices to ensure they match up postvs_ref = {