Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ struct MVKImplicitBufferData {
MVKSmallVector<uint32_t, 8> textureSwizzles;
MVKSmallVector<uint32_t, 8> bufferSizes;
MVKSmallVector<uint32_t, 8> dynamicOffsets;
uint32_t emulatedReversedDepthViewport = 0;
};

enum class MVKResourceUsageStages : uint8_t {
Expand Down Expand Up @@ -476,6 +477,8 @@ class MVKCommandEncoderState {
}
/** Binds the given graphics pipeline to the Vulkan graphics state, invalidating any necessary resources. */
void bindGraphicsPipeline(MVKGraphicsPipeline* pipeline);
/** Updates whether graphics shaders should emulate a reversed-depth viewport. */
void setGraphicsEmulatedReversedDepthViewport(uint32_t state);
/** Binds the given compute pipeline to the Vulkan graphics state, invalidating any necessary resources. */
void bindComputePipeline(MVKComputePipeline* pipeline);
/** Binds the given push constants to the Vulkan state, invalidating any necessary resources. */
Expand Down
35 changes: 33 additions & 2 deletions MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,14 @@ static void bindMetalResources(id<MTLCommandEncoder> encoder,
binder.setBytes(encoder, viewRange, sizeof(viewRange), idx);
break;
}
case MVKNonVolatileImplicitBuffer::EmulatedReversedDepthViewport:
bindImmediateData(encoder,
mvkEncoder,
reinterpret_cast<const uint8_t*>(&implicitBufferData.emulatedReversedDepthViewport),
sizeof(implicitBufferData.emulatedReversedDepthViewport),
idx,
binder);
break;
case MVKNonVolatileImplicitBuffer::Count:
assert(0);
break;
Expand Down Expand Up @@ -1107,6 +1115,10 @@ static uint32_t getSampleCount(VkSampleCountFlags vk) {

static constexpr MVKRenderStateFlags FlagsHandledByBindStateData = FlagsViewportScissor | FlagsMetalState;

static bool shouldEmulateReversedDepthViewport(const MVKPhysicalDevice* physicalDevice) {
return physicalDevice->shouldEmulateReversedDepthViewport();
}

void MVKMetalGraphicsCommandEncoderState::bindStateData(
id<MTLRenderCommandEncoder> encoder,
MVKCommandEncoder& mvkEncoder,
Expand All @@ -1121,14 +1133,20 @@ static uint32_t getSampleCount(VkSampleCountFlags vk) {
mvkCopy(_viewports, viewports, data.numViewports);
MTLViewport mtlViewports[kMVKMaxViewportScissorCount];
uint32_t numViewports = data.numViewports;
bool isEmulatingReversedDepthViewport = false;
bool shouldEmulateReversedDepthViewports = shouldEmulateReversedDepthViewport(mvkEncoder.getDevice()->getPhysicalDevice());
for (uint32_t i = 0; i < numViewports; i++) {
mtlViewports[i].width = viewports[i].width;
mtlViewports[i].height = viewports[i].height;
mtlViewports[i].originX = viewports[i].x;
mtlViewports[i].originY = viewports[i].y;
mtlViewports[i].znear = viewports[i].minDepth;
mtlViewports[i].zfar = viewports[i].maxDepth;
bool isReversedDepthViewport = viewports[i].minDepth > viewports[i].maxDepth;
// Only reversed Vulkan depth ranges are emulated. Normal depth ranges are passed to Metal unchanged.
isEmulatingReversedDepthViewport = isEmulatingReversedDepthViewport || (shouldEmulateReversedDepthViewports && isReversedDepthViewport);
mtlViewports[i].znear = shouldEmulateReversedDepthViewports && isReversedDepthViewport ? 0.0 : viewports[i].minDepth;
mtlViewports[i].zfar = shouldEmulateReversedDepthViewports && isReversedDepthViewport ? 1.0 : viewports[i].maxDepth;
Comment on lines +1146 to +1147

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be viewports[i].maxDepth : viewports[i].minDepth and viewports[i].minDepth : viewports[i].maxDepth in case you are wondering. but keeping 0.0 / 1.0 seems easier to read for a draft

}
mvkEncoder.getState().setGraphicsEmulatedReversedDepthViewport(isEmulatingReversedDepthViewport ? 1 : 0);
if (numViewports == 1) {
[encoder setViewport:mtlViewports[0]];
} else {
Expand Down Expand Up @@ -1684,6 +1702,19 @@ static void invalidateImplicitBuffer(MVKCommandEncoderState& state, VkPipelineBi
state.applyToActiveMTLState(bindPoint, [buffer](auto& mtl){ invalidateImplicitBuffer(mtl, buffer); });
}

void MVKCommandEncoderState::setGraphicsEmulatedReversedDepthViewport(uint32_t state) {
bool changed = false;
for (auto& stageData : _vkGraphics._implicitBufferData) {
if (stageData.emulatedReversedDepthViewport != state) {
stageData.emulatedReversedDepthViewport = state;
changed = true;
}
}
if (changed) {
invalidateImplicitBuffer(*this, VK_PIPELINE_BIND_POINT_GRAPHICS, MVKNonVolatileImplicitBuffer::EmulatedReversedDepthViewport);
}
}

void MVKCommandEncoderState::bindGraphicsPipeline(MVKGraphicsPipeline* pipeline) {
_mtlGraphics.changePipeline(_vkGraphics._pipeline, pipeline);
_vkGraphics._pipeline = pipeline;
Expand Down
6 changes: 6 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ class MVKPhysicalDevice : public MVKDispatchableVulkanAPIObject {
/** Returns whether the GPU supports exactly Metal Mac GPU Family 1. */
bool isMacGPUFamily1() const;

/** Returns whether this GPU is in the affected AMD Mac family for the reversed-depth viewport workaround. */
bool needsAMDMac2ReversedDepthViewportWorkaround() const;

/** Returns whether reversed-depth viewports should be emulated instead of passed through to Metal. */
bool shouldEmulateReversedDepthViewport() const;

/** Populates the specified structure with the format properties of this device. */
void getFormatProperties(VkFormat format, VkFormatProperties* pFormatProperties);

Expand Down
8 changes: 8 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -3324,6 +3324,14 @@ static uint32_t mvkGetEntryProperty(io_registry_entry_t entry, CFStringRef prope
return !_gpuCapabilities.isAppleGPU && _gpuCapabilities.supportsMac1 && !_gpuCapabilities.supportsMac2;
}

bool MVKPhysicalDevice::needsAMDMac2ReversedDepthViewportWorkaround() const {
return _properties.vendorID == kAMDVendorId && !_gpuCapabilities.isAppleGPU && _gpuCapabilities.supportsMac2 && !_gpuCapabilities.supportsMetal3;
}

bool MVKPhysicalDevice::shouldEmulateReversedDepthViewport() const {
return needsAMDMac2ReversedDepthViewportWorkaround();
}

void MVKPhysicalDevice::initMemoryProperties() {

mvkClear(&_memoryProperties); // Start with everything cleared
Expand Down
28 changes: 25 additions & 3 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,7 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
case MVKImplicitBuffer::BufferSize: return "buffer size";
case MVKImplicitBuffer::DynamicOffset: return "dynamic offset";
case MVKImplicitBuffer::ViewRange: return "view range";
case MVKImplicitBuffer::EmulatedReversedDepthViewport: return "emulated reversed-depth viewport";
case MVKImplicitBuffer::IndirectParams: return "indirect parameter";
case MVKImplicitBuffer::Output: return "per-vertex output";
case MVKImplicitBuffer::PatchOutput: return "per-patch output";
Expand Down Expand Up @@ -1472,6 +1473,11 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
dst.options.mslOptions.dynamic_offsets_buffer_index = src[MVKImplicitBuffer::DynamicOffset];
}

static void setEmulatedReversedDepthViewportConfig(SPIRVToMSLConversionConfiguration& shaderConfig, const MVKOnePerEnumEntry<uint8_t, MVKImplicitBuffer>& implicit, bool enable) {
shaderConfig.options.shouldEmulateReversedDepthViewportDynamically = enable;
shaderConfig.options.emulatedReversedDepthViewportBufferIndex = enable ? implicit[MVKImplicitBuffer::EmulatedReversedDepthViewport] : 0;
}

bool MVKGraphicsPipeline::verifyImplicitBuffers(MVKShaderStage stage) {
const char* stageNames[] = {
"Vertex",
Expand Down Expand Up @@ -1499,6 +1505,7 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
shaderConfig.options.mslOptions.draw_id_buffer_index = implicit[MVKImplicitBuffer::DrawId];
shaderConfig.options.mslOptions.capture_output_to_buffer = false;
shaderConfig.options.mslOptions.disable_rasterization = !_isRasterizing;
setEmulatedReversedDepthViewportConfig(shaderConfig, implicit, getPhysicalDevice()->shouldEmulateReversedDepthViewport());
addVertexInputToShaderConversionConfig(shaderConfig, pCreateInfo);

MVKMTLFunction func = getMTLFunction(shaderConfig, pVertexSS, pVertexFB, _vertexModule, "Vertex");
Expand All @@ -1509,6 +1516,7 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
auto& funcRslts = func.shaderConversionResults;
plDesc.rasterizationEnabled = !funcRslts.isRasterizationDisabled;
populateResourceUsage(_stageResources[kMVKShaderStageVertex], shaderConfig, funcRslts, spv::ExecutionModelVertex);
_stageResources[kMVKShaderStageVertex].implicitBuffers.needed.set(MVKImplicitBuffer::EmulatedReversedDepthViewport, shaderConfig.options.shouldEmulateReversedDepthViewportDynamically);
_layout->populateBindOperations(_stageResources[kMVKShaderStageVertex].bindScript, shaderConfig, spv::ExecutionModelVertex);

if (funcRslts.isRasterizationDisabled) {
Expand All @@ -1535,6 +1543,7 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
shaderConfig.options.mslOptions.capture_output_to_buffer = true;
shaderConfig.options.mslOptions.vertex_for_tessellation = true;
shaderConfig.options.mslOptions.disable_rasterization = true;
setEmulatedReversedDepthViewportConfig(shaderConfig, implicit, false);
addVertexInputToShaderConversionConfig(shaderConfig, pCreateInfo);
addNextStageInputToShaderConversionConfig(shaderConfig, tcInputs);

Expand Down Expand Up @@ -1581,6 +1590,7 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
shaderConfig.options.mslOptions.capture_output_to_buffer = true;
shaderConfig.options.mslOptions.multi_patch_workgroup = true;
shaderConfig.options.mslOptions.fixed_subgroup_size = mvkIsAnyFlagEnabled(pTessCtlSS->flags, VK_PIPELINE_SHADER_STAGE_CREATE_ALLOW_VARYING_SUBGROUP_SIZE_BIT) ? 0 : getMetalFeatures().maxSubgroupSize;
setEmulatedReversedDepthViewportConfig(shaderConfig, implicit, false);
addPrevStageOutputToShaderConversionConfig(shaderConfig, vtxOutputs);
addNextStageInputToShaderConversionConfig(shaderConfig, teInputs);

Expand Down Expand Up @@ -1617,6 +1627,7 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
shaderConfig.options.mslOptions.capture_output_to_buffer = false;
shaderConfig.options.mslOptions.raw_buffer_tese_input = true;
shaderConfig.options.mslOptions.disable_rasterization = !_isRasterizing;
setEmulatedReversedDepthViewportConfig(shaderConfig, implicit, getPhysicalDevice()->shouldEmulateReversedDepthViewport());
addPrevStageOutputToShaderConversionConfig(shaderConfig, tcOutputs);

MVKMTLFunction func = getMTLFunction(shaderConfig, pTessEvalSS, pTessEvalFB, _tessEvalModule, "Tessellation evaluation");
Expand All @@ -1627,6 +1638,7 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
auto& funcRslts = func.shaderConversionResults;
plDesc.rasterizationEnabled = !funcRslts.isRasterizationDisabled;
populateResourceUsage(_stageResources[kMVKShaderStageTessEval], shaderConfig, funcRslts, spv::ExecutionModelTessellationEvaluation);
_stageResources[kMVKShaderStageTessEval].implicitBuffers.needed.set(MVKImplicitBuffer::EmulatedReversedDepthViewport, shaderConfig.options.shouldEmulateReversedDepthViewportDynamically);
_layout->populateBindOperations(_stageResources[kMVKShaderStageTessEval].bindScript, shaderConfig, spv::ExecutionModelTessellationEvaluation);

if (funcRslts.isRasterizationDisabled) {
Expand All @@ -1651,11 +1663,18 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
shaderConfig.options.entryPointName = pFragmentSS->pName;
shaderConfig.options.mslOptions.capture_output_to_buffer = false;
shaderConfig.options.mslOptions.fixed_subgroup_size = mvkIsAnyFlagEnabled(pFragmentSS->flags, VK_PIPELINE_SHADER_STAGE_CREATE_ALLOW_VARYING_SUBGROUP_SIZE_BIT) ? 0 : mtlFeats.maxSubgroupSize;
bool shouldEmulateReversedDepthViewports = getPhysicalDevice()->shouldEmulateReversedDepthViewport();
setEmulatedReversedDepthViewportConfig(shaderConfig, implicit, false);
shaderConfig.options.mslOptions.check_discarded_frag_stores = true;
// On NVIDIA, simd_is_helper_thread() can trigger a Metal compiler internal error.
if (getPhysicalDevice()->isMacGPUFamily1()) {
shaderConfig.options.mslOptions.check_discarded_frag_stores = false;
}
if (shouldEmulateReversedDepthViewports) {
// The discard-store helper path uses simd_is_helper_thread(), which is unreliable on the
// same AMD Metal path that needs reversed-depth viewport emulation.
shaderConfig.options.mslOptions.check_discarded_frag_stores = false;
}
/* Enabling makes dEQP-VK.fragment_shader_interlock.basic.discard.image.pixel_ordered.1xaa.no_sample_shading.1024x1024 and similar tests fail. Requires investigation */
shaderConfig.options.mslOptions.force_fragment_with_side_effects_execution = false;
shaderConfig.options.mslOptions.input_attachment_is_ds_attachment = _inputAttachmentIsDSAttachment;
Expand Down Expand Up @@ -2067,6 +2086,7 @@ static void addCommonImplicitBuffersToShaderConfig(SPIRVToMSLConversionConfigura
_stageResources[stage].implicitBuffers.ids[MVKImplicitBuffer::BufferSize] = getImplicitBufferIndex(stage, 1);
_stageResources[stage].implicitBuffers.ids[MVKImplicitBuffer::Swizzle] = getImplicitBufferIndex(stage, 2);
_stageResources[stage].implicitBuffers.ids[MVKImplicitBuffer::Output] = getImplicitBufferIndex(stage, 4);
_stageResources[stage].implicitBuffers.ids[MVKImplicitBuffer::EmulatedReversedDepthViewport] = getImplicitBufferIndex(stage, 7);
uint32_t extra = getImplicitBufferIndex(stage, 3);
switch (stage) {
case kMVKShaderStageVertex:
Expand Down Expand Up @@ -2978,7 +2998,9 @@ void serialize(Archive & archive, SPIRVToMSLConversionOptions& opt) {
opt.tessPatchKind,
opt.numTessControlPoints,
opt.shouldFlipVertexY,
opt.shouldFixupClipSpace);
opt.shouldFixupClipSpace,
opt.shouldEmulateReversedDepthViewportDynamically,
opt.emulatedReversedDepthViewportBufferIndex);
}

template<class Archive>
Expand Down Expand Up @@ -3181,11 +3203,11 @@ void mvkValidateCeralArchiveDefinitions() {
missingBytes += mvkValidateCerealArchiveSize<SPIRV_CROSS_NAMESPACE::MSLConstexprSampler>();
missingBytes += mvkValidateCerealArchiveSize<mvk::SPIRVWorkgroupSizeDimension>(3);
missingBytes += mvkValidateCerealArchiveSize<mvk::SPIRVEntryPoint>(20); // Contains string
missingBytes += mvkValidateCerealArchiveSize<mvk::SPIRVToMSLConversionOptions>(26); // Contains string
missingBytes += mvkValidateCerealArchiveSize<mvk::SPIRVToMSLConversionOptions>(29); // Contains string
missingBytes += mvkValidateCerealArchiveSize<mvk::MSLShaderInterfaceVariable>(3);
missingBytes += mvkValidateCerealArchiveSize<mvk::MSLResourceBinding>(2);
missingBytes += mvkValidateCerealArchiveSize<mvk::DescriptorBinding>();
missingBytes += mvkValidateCerealArchiveSize<mvk::SPIRVToMSLConversionConfiguration>(106); // Contains collection
missingBytes += mvkValidateCerealArchiveSize<mvk::SPIRVToMSLConversionConfiguration>(109); // Contains collection
missingBytes += mvkValidateCerealArchiveSize<mvk::SPIRVToMSLConversionResultInfo>(40); // Contains collection
missingBytes += mvkValidateCerealArchiveSize<mvk::MSLSpecializationMacroInfo>(22); // Contains string
missingBytes += mvkValidateCerealArchiveSize<MVKShaderModuleKey>();
Expand Down
2 changes: 2 additions & 0 deletions MoltenVK/MoltenVK/Utility/MVKStateTracking.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ enum class MVKNonVolatileImplicitBuffer : uint32_t {
BufferSize,
DynamicOffset,
ViewRange,
EmulatedReversedDepthViewport,
Count
};

Expand All @@ -181,6 +182,7 @@ enum class MVKImplicitBuffer : uint32_t {
BufferSize = static_cast<uint32_t>(MVKNonVolatileImplicitBuffer::BufferSize),
DynamicOffset = static_cast<uint32_t>(MVKNonVolatileImplicitBuffer::DynamicOffset),
ViewRange = static_cast<uint32_t>(MVKNonVolatileImplicitBuffer::ViewRange),
EmulatedReversedDepthViewport = static_cast<uint32_t>(MVKNonVolatileImplicitBuffer::EmulatedReversedDepthViewport),

// Volatile implicit buffers
// These buffers are updated per draw call, and are therefore always considered dirty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

#include "SPIRVToMSLConverter.h"
#include "SPIRVToMSLEmulatedReversedDepthViewport.h"
#include "MVKCommonEnvironment.h"
#include "MVKStrings.h"
#include "FileSupport.h"
Expand Down Expand Up @@ -54,6 +55,8 @@ MVK_PUBLIC_SYMBOL bool SPIRVToMSLConversionOptions::matches(const SPIRVToMSLConv
if (numTessControlPoints != other.numTessControlPoints) { return false; }
if (shouldFlipVertexY != other.shouldFlipVertexY) { return false; }
if (shouldFixupClipSpace != other.shouldFixupClipSpace) { return false; }
if (shouldEmulateReversedDepthViewportDynamically != other.shouldEmulateReversedDepthViewportDynamically) { return false; }
if (emulatedReversedDepthViewportBufferIndex != other.emulatedReversedDepthViewportBufferIndex) { return false; }
return true;
}

Expand Down Expand Up @@ -338,6 +341,13 @@ MVK_PUBLIC_SYMBOL bool SPIRVToMSLConverter::convert(SPIRVToMSLConversionConfigur
}
}
conversionResult.msl = pMSLCompiler->compile();
if (shaderConfig.options.shouldEmulateReversedDepthViewportDynamically &&
(shaderConfig.options.entryPointStage == ExecutionModelVertex ||
shaderConfig.options.entryPointStage == ExecutionModelTessellationEvaluation)) {
if (!mvkPatchMSLEmulatedReversedDepthViewport(conversionResult.msl, shaderConfig.options.emulatedReversedDepthViewportBufferIndex)) {
wasConverted = logError(conversionResult.resultLog, "SPIR-V to MSL conversion error: could not patch MSL for emulated reversed-depth viewport.");
}
}

if (shouldLogMSL) { logSource(conversionResult.resultLog, conversionResult.msl, "MSL", "Converted"); }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ namespace mvk {
uint32_t numTessControlPoints = 0;
bool shouldFlipVertexY = true;
bool shouldFixupClipSpace = false;
// Enables runtime emulation of reversed-depth viewports, using the buffer index for the hidden per-draw flag.
bool shouldEmulateReversedDepthViewportDynamically = false;
uint32_t emulatedReversedDepthViewportBufferIndex = 0;

/**
* Returns whether the specified options match this one.
Expand Down
Loading
Loading