More detailed design of pipelines to start discussions - #29
Conversation
|
Note that you can use the "rich diff" to see the file formatted. |
| For example a ```DepthStencilState``` object would be allocated and a pointer to it would be stored in the ```RenderPipelineDescriptor```. | ||
|
|
||
| Mismatch: | ||
| - Metal doesn’t have primitive restart. |
There was a problem hiding this comment.
Primitive restart functionality is enabled with the largest unsigned integer index value, relative to indexType (0xFFFF for MTLIndexTypeUInt16 or 0xFFFFFFFF for MTLIndexTypeUInt32). This feature finishes drawing the current primitive at the specified index and starts drawing a new one with the next index.
There was a problem hiding this comment.
What Metal actually doesn't have is a way to disable the primitive restart functionality.
There was a problem hiding this comment.
Thanks, for the pointers. Updated the design with this but it showed a new mismatch with D3D12.
| Mismatch: | ||
| - Metal doesn’t have primitive restart. | ||
| - Metal doesn’t have a sample mask. | ||
| - Vulkan can have some state like scissor and viewport set on the pipeline as an optimization on some GPUs. |
There was a problem hiding this comment.
I don't think it's worth for us to take this into account, given how unique this ability is and how easy it is in Vulkan to create PSO without those states baked in.
There was a problem hiding this comment.
Agreed, added that all dynamic states are always set in the Vulkan.
| int subpassIndex; | ||
|
|
||
| // Fixed function state | ||
| DepthStencilState* depthStencil; |
There was a problem hiding this comment.
I don't think we need to have parts of the PSO descriptors like DepthStencilState being separate objects
There was a problem hiding this comment.
For simplicity we assume most fixed-function state is created in separate object.
Added the following to explain it is a cosmetic decision that will be left for later.
This is part of the UX of the API and could be replaced with chained structure like Vulkan or member structure like D3D12.
| Translation to the backing APIs would be the following: | ||
| - **D3D12**: Translates to ```ID3D12::CreateComputePipelineState``` | ||
| - **Metal**: Translates to ```MTLDevice::makeRenderPipelineState``` | ||
| - **Vulkan**: Translates to ```vkCreateGraphicsPipelines``` |
There was a problem hiding this comment.
mismatch: Vulkan allows creating PSOs in bulk, and it's easy to emulate on other APIs
| - **D3D12**: Translates to a ```D3D12_INPUT_DESC```. | ||
| Each enabled attribute corresponds to a ```D3D12_INPUT_ELEMENT_DESC``` with ```InputSlot``` being the index of the attribute. | ||
| Other members of the ```D3D12_INPUT_ELEMENT_DESC``` are translated trivially. | ||
| The stride is looked up in the pipeline state before calls to ```ID3D12GraphicsCommandList::IASetVertexBuffers```. |
There was a problem hiding this comment.
the d3d12 backend may need to defer the setup of vertex buffers until the actual draw call, since any PSO binding between the vertex buffer setup and the draw call would potentially change the strides (assuming we keep the strides in PSO)
There was a problem hiding this comment.
Good point, I need to add an NXT test for it as we have this wrong for now :)
| ```depthTestEnable``` would be set to ```depthCompare != Always```. | ||
|
|
||
| Open question: | ||
| what about Vulkan’s ```VkPipelineDepthStencilStateCreateInfo::depthBoundTestEnable```? |
There was a problem hiding this comment.
partially supported by d3d12, but I don't see it on Metal: https://msdn.microsoft.com/en-us/library/windows/desktop/mt492556(v=vs.85).aspx
| - **Vulkan**: Translates to ```vkCreateGraphicsPipelines``` | ||
| - **Vulkan**: Translates to ```vkCreateGraphicsPipelines```. ```VkPipelineInputAssemblyStateCreateInfo```'s ```primitiveRestartEnable``` is always set to true. All dynamic states are set on all pipelines. | ||
|
|
||
| Open question: should the type of the indices be set in ```RenderPipelineDescriptor```? If not, how is the D3D12 ```IBStripCutValue``` chosen? |
There was a problem hiding this comment.
Good question. We don't want to be delaying the PSO creation ever, so my vote is to specify the index type in RenderPipelineDescriptor so that D3D12 can set the proper IBStripCutValue
No description provided.