Disable unsupported Metal Pixel formats for iOS/tvOS Simulator

[SRSaunders]

Defines certain unsupported Metal Pixel formats as invalid for the iOS/tvOS Simulator. Fixes #2353 for MoltenVK 1.2.11.

I have tested this on an x86_64 host using Xcode 15.2 with the iOS Simulator (running iOS 17.2). I can't test on tvOS so I would appreciate others (@warmenhoven) checking out this scenario.

Also, I am not sure whether the #ifdef condition should also contain an Xcode version test. Perhaps @cdavis5e could weigh in on that aspect. Thanks.

[cdavis5e]

A version check is only necessary for when earlier versions don't have the pixel format enumerator in the SDK but later versions do. That doesn't apply here, since all of those pixel formats have been supported since iOS 8. AFAIK, those formats have never been supported under the simulator, even on Apple Silicon for some reason.

[billhollings]

Thanks for submitting this! And thanks everyone for all the discussion on #2353!

First of all, I'm sorry that my 5afeaa4 triggered the need for this!

Based upon the "I recall reading somewhere" comment by @cdavis5e in #2353, I went hunting, and sure enough, hidden away in the Apple docs, is the explanation.

Unfortunately, that doc also make this somewhat more complicated.

These 5 formats you identified MTLPixelFormats, plus all of the XR10 and YUV formats, are not supported on the simulator platforms.

So, @SRSaunders, can you add the following to this PR, please and thanks?

1. Add all the XR10 and YUV formats to your conditional format remapping.
2. Add && !MVK_OS_SIMULATOR to the #if MVK_APPLE_SILICON for the XR10 formats in MVKPhysicalDevice::getSurfaceFormats().
3. According to the same doc, the format MTLPixelFormatRGB9E5Float cannot be used as a render target on the simulator. I think the best way to deal with this is in MVKPixelFormats::modifyMTLFormatCapabilities(). Perhaps add the following, right below the other two mods there for RGB9E5Float:

    disableMTLPixFmtCapsIf( MVK_OS_SIMULATOR, RGB9E5Float, ColorAtt );

5. And finally, none of the _sRGB formats can be written to in the simulator. I think the best way to handle this might be in #define addMTLPixelFormatDescSRGB(), by adding something like:

    if(MVK_APPLE_SILICON) { mvkDisableFlags(appleGPUCaps, kMVKMTLFmtCapsWrite); }

Uggh! Needless to say, I'm not a fan of the simulator! 😉

[SRSaunders]

Looking at the Apple docs, they do not mention MTLPixelFormatRG8Unorm_sRGB, but disabling this on the Simulator seems necessary. Did this format come later? Note I also added MTLPixelFormatBGR5A1Unorm (from the docs) for completeness, although this is already disabled elsewhere via addVkFormatDesc( B5G5R5A1_UNORM_PACK16, Invalid, ...).

These changes run correctly for my limited test cases (Vulkan-Samples project running on iOS Simulator on x86_64 host, macOS Ventura, Xcode 15.2). However, I cannot test all of the new changes here. This PR could benefit from additional code review at a minimum.

Question: Given release timing, does this mean the Simulator will be broken for MVK 1.2.11, Vulkan SDK 1.3.296? If so that's an unfortunate regression, and at minimum should be highlighted in the release notes.

Lastly, I have another Simulator issue outstanding - see #2285. When running the Vulkan-Samples on the Simulator, starting with MoltenVK 1.2.11, certain samples do not render (black screen) unless I disable Metal Argument Buffers. What is the status of Argument Buffers on the Simulator? Should they be disabled by default on the Simulator, or is there another defect lingering somewhere else that causes this behaviour?

[billhollings]

Thanks. LGTM now.

Bummer about all the changes required to the define macros, but that's the nature of macros.

I'll just wait for @cdavis5e to have a look before pulling this in.

[billhollings]

Lastly, I have another Simulator issue outstanding - see #2285. When running the Vulkan-Samples on the Simulator, starting with MoltenVK 1.2.11, certain samples do not render (black screen) unless I disable Metal Argument Buffers. What is the status of Argument Buffers on the Simulator? Should they be disabled by default on the Simulator, or is there another defect lingering somewhere else that causes this behaviour?

Now that the SDK release is done, I'll have another closer look at #2285.

I really, REALLY don't want to have to disable arg buffers for any platform, so that will be an absolute last resort. Moving forward, arg buffers are important for a lot of Vulkan functionality, starting with the availability of larger numbers of bindless descriptors.

[SRSaunders]

Bummer about all the changes required to the define macros, but that's the nature of macros.

Agreed the macro changes are kind of ugly. If you see a better solution please feel free to modify.

I really, REALLY don't want to have to disable arg buffers for any platform, so that will be an absolute last resort.

That's fine. The issue identified some Vulkan-Samples problems and a potential solution in the form of Arg Buffer disabling on the Simulator. But first I wanted to determine your overall intentions re Arg Buffers and the Simulator. I have my answer now, and will look at #2285 as a possible defect. Thanks.

[SRSaunders]

A small change to move unsupported pixel formats to top level to ensure visionOS Simulator is covered too, not just iOS and tvOS Simulators. Docs indicate these pixel format restrictions apply to all Simulators.

[SRSaunders]

Not sure if you care, but looking around in the code where MVK_OS_SIMULATOR is used I see two potential issues that limit scope to the iOS Simulator only, and exclude tvOS / visionOS Simulators. Should these be brought to the top level, or alternatively, similar code added for tvOS and visionOS?

In MVKDevice.mm:

#if MVK_IOS
...
#if MVK_OS_SIMULATOR
		_metalFeatures.nativeTextureSwizzle = false;
#else
		_metalFeatures.nativeTextureSwizzle = true;
#endif
...
#endif

and

#if MVK_IOS
...
#if MVK_OS_SIMULATOR
	_features.depthClamp = false;
#else
	if (supportsMTLGPUFamily(Apple2)) {
		_features.depthClamp = true;
	}
#endif
...
#endif

[billhollings]

Should these be brought to the top level, or alternatively, similar code added for tvOS and visionOS?

Yes. Makes sense to deal with them here. Thanks for taking a wider inventory.

I assume you're thinking of setting the non-simulator value (eg. _metalFeatures.nativeTextureSwizzle = true;) in the platform section, and then, below all the platform sections, adding a separate MVK_OS_SIMULATOR section that overrides those platform values accordingly (eg. _metalFeatures.nativeTextureSwizzle = false;)?

[SRSaunders]

I assume you're thinking of setting the non-simulator value (eg. _metalFeatures.nativeTextureSwizzle = true;) in the platform section, and then, below all the platform sections, adding a separate MVK_OS_SIMULATOR section that overrides those platform values accordingly (eg. _metalFeatures.nativeTextureSwizzle = false;)?

Yes, exactly. In fact there is already a MVK_OS_SIMULATOR override for _metalFeatures that I reused for swizzle. And I simply added an additional MVK_OS_SIMULATOR override for _features. Note that I did NOT change or add any *feature = true assignments, but simply moved the Simulator *feature = false changes to the overrides.

However, I do notice two things: a) _features.depthClamp = true is not set for tvOS, and b) the feature settings for visionOS are quite sparse compared to iOS and tvOS. Are both of these intentional?

[billhollings]

However, I do notice two things: a) _features.depthClamp = true is not set for tvOS, and b) the feature settings for visionOS are quite sparse compared to iOS and tvOS. Are both of these intentional?

Good catch on _features.depthClamp = true. This is almost certainly an oversight, and should be set as it is for iOS.

visionOS is only partially supported on MoltenVK. It really requires someone with an Apple Vision device to overhaul it. For now, you can leave it until that gets done in a separate PR in the future.

[billhollings]

Good catch on _features.depthClamp = true. This is almost certainly an oversight, and should be set as it is for iOS.

@SRSaunders

Are you able to make this change on this PR, or should we leave it for another PR?

I'm just waiting on this before pulling this PR in.

[SRSaunders]

Are you able to make this change on this PR, or should we leave it for another PR?

Sorry for the delay, but I have been trying to test Vulkan-Samples on tvOS Simulator to see if we have things right.

I have now pushed this change, and also added one more difference that I see for AppleGPUFamily cases that are <= Apple3: _features.shaderTessellationAndGeometryPointSize = true. If I have this wrong, please revert.

The Vulkan-Samples tests run mostly ok on tvOS Simulator, with the exception of samples hpp_oit_linked_lists and oit_linked_lists. On those I get the following:

validateWithDevice:4343: failed assertion `Render Pipeline Descriptor Validation
No valid pixelFormats set.

This happens in:

id<MTLRenderPipelineState> MVKRenderPipelineCompiler::newMTLRenderPipelineState(MTLRenderPipelineDescriptor* mtlRPLDesc) {
	unique_lock<mutex> lock(_completionLock);

	compile(lock, ^{
		auto mtlDev = getMTLDevice();
		@synchronized (mtlDev) {
			[mtlDev newRenderPipelineStateWithDescriptor: mtlRPLDesc
									   completionHandler: ^(id<MTLRenderPipelineState> ps, NSError* error) {
										   bool isLate = compileComplete(ps, error);
										   if (isLate) { destroy(); }
									   }];
		}
	});

	return [_mtlRenderPipelineState retain];
}

Unfortunately I don't have time to research now as I will be away for a bit. Otherwise I would try to track this down. I think this is the relevant SPIRV and MSL:

[mvk-info] Compiling Metal shader with FastMath enabled.
[mvk-info] Converting SPIR-V:
; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 11
; Bound: 67
; Schema: 0
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %39 %56
OpExecutionMode %4 OriginUpperLeft
OpSource GLSL 450
OpName %4 "main"
OpName %8 "nextFragmentIndex"
OpName %9 "FragmentCounter"
OpMemberName %9 0 "value"
OpName %11 "fragmentCounter"
OpName %23 "SceneConstants"
OpMemberName %23 0 "projection"
OpMemberName %23 1 "view"
OpMemberName %23 2 "background_grayscale"
OpMemberName %23 3 "sortFragments"
OpMemberName %23 4 "fragmentMaxCount"
OpMemberName %23 5 "sortedFragmentCount"
OpName %25 "sceneConstants"
OpName %34 "previousFragmentIndex"
OpName %37 "linkedListHeadTex"
OpName %39 "gl_FragCoord"
OpName %51 "FragmentBuffer"
OpMemberName %51 0 "data"
OpName %53 "fragmentBuffer"
OpName %56 "inColor"
OpMemberDecorate %9 0 Offset 0
OpDecorate %9 BufferBlock
OpDecorate %11 DescriptorSet 0
OpDecorate %11 Binding 4
OpMemberDecorate %23 0 ColMajor
OpMemberDecorate %23 0 Offset 0
OpMemberDecorate %23 0 MatrixStride 16
OpMemberDecorate %23 1 ColMajor
OpMemberDecorate %23 1 Offset 64
OpMemberDecorate %23 1 MatrixStride 16
OpMemberDecorate %23 2 Offset 128
OpMemberDecorate %23 3 Offset 132
OpMemberDecorate %23 4 Offset 136
OpMemberDecorate %23 5 Offset 140
OpDecorate %23 Block
OpDecorate %25 DescriptorSet 0
OpDecorate %25 Binding 0
OpDecorate %37 DescriptorSet 0
OpDecorate %37 Binding 2
OpDecorate %39 BuiltIn FragCoord
OpDecorate %50 ArrayStride 16
OpMemberDecorate %51 0 Offset 0
OpDecorate %51 BufferBlock
OpDecorate %53 DescriptorSet 0
OpDecorate %53 Binding 3
OpDecorate %56 Location 0
%2 = OpTypeVoid
%3 = OpTypeFunction %2
%6 = OpTypeInt 32 0
%7 = OpTypePointer Function %6
%9 = OpTypeStruct %6
%10 = OpTypePointer Uniform %9
%11 = OpVariable %10 Uniform
%12 = OpTypeInt 32 1
%13 = OpConstant %12 0
%14 = OpTypePointer Uniform %6
%16 = OpConstant %6 1
%17 = OpConstant %6 0
%20 = OpTypeFloat 32
%21 = OpTypeVector %20 4
%22 = OpTypeMatrix %21 4
%23 = OpTypeStruct %22 %22 %20 %6 %6 %6
%24 = OpTypePointer Uniform %23
%25 = OpVariable %24 Uniform
%26 = OpConstant %12 4
%29 = OpTypeBool
%35 = OpTypeImage %6 2D 0 0 0 2 R32ui
%36 = OpTypePointer UniformConstant %35
%37 = OpVariable %36 UniformConstant
%38 = OpTypePointer Input %21
%39 = OpVariable %38 Input
%40 = OpTypeVector %20 2
%43 = OpTypeVector %12 2
%46 = OpTypePointer Image %6
%49 = OpTypeVector %6 3
%50 = OpTypeRuntimeArray %49
%51 = OpTypeStruct %50
%52 = OpTypePointer Uniform %51
%53 = OpVariable %52 Uniform
%56 = OpVariable %38 Input
%59 = OpConstant %6 2
%60 = OpTypePointer Input %20
%65 = OpTypePointer Uniform %49
%4 = OpFunction %2 None %3
%5 = OpLabel
%8 = OpVariable %7 Function
%34 = OpVariable %7 Function
%15 = OpAccessChain %14 %11 %13
%18 = OpAtomicIAdd %6 %15 %16 %17 %16
OpStore %8 %18
%19 = OpLoad %6 %8
%27 = OpAccessChain %14 %25 %26
%28 = OpLoad %6 %27
%30 = OpUGreaterThanEqual %29 %19 %28
OpSelectionMerge %32 None
OpBranchConditional %30 %31 %32
%31 = OpLabel
OpKill
%32 = OpLabel
%41 = OpLoad %21 %39
%42 = OpVectorShuffle %40 %41 %41 0 1
%44 = OpConvertFToS %43 %42
%45 = OpLoad %6 %8
%47 = OpImageTexelPointer %46 %37 %44 %17
%48 = OpAtomicExchange %6 %47 %16 %17 %45
OpStore %34 %48
%54 = OpLoad %6 %8
%55 = OpLoad %6 %34
%57 = OpLoad %21 %56
%58 = OpExtInst %6 %1 PackUnorm4x8 %57
%61 = OpAccessChain %60 %39 %59
%62 = OpLoad %20 %61
%63 = OpBitcast %6 %62
%64 = OpCompositeConstruct %49 %55 %58 %63
%66 = OpAccessChain %65 %53 %13 %54
OpStore %66 %64
OpReturn
OpFunctionEnd

End SPIR-V

Converted MSL:
#pragma clang diagnostic ignored "-Wunused-variable"

#include <metal_stdlib>
#include <simd/simd.h>
#include <metal_atomic>

using namespace metal;

struct FragmentCounter
{
uint value;
};

struct SceneConstants
{
float4x4 projection;
float4x4 view;
float background_grayscale;
uint sortFragments;
uint fragmentMaxCount;
uint sortedFragmentCount;
};

struct FragmentBuffer
{
uint3 data[1];
};

struct main0_in
{
float4 inColor [[user(locn0)]];
};

fragment void main0(main0_in in [[stage_in]], constant SceneConstants& sceneConstants [[buffer(8)]], device FragmentBuffer& fragmentBuffer [[buffer(10)]], device FragmentCounter& fragmentCounter [[buffer(11)]], texture2d<uint, access::read_write> linkedListHeadTex [[texture(0)]], float4 gl_FragCoord [[position]])
{
bool gl_HelperInvocation = {};
gl_HelperInvocation = simd_is_helper_thread();
uint _18 = (!gl_HelperInvocation ? atomic_fetch_add_explicit((device atomic_uint*)&fragmentCounter.value, 1u, memory_order_relaxed) : uint{});
uint nextFragmentIndex = _18;
if (nextFragmentIndex >= sceneConstants.fragmentMaxCount)
{
gl_HelperInvocation = true, discard_fragment();
}
uint _48 = (!gl_HelperInvocation ? linkedListHeadTex.atomic_exchange(uint2(int2(gl_FragCoord.xy)), nextFragmentIndex).x : uint{});
uint previousFragmentIndex = _48;
if (!gl_HelperInvocation)
{
fragmentBuffer.data[nextFragmentIndex] = uint3(previousFragmentIndex, pack_float_to_unorm4x8(in.inColor), as_type(gl_FragCoord.z));
}
}

End MSL

[billhollings]

Okay. Thanks. I'm going to pull this in now to get the basics working, and we can explore the the simulator issues you mentioned separately.