Skip to content

Commit

Permalink
Merge pull request #2042 from billhollings/VK_EXT_extended_dynamic_st…
Browse files Browse the repository at this point in the history
…ate-fixes

Fixes for VK_EXT_extended_dynamic_state.
  • Loading branch information
billhollings authored Oct 17, 2023
2 parents 61b8712 + d706ed0 commit 107cf2c
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 43 deletions.
4 changes: 4 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1711,7 +1711,11 @@
_metalFeatures.maxPerStageStorageTextureCount = 8;

_metalFeatures.vertexStrideAlignment = supportsMTLGPUFamily(Apple5) ? 1 : 4;

#if MVK_XCODE_15
// Dynamic vertex stride needs to have everything aligned - compiled with support for vertex stride calls, and supported by both runtime OS and GPU.
_metalFeatures.dynamicVertexStride = mvkOSVersionIsAtLeast(14.0, 17.0, 1.0) && (supportsMTLGPUFamily(Apple4) || supportsMTLGPUFamily(Mac2));
#endif

// GPU-specific features
switch (_properties.vendorID) {
Expand Down
1 change: 1 addition & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ enum MVKRenderStateType {
DepthTestEnable,
DepthWriteEnable,
FrontFace,
LineWidth,
LogicOp,
LogicOpEnable,
PatchControlPoints,
Expand Down
87 changes: 44 additions & 43 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ static MVKRenderStateType getRenderStateType(VkDynamicState vkDynamicState) {
case VK_DYNAMIC_STATE_DEPTH_TEST_ENABLE: return DepthTestEnable;
case VK_DYNAMIC_STATE_DEPTH_WRITE_ENABLE: return DepthWriteEnable;
case VK_DYNAMIC_STATE_FRONT_FACE: return FrontFace;
case VK_DYNAMIC_STATE_LINE_WIDTH: return LineWidth;
case VK_DYNAMIC_STATE_LOGIC_OP_EXT: return LogicOp;
case VK_DYNAMIC_STATE_LOGIC_OP_ENABLE_EXT: return LogicOpEnable;
case VK_DYNAMIC_STATE_PATCH_CONTROL_POINTS_EXT: return PatchControlPoints;
Expand Down Expand Up @@ -1366,18 +1367,16 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
const VkVertexInputBindingDescription* pVKVB = &pVI->pVertexBindingDescriptions[i];
if (shaderConfig.isVertexBufferUsed(pVKVB->binding)) {

// Vulkan allows any stride, but Metal only allows multiples of 4.
// TODO: We could try to expand the buffer to the required alignment in that case.
VkDeviceSize mtlVtxStrideAlignment = _device->_pMetalFeatures->vertexStrideAlignment;
if ((pVKVB->stride % mtlVtxStrideAlignment) != 0) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Under Metal, vertex attribute binding strides must be aligned to %llu bytes.", mtlVtxStrideAlignment));
// Vulkan allows any stride, but Metal requires multiples of 4 on older GPUs.
if (isVtxStrideStatic && (pVKVB->stride % _device->_pMetalFeatures->vertexStrideAlignment) != 0) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "Under Metal, vertex attribute binding strides must be aligned to %llu bytes.", _device->_pMetalFeatures->vertexStrideAlignment));
return false;
}

maxBinding = max(pVKVB->binding, maxBinding);
uint32_t vbIdx = getMetalBufferIndexForVertexAttributeBinding(pVKVB->binding);
auto vbDesc = inputDesc.layouts[vbIdx];
if (pVKVB->stride == 0) {
if (isVtxStrideStatic && pVKVB->stride == 0) {
// Stride can't be 0, it will be set later to attributes' maximum offset + size
// to prevent it from being larger than the underlying buffer permits.
vbDesc.stride = 0;
Expand Down Expand Up @@ -1418,52 +1417,54 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
if (shaderConfig.isShaderInputLocationUsed(pVKVA->location)) {
uint32_t vaBinding = pVKVA->binding;
uint32_t vaOffset = pVKVA->offset;
auto vaDesc = inputDesc.attributes[pVKVA->location];
auto mtlFormat = (decltype(vaDesc.format))getPixelFormats()->getMTLVertexFormat(pVKVA->format);

// Vulkan allows offsets to exceed the buffer stride, but Metal doesn't.
// If this is the case, fetch a translated artificial buffer binding, using the same MTLBuffer,
// but that is translated so that the reduced VA offset fits into the binding stride.
const VkVertexInputBindingDescription* pVKVB = pVI->pVertexBindingDescriptions;
uint32_t attrSize = 0;
for (uint32_t j = 0; j < vbCnt; j++, pVKVB++) {
if (pVKVB->binding == pVKVA->binding) {
attrSize = getPixelFormats()->getBytesPerBlock(pVKVA->format);
if (pVKVB->stride == 0) {
// The step is set to constant, but we need to change stride to be non-zero for metal.
// Look for the maximum offset + size to set as the stride.
uint32_t vbIdx = getMetalBufferIndexForVertexAttributeBinding(pVKVB->binding);
auto vbDesc = inputDesc.layouts[vbIdx];
uint32_t strideLowBound = vaOffset + attrSize;
if (vbDesc.stride < strideLowBound) vbDesc.stride = strideLowBound;
} else if (vaOffset && vaOffset + attrSize > pVKVB->stride) {
// Move vertex attribute offset into the stride. This vertex attribute may be
// combined with other vertex attributes into the same translated buffer binding.
// But if the reduced offset combined with the vertex attribute size still won't
// fit into the buffer binding stride, force the vertex attribute offset to zero,
// effectively dedicating this vertex attribute to its own buffer binding.
uint32_t origOffset = vaOffset;
vaOffset %= pVKVB->stride;
if (vaOffset + attrSize > pVKVB->stride) {
vaOffset = 0;
if (isVtxStrideStatic) {
const VkVertexInputBindingDescription* pVKVB = pVI->pVertexBindingDescriptions;
uint32_t attrSize = 0;
for (uint32_t j = 0; j < vbCnt; j++, pVKVB++) {
if (pVKVB->binding == pVKVA->binding) {
attrSize = getPixelFormats()->getBytesPerBlock(pVKVA->format);
if (pVKVB->stride == 0) {
// The step is set to constant, but we need to change stride to be non-zero for metal.
// Look for the maximum offset + size to set as the stride.
uint32_t vbIdx = getMetalBufferIndexForVertexAttributeBinding(pVKVB->binding);
auto vbDesc = inputDesc.layouts[vbIdx];
uint32_t strideLowBound = vaOffset + attrSize;
if (vbDesc.stride < strideLowBound) vbDesc.stride = strideLowBound;
} else if (vaOffset && vaOffset + attrSize > pVKVB->stride) {
// Move vertex attribute offset into the stride. This vertex attribute may be
// combined with other vertex attributes into the same translated buffer binding.
// But if the reduced offset combined with the vertex attribute size still won't
// fit into the buffer binding stride, force the vertex attribute offset to zero,
// effectively dedicating this vertex attribute to its own buffer binding.
uint32_t origOffset = vaOffset;
vaOffset %= pVKVB->stride;
if (vaOffset + attrSize > pVKVB->stride) {
vaOffset = 0;
}
vaBinding = getTranslatedVertexBinding(vaBinding, origOffset - vaOffset, maxBinding);
if (zeroDivisorBindings.count(pVKVB->binding)) {
zeroDivisorBindings.insert(vaBinding);
}
}
vaBinding = getTranslatedVertexBinding(vaBinding, origOffset - vaOffset, maxBinding);
if (zeroDivisorBindings.count(pVKVB->binding)) {
zeroDivisorBindings.insert(vaBinding);
}
break;
}
break;
}
if (pVKVB->stride && attrSize > pVKVB->stride) {
/* Metal does not support overlapping loads. Truncate format vector length to prevent an assertion
* and hope it's not used by the shader. */
MTLVertexFormat newFormat = mvkAdjustFormatVectorToSize((MTLVertexFormat)mtlFormat, pVKVB->stride);
reportError(VK_SUCCESS, "Found attribute with size (%u) larger than it's binding's stride (%u). Changing descriptor format from %s to %s.",
attrSize, pVKVB->stride, getPixelFormats()->getName((MTLVertexFormat)mtlFormat), getPixelFormats()->getName(newFormat));
mtlFormat = (decltype(vaDesc.format))newFormat;
}
}

auto vaDesc = inputDesc.attributes[pVKVA->location];
auto mtlFormat = (decltype(vaDesc.format))getPixelFormats()->getMTLVertexFormat(pVKVA->format);
if (pVKVB->stride && attrSize > pVKVB->stride) {
/* Metal does not support overlapping loads. Truncate format vector length to prevent an assertion
* and hope it's not used by the shader. */
MTLVertexFormat newFormat = mvkAdjustFormatVectorToSize((MTLVertexFormat)mtlFormat, pVKVB->stride);
reportError(VK_SUCCESS, "Found attribute with size (%u) larger than it's binding's stride (%u). Changing descriptor format from %s to %s.",
attrSize, pVKVB->stride, getPixelFormats()->getName((MTLVertexFormat)mtlFormat), getPixelFormats()->getName(newFormat));
mtlFormat = (decltype(vaDesc.format))newFormat;
}
vaDesc.format = mtlFormat;
vaDesc.bufferIndex = (decltype(vaDesc.bufferIndex))getMetalBufferIndexForVertexAttributeBinding(vaBinding);
vaDesc.offset = vaOffset;
Expand Down

0 comments on commit 107cf2c

Please sign in to comment.