Add support for RG11B10 and RGB9E5 vertex formats

[spnda]

Metal 3.1 and iOS 17/macOS 14 add support for these two new vertex/attribute formats.

I also put in all of the necessary fixes (afaik) for Xcode 15 into this PR.

[billhollings]

LGTM. Thanks for taking care of this.

Are you expecting to add more, or do you want to move this from Draft status so I can pull it in?

[spnda]

No the draft status is because I don't think it worked on my machine, I tried looking at VulkanCapsViewer and it didn't look like the VERTEX_BUFFER usage flag was enabled for either of the formats. I was going to ask if I did do everything necessary for new vertex formats? Couldn't find anything else that I would maybe have to enable/change.

[cdavis5e]

I can't see anything major you missed. Just a few minor problems.

Are you sure you built with Xcode 15? Are you sure MVK_XCODE_15 is true in that case? (This is the only other thing I can think of.)

[billhollings]

I can't see anything major you missed. Just a few minor problems.

Are you sure you built with Xcode 15? Are you sure MVK_XCODE_15 is true in that case? (This is the only other thing I can think of.)

Also, are you running VulkanCapsViewer on macOS 14 / Sonoma? Are you brave enough to have upgraded your OS already, too?

[spnda]

I'm definitely building with Xcode 15 and MVK_XCODE_15 is definitely true. No, I am still using macOS 13 but I figured if it would show on my machine it would also show on macOS 14. On macOS 13 it would just not be usable. Now that I think about it, it's obvious why it wasn't showing: It's only being enabled for macOS 14... Not sure if I want to upgrade that Mac though because it's technically not mine.

[billhollings]

Not sure if I want to upgrade that Mac though because it's technically not mine.

Yeah...I usually wait until later in the beta cycle.

The challenges of such early beta is why we usually don't jump directly into trying to implement new capabilities at this point.

However, your change is fairly straightforward. I think if you implement the

#if !MVK_XCODE_15
#define MTLVertexFormatFloatRG11B10 MTLVertexFormatInvalid
#define MTLVertexFormatFloatRGB9E5 MTLVertexFormatInvalid
#endif

changes, and leave the MVK_XCODE_15 guards around addGPUOSMTLVtxFmtCaps(), we should be good.

[billhollings]

What's the current status of this PR?

Although it would be good to get it all resolved, I'd like to get at least the generic Xcode SDK 15 changes in these files pulled in:

MVKCommonEnvironment.h
MVKDevice.mm
OSSupport.mm

and we can add the RG11B10 and RGB9E5 support later if we can't resolve it now.

[spnda]

What's the current status of this PR?

Although it would be good to get it all resolved, I'd like to get at least the generic Xcode SDK 15 changes in these files pulled in:

MVKCommonEnvironment.h
MVKDevice.mm
OSSupport.mm

and we can add the RG11B10 and RGB9E5 support later if we can't resolve it now.

Sorry for getting back this late. Seems like #1966 already addressed all of these concerns? So I'll just rebase this to add the two formats in a single commit.

The changes are fairly simple now.

[spnda]

@billhollings Is this mergeable now? I don't have macOS 14 to test myself, but theoretically this should be fine, right?

[AntarticCoder]

@spnda I'm running MacOS 14, would you like me to try and build it?

[spnda]

@spnda I'm running MacOS 14, would you like me to try and build it?

Building isn't the issue, but yeah if you want you can try and check if the formats are properly marked as available.

[billhollings]

LGTM. Just waiting for @cdavis5e to clear his previous review..

[AntarticCoder]

@spnda So I compiled your PR on my machine, and I temporally replaced libMoltenVK in my /usr/local/lib with your PR build. I then took my hobby renderer and just changed the vertex format, and changed it to VK_FORMAT_E5B9G9R9_UFLOAT_PACK32, however this failed because I got an error, which is as follows.

[mvk-error] VK_ERROR_FORMAT_NOT_SUPPORTED: VkFormat VK_FORMAT_E5B9G9R9_UFLOAT_PACK32 is not supported for vertex buffers on this device.

Note: I also tried this for RG11B10 and got the same result.

I know that I'm using your PR build because I recieve all of the config messages, and before changing vertex format, my graphics were displaying perfectly the same as the Vulkan SDK build.

[billhollings]

So I compiled your PR on my machine

Just checking...definitely using Xcode 15?

[AntarticCoder]

@billhollings Definitely

[cdavis5e]

Are you using Apple Silicon? The new formats only appear to be enabled for Apple Silicon GPUs.

[AntarticCoder]

Yes, I'm using an M1 MacBook Air, I'll try and see if I did anything wrong for linking or building tomorrow.

[cdavis5e]

Clearly, we've all missed something.

What does vkGetPhysicalDeviceFormatProperties() return for B10G11R11_UFLOAT_PACK32 and E5B9G9R9_UFLOAT_PACK32? Is the VERTEX_BUFFER_BIT set in bufferFeatures?

[AntarticCoder]

@cdavis5e I ran the function that you provided and got:

VK_FORMAT_B10G11R11_UFLOAT_PACK32 has been found! VK_FORMAT_E5B9G9R9_UFLOAT_PACK32 has been found!

This is the code I'm running if you want to know:

    VkFormatProperties props1 = {};
    vkGetPhysicalDeviceFormatProperties(physicalDevice, VK_FORMAT_B10G11R11_UFLOAT_PACK32, &props1);

    if(props1.bufferFeatures & VK_FORMAT_FEATURE_VERTEX_BUFFER_BIT)
    {
        std::cout << "VK_FORMAT_B10G11R11_UFLOAT_PACK32 has been found!" << std::endl;
    }else 
    {
        std::cout << "VK_FORMAT_B10G11R11_UFLOAT_PACK32 has not been found!" << std::endl;
    }

    VkFormatProperties props2 = {};
    vkGetPhysicalDeviceFormatProperties(physicalDevice, VK_FORMAT_E5B9G9R9_UFLOAT_PACK32, &props2);

    if(props2.bufferFeatures & VK_FORMAT_FEATURE_VERTEX_BUFFER_BIT)
    {
        std::cout << "VK_FORMAT_E5B9G9R9_UFLOAT_PACK32 has been found!" << std::endl;
    }else 
    {
        std::cout << "VK_FORMAT_E5B9G9R9_UFLOAT_PACK32 has not been found!" << std::endl;
    }

Tell me if I'm doing anything wrong. Thanks

[spnda]

That result would mean that it does work. Your hobby project must be doing something differently/wrong for it to think it's not supported. Or am I misremembering that a vertex buffer just needs to have a format with VERTEX_BUFFER_BIT set to be used validly?

[AntarticCoder]

@spnda Here's my vertex code:

class Vertex
{
public:
    glm::vec2 position;
    glm::vec3 color;

    static VkVertexInputBindingDescription GetBindingDescription()
    {
        VkVertexInputBindingDescription bindingDescription{};
        bindingDescription.binding = 0;
        bindingDescription.stride = sizeof(Vertex);
        bindingDescription.inputRate = VK_VERTEX_INPUT_RATE_VERTEX;
        return bindingDescription;
    }

    static std::array<VkVertexInputAttributeDescription, 2> GetAttributeDescriptions()
    {
        std::array<VkVertexInputAttributeDescription, 2> attributeDescriptions{};

        attributeDescriptions[0].binding = 0;
        attributeDescriptions[0].location = 0;
        attributeDescriptions[0].format = VK_FORMAT_R32G32_SFLOAT;
        attributeDescriptions[0].offset = offsetof(Vertex, position);

        attributeDescriptions[1].binding = 0;
        attributeDescriptions[1].location = 1;
        // attributeDescriptions[1].format = VK_FORMAT_R32G32B32_SFLOAT;
        attributeDescriptions[1].format = VK_FORMAT_E5B9G9R9_UFLOAT_PACK32;
        attributeDescriptions[1].offset = offsetof(Vertex, color);

        return attributeDescriptions;
    }
};

This is all based on Vulkan Tutorial. Whenever I switch to the new vulkan formats, it gives me this error.

VK_ERROR_FORMAT_NOT_SUPPORTED: VkFormat VK_FORMAT_E5B9G9R9_UFLOAT_PACK32 is not supported for vertex buffers on this device.

Perhaps you forgot to add something somewhere in the code, cause it says it's supported when asked, but when using the new formats, just breaks.

I'm not formatting my vertices for these new formats, but they should cause validation errors, not MoltenVK errors.

[cdavis5e]

That error message comes from MVKPixelFormats::getMTLVertexFormat(). It gets printed when the format description returned by getVkFormatDesc() returns one with MTLVertexFormatInvalid. This would suggest that the vertex format isn't getting set for some reason in the _vkFormatDescriptions entry for the two formats.

[cdavis5e]

This would suggest that the vertex format isn't getting set for some reason in the _vkFormatDescriptions entry for the two formats.

Or is getting set back to Invalid, which happens when MVKMTLFormatDesc::isSupported() returns false.

...But wait, that method is for the MTLPixelFormat! For the vertex format, it's supposed to call MVKMTLFormatDesc::vertexIsSupported()! There's the problem! EDIT: Oh wait, that's on the MVKVkFormatDesc. So that's not it...

[cdavis5e]

So MVKMTLFormatDesc::isSupported() returns false if either of the following is true:

• the format is MTLPixelFormatInvalid/MTLVertexFormatInvalid (which happen to be both zero); or
• the format capabilities are kMVKMTLFmtCapsNone.

Since we know the format itself is not MTLVertexFormatInvalid, it must be because the caps aren't getting set.

[spnda]

I've now got my own Mac finally, so I decided to try out this PR again. After a lot of installing & configuring I finally got everything to work.

And for me B10R11G11_UFLOAT_PACK32 definitely has the VERTEX_BUFFER_BIT set. I am only using a rebased version of this PR, so nothing has really changed as far as I know. Same thing for R9G9B9E5_UFLOAT_PACK32. I'm on macOS 14.2.1 with a fresh build of MVK and VulkanCapsViewer. I'll push again to get CI to run on Xcode 11.7 and fix the issues there.

[spnda]

I see that the Xcode 11 CI was removed, and now everything builds correctly. @AntarticCoder could you perhaps re-test to see if it wasn't anything else causing the issue on your end? Otherwise, @billhollings, care to review & merge?

[billhollings]

This is still causing problems. The CTS vertex format tests for these formats fail.

It turns out the problem all along has been that _mtlVertexFormatCount is set too low. Please update the following line at the top of MVKPixelFormats.h as follows:

static const uint32_t _mtlVertexFormatCount = MTLVertexFormatFloatRGB9E5 + 1;

This is not your mistake. There is an assertion that should be triggered when this count is too low, but the assertion test itself is broken.

It also looks like the vertex format lookups in MVKPixelFormats could be simplified. Rather than deal with all that here, once you've made this change (plus the change I requested inline), and this PR is pulled in, I'll submit a PR to correct the vertex format lookup logic.

[billhollings]

Shoot! It's now (understandably) failing Xcode before 15.

We'll have to do what we did with _mtlPixelFormatCoreCount:

static const uint32_t _mtlVertexFormatCount = MTLVertexFormatHalf + 3;     // The actual last enum value is not available prior to Xcode 15.

[billhollings]

LGTM now. Thanks!