WIP: Vulkan sampling support

[Valakor]

NOTE: This pull request includes a handful of fixes that aren't strictly related to Vulkan sampling which I'll list below. They could be pulled out into a separate change if required.

1. Fix Remotery_Destructor: Move clearing of g_Remotery to after graphics API-specific deletion/unbind calls, avoiding crashes because of unchecked access to g_Remotery.
2. Fix RMT_ARCH_64BIT: Check __arm64__ as well.
3. Fix rmt_GetLastErrorMessage declaration: Enclose inside extern "C" block for C++ users.
4. Fix rmt_ScopedD3D12Sample: Remove extra parens in end-of-scope object declaration which confuse some compilers that think it's a function declaration (most vexing parse).
5. Fix Remotery.js deserialization of Uint64 and Sint64 properties: Remotery.c already converts these to rmtF64 prior to sending them over the socket.
6. Add support for thread names on MacOS via pthread_setname_np and pthread_getname_np.

This is a first-pass implementation of Vulkan sampling support for Remotery based on the D3D12 implementation. It currently expects a handful of things about the user's Vulkan environment that are likely more restrictive than necessary but make the implementation much simpler.

• The user must either use Vulkan 1.2 with the hostQueryReset and timelineSemaphore features enabled, or have enabled the extensions VK_EXT_host_query_reset and VK_KHR_timeline_semaphore. These features/extensions are pretty widely supported so it seems maybe reasonable to require that users enable them; without them the implementation gets reasonably more complex and less performant.
• The user must enable the extension VK_EXT_calibrated_timestamps. It'd probably be pretty easy to make usage of this extension optional.
• The implementation always loads Vulkan functions dynamically. This is generally the most-preferred method, but it might be nicer to instead just have the user provide all the functions as a configuration option up-front. Unfortunately that's difficult without a bunch of horrible forward declarations, or including Vulkan.h in Remotery.h.
• The way I'm passing vkGetInstanceProcAddr feels very hacky, definitely open to suggestions here.
• I've left a handful of TODO(valakor)'s in the code for potential future improvement points.
• I've tried to do everything to support Windows+MacOS+Linux but it's possible I missed some things on non-Windows platforms. Mac is especially tricky since the normal Vulkan API doesn't understand that Metal doesn't support writing timestamps inside render/compute passes, but this is true for any user of Vulkan via MoltenVK.

I'm open to discussion/feedback about this implementation!

[dwilliamson]

Matthew, this looks amazing. I've just ended my day so will go through all this first thing tomorrow morning. Thank you!

[dwilliamson]

This all looks great. Not really much to critique, though I did go through each commit and try to use my very limited Vulkan knowledge to ensure all was OK.

[dwilliamson]

What's left to pull this out of Draft?

[Valakor]

What's left to pull this out of Draft?

I think it might be worth discussing the API of rmt_BindVulkan and whether the user should provide function pointers to Remotery or whether Remotery should continue to look them up dynamically. I'm not 100% sure if the current implementation will work in cases where the user links to Vulkan statically; having the user provide the function pointers would allow Remotery to support a wider range of Vulkan implementations / environments. Unfortunately, doing this would probably require including Vulkan.h in the Remotery header to avoid super gross forward declarations which I wasn't sure was acceptable - it's a pretty big include and Remotery.h is currently fairly lightweight.

Edit: Somehow this whole time I missed that Remotery already has an answer to this problem for CUDA - declaring all the required function pointers as void* passed via a struct. It would be fairly simple for me to change the Vulkan code to something similar. I could also model the Vulkan error handling similarly to CUDA if requested by mapping Vulkan error codes to new rmtError codes.

It's probably also worth discussing whether it's worth relaxing the current Vulkan version / extension requirements. If you're fine with the current limitations then it could probably be merged in its current form and future changes could be taken to support older Vulkan implementations.

[dwilliamson]

I think it might be worth discussing the API of rmt_BindVulkan and whether the user should provide function pointers to Remotery or whether Remotery should continue to look them up dynamically. I'm not 100% sure if the current implementation will work in cases where the user links to Vulkan statically; having the user provide the function pointers would allow Remotery to support a wider range of Vulkan implementations / environments. Unfortunately, doing this would probably require including Vulkan.h in the Remotery header to avoid super gross forward declarations which I wasn't sure was acceptable - it's a pretty big include and Remotery.h is currently fairly lightweight.

Edit: Somehow this whole time I missed that Remotery already has an answer to this problem for CUDA - declaring all the required function pointers as void* passed via a struct. It would be fairly simple for me to change the Vulkan code to something similar. I could also model the Vulkan error handling similarly to CUDA if requested by mapping Vulkan error codes to new rmtError codes.

By static linking, I presume you mean platforms like iOS? This issue here seems to imply you can link statically and use vkGetInstanceProcAddr as well: KhronosGroup/MoltenVK#1809

Also, do you have to pass the vkGetInstanceProcAddr function to rmt_BindVulkan? We manage to avoid this with OpenGL and wglGetProcAddress, but only really because each platform has its own OpenGL function getter.

It's probably also worth discussing whether it's worth relaxing the current Vulkan version / extension requirements. If you're fine with the current limitations then it could probably be merged in its current form and future changes could be taken to support older Vulkan implementations.

I think it's not worth worrying about. If anybody needs extra functionality and you personally do not need it right now, it's worth waiting until somebody adds a PR for it.

[Valakor]

By static linking, I presume you mean platforms like iOS? This issue here seems to imply you can link statically and use vkGetInstanceProcAddr as well: KhronosGroup/MoltenVK#1809

Yeah there are a handful of edge cases related to statically linking Vulkan that I haven't tested, though my understanding is that dynamic linking is almost always preferable (or even required) on most platforms.

Also, do you have to pass the vkGetInstanceProcAddr function to rmt_BindVulkan? We manage to avoid this with OpenGL and wglGetProcAddress, but only really because each platform has its own OpenGL function getter.

In theory no, but I was trying to avoid having to re-implement something like the dynamic loader in Volk (see volkInitialize). Further, I think it's possible the user may have a "special" loader function that is not the one retrieved directly from the dynamic library, but I don't know how common that is in practice. I think it's overall a lot easier and less error prone for Remotery to be passed all the function pointers, but it does make initialization more complex for the user.

[dwilliamson]

That's pretty much what we do with OpenGL.

However, in the absence of a wide array of platforms to test this on, currently, I'm more than happy to accept a PR that requires you to pass Vulkan function pointers as part of rmt_BindVulkan.

That said: Volk is MIT licensed, so I would also have no issues if the function pointer loading code was copied over for just the functions we need. Reference would have to be given in the code, of course.

Another Edit: And while I'm here, the code as it stands is perfect for a PR anyway. It catches your use-case well and you have done the heavy lifting of the entire Vulkan port. If it doesn't meet another user's needs, we'll get a PR and anybody can work on updating it.

[Valakor]

I think I'll submit changes to have the user pass the function pointers. I realized when I was doing some final cleanup that many (all?) Vulkan implementations might expose function pointers for extensions that aren't enabled (or not even supported) by the implementation, and there's no way to check for extension enable state.

That might be fine if we just expect the user to have enabled the right extensions ahead of time, but unfortunately there's no way to tell whether they're using an extension or promoted feature, e.g. an EXT extension promoted to KHR or any extension promoted to core functionality in a later Vulkan version.

[dwilliamson]

The new fn registration looks good, we can make smoother API changes later if required, but I doubt we'll need it.

The only build failures are typical Github spurious fails that have nothing to do with the code.

This is some great work; thanks for pushing it through!