iOS Support

[iconiqlabs]

Pull Request Template

Description

Support for iOS.

Fixes # (issue)

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested to run in iOS

Test Configuration:

• Firmware version:
• Hardware:
• Toolchain:
• SDK:

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[robertosfield]

Thanks @iconiqlabs. I have merged and built under Linux. This built but caused a linked error relating to the change to use vkCreateRenderPass2KHR. This also broke the automated Windows build so I've changed the code back to the original vkCreateRenderPass2KHR and added this macro to RenderPass.cpp to try and keep things compiling on the iOS build:

#if !defined(VK_VERSION_1_2)
#define vkCreateRenderPass2 vkCreateRenderPass2KHR
#endif

Could you try VSG master to see if builds cleanly on iOS?

[iconiqlabs]

Unfortunately, this doesn't help. I can't build with this. The VK_VERSION_1_2 macro is defined here too (even though this is wrong as they claim to support Vulkan API 1.1). I've noticed there is
#define MVK_VERSION_MAJOR 1
#define MVK_VERSION_MAJOR 1
#define MVK_VERSION_MAJOR 4

I was thinking to actually define a MVK_VERSION_1_1 via src/CMakeLists.txt and do what you did. Would you agree with this?

[robertosfield]

On Sat, 23 Apr 2022 at 16:32, iconiqlabs ***@***.***> wrote: Unfortunately, this doesn't help. I can't build with this. The VK_VERSION_1_2 macro is defined here too (even though this is wrong as they claim to support Vulkan API 1.1). I've noticed there is #define MVK_VERSION_MAJOR 1 #define MVK_VERSION_MAJOR 1 #define MVK_VERSION_MAJOR 4 I was thinking to actually define a MVK_VERSION_1_1 via src/CMakeLists.txt and do what you did. Would you agree with this?

If VK_VERSION_1_2 is defined then then the header at least should have the vkCreateRenderPass2 function, so I would expect it to compile. Is it a compile error or a link error that you are seeing? Or a runtime error?

…

Message ID: ***@***.***>

[iconiqlabs]

It compiles, yes. But it's a linker error:
Undefined symbols for architecture arm64:
"_vkCreateRenderPass2", referenced from:
vsg::RenderPass:RenderPass(vsg::Device*, std::__1::vector<vsg::AttachmentDescription, ....

I am sure this is a MoltenVK issue. So, I think that instead of (or better said in addition of) looking for Vulkan version, we need to look for a MoltenVK version when it exists and check it is 1.1, 1.2, etc...

[robertosfield]

On Sat, 23 Apr 2022 at 17:24, iconiqlabs ***@***.***> wrote: It compiles, yes. But it's a linker error: Undefined symbols for architecture arm64: "_vkCreateRenderPass2", referenced from: vsg::RenderPass:RenderPass(vsg::Device*, std::__1::vector<vsg::AttachmentDescription, .... I am sure this is a MoltenVK issue. So, I think that instead of (or better said in addition of) looking for Vulkan version, we need to look for a MoltenVK version when it exists and check it is 1.1, 1.2, etc...

Thanks for the explanation that makes it a bit clearer and explains why the version check I added doesn't work. Are you building against VulkanSDK headers but linking to MoltenVK libs? Do the MoltenVK libs come with headers? Or are the MoltenVK libs now part of the VulkanSDK? Perhaps another workaround would be to query for the vkCreateRenderPass2/KHR function at runtime as we do for the mesh shader and RTX extensions.

…

Message ID: ***@***.***>

[iconiqlabs]

MoltenVK comes with its own headers. But there, they still do define the VK_VERSION_1_2.

If you can query for the function at runtime that is probably the tidiest approach. I have tried this to check I would work and it does

In the src/CMakeLists.txt after the target is created

if (IOS)
    set(MOLTEN_VK_CORE_HEADER ${Vulkan_INCLUDE_DIR}/MoltenVK/vk_mvk_moltenvk.h)
    if (EXISTS ${MOLTEN_VK_CORE_HEADER})
        # search for the Vulkan API actually being implemented
        file(READ ${MOLTEN_VK_CORE_HEADER} ver)
        string(REGEX MATCH "MVK_VRSION_MAJOR( +)[0-9]+" _ ${ver})
        set(MVK_VER_MAJOR ${CMAKE_MATCH_2})
        string(REGEX MATCH "MVK_VRSION_MINOR( +)[0-9]+" _ ${ver})
        set(MVK_VER_MINOR ${CMAKE_MATCH_2})
        string(REGEX MATCH "MVK_VRSION_PATCH( +)[0-9]+" _ ${ver})
        set(MVK_VER_PATCH ${CMAKE_MATCH_2})
        if (MVK_VER_MAJOR)
             message("-- Detected MoltenVK version ${MVK_VER_MAJOR}.${MVK_VER_MINOR}.${MVK_VER_PATCH}")
             target_compile_definitions(vsg PUBLIC MOLTEN_VK=1)
             foreach(mvk_major RANGE 1 ${MVK_VER_MAJOR})
                  foreach(mvk_minor RANGE ${MVK_VER_MINOR})
                       target_compile_definitions(vsg PUBLIC MVK_VERSION_${mvk_major}_${mvk_minor}=1)
                  endforeach()
             endforeach()
        endif()
    endif()
endif()

which causes macros MOLTEN_VK, MVK_VERSION_1_0 and MVK_VERSION_1_1 to be defined. Then, in the RenderPass.cpp code, using this:

#if !defined(VK_VERSION_1_2) || (defined(MOLTEN_VK) && !defined(MVK_VERSION_1_2))
    #define vkCreateRenderPass2 vkCreateRenderPass2KHR
#endif

makes it to build.

Not too bad, but no doubt your approach is better.

[robertosfield]

I think workaround for different MoltemVK versions could end up being awkward to maintain. I current feel the best approach will be just treat it as an extension and query the function pointer at runtime.

[robertosfield]

I have added a vkCreateRenderPass2 member to vsg::Extension, added use of this to RenderPass.cpp and checked this into a ExtensionRefactor branch: https://github.com/vsg-dev/VulkanSceneGraph/tree/ExtensionRefactor I'm not yet finished on this as I want to replace the static variables from the Extension.cpp, and add the ability to get the proc address of non extensions for cases where vkRenderPass2 is actually provided by the driver but the vkGetDeviceProcAddr function isn't locating it. The static member issue needs to be resolved as I've get a seg fault on exit from vsgdraw/vsgviewer etc because of this. While I've still got more work to do the checked in changes should be sufficient for testing under iOS. @iconiqlabs could you test whether these changes build and runtime on iOS?

[iconiqlabs]

Looks good. I made the test myself with this

$git diff
diff --git a/include/vsg/vk/Extensions.h b/include/vsg/vk/Extensions.h
index 5aa219e5..8039cd34 100644
--- a/include/vsg/vk/Extensions.h
+++ b/include/vsg/vk/Extensions.h
@@ -33,6 +33,8 @@ namespace vsg

         explicit Extensions(Device* device);

+        PFN_vkCreateRenderPass2KHR vkCreateRenderPass2KHR = nullptr;
+
         // VK_KHR_ray_tracing
         PFN_vkCreateAccelerationStructureKHR vkCreateAccelerationStructureKHR = nullptr;
         PFN_vkDestroyAccelerationStructureKHR vkDestroyAccelerationStructureKHR = nullptr;
diff --git a/src/vsg/vk/Extensions.cpp b/src/vsg/vk/Extensions.cpp
index 7e7f49ac..4328cd99 100644
--- a/src/vsg/vk/Extensions.cpp
+++ b/src/vsg/vk/Extensions.cpp
@@ -74,6 +74,8 @@ Extensions* Extensions::Get(Device* device, bool createIfNotInitalized)

 Extensions::Extensions(Device* device)
 {
+    vkCreateRenderPass2KHR = reinterpret_cast<PFN_vkCreateRenderPass2KHR>(vkGetDeviceProcAddr(*device, "vkCreateRenderPass2KHR"));
+
     // VK_KHR_ray_tracing
     vkCreateAccelerationStructureKHR = reinterpret_cast<PFN_vkCreateAccelerationStructureKHR>(vkGetDeviceProcAddr(*device, "vkCreateAccelerationStructureKHR"));
     vkDestroyAccelerationStructureKHR = reinterpret_cast<PFN_vkDestroyAccelerationStructureKHR>(vkGetDeviceProcAddr(*device, "vkDestroyAccelerationStructureKHR"));
diff --git a/src/vsg/vk/RenderPass.cpp b/src/vsg/vk/RenderPass.cpp
index 11604f72..0c0f7358 100644
--- a/src/vsg/vk/RenderPass.cpp
+++ b/src/vsg/vk/RenderPass.cpp
@@ -14,6 +14,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 #include <vsg/core/ScratchMemory.h>
 #include <vsg/io/Options.h>
 #include <vsg/vk/RenderPass.h>
+#include <vsg/vk/Extensions.h>

 #include <array>

@@ -161,7 +162,7 @@ RenderPass::RenderPass(Device* in_device, const Attachments& in_attachments, con
         renderPassInfo.correlatedViewMaskCount = static_cast<uint32_t>(correlatedViewMasks.size());
         renderPassInfo.pCorrelatedViewMasks = correlatedViewMasks.empty() ? nullptr : correlatedViewMasks.data();

-        VkResult result = vkCreateRenderPass2(*device, &renderPassInfo, device->getAllocationCallbacks(), &_renderPass);
+        VkResult result = Extensions::Get(device, false)->vkCreateRenderPass2KHR(*device, &renderPassInfo, device->getAllocationCallbacks(), &_renderPass);
         if (result != VK_SUCCESS)
         {
             throw Exception{"Error: vsg::RenderPass::create(...) Failed to create VkRenderPass.", result};

I think it is ok to merge your code into master. I will test it as soon as it is there :-)
Thanks a lot!

[robertosfield]

I have completed my changes to vgsg:Extension merged it with VSG master: #435 Message ID: ***@***.***> Could you test this and let me know whether it now works out of the box with iOS?

…

[iconiqlabs]

Hi Robert.
I just tested this and I confirm this is working.
Good job!

I'll make some more improvements in iOS support and let you know about them for merging them

[robertosfield]

On Tue, 26 Apr 2022 at 10:12, iconiqlabs ***@***.***> wrote: Hi Robert. I just tested this and I confirm this is working. Good job! I'll make some more improvements in iOS support and let you know about them for merging them

I haven't announced support for iOS on twitter yet, as I was waiting for the build to work out of the box. Should I wait to these further improvements or is it fine to go ahead? Do you have a twitter handle?

…

Message ID: ***@***.***>

[iconiqlabs]

I think it is fine to go ahead. The changes that are on the way are not expected immediately available. They are just 2:

• Keyboard events which are not very common in a viewer-only application on a handheld device (plus I still don't know how it works in iOS). So this is quite... minor.
• extract the render loop outside the vsg code. In a viewer-only application this is just a problem when one does not want the application to perform a frame every time a frame is requested. Probably the most important but not a showstopper.

I will need these two things in a framework I am writing. Will perform them when I get to the point.

I blush to confess I never used Twitte. But the occasion deserves it. I created an account for the company. Here https://twitter.com/IconiqLabs