Store function pointers in an Option

[Friz64]

Currently, when a function pointer is not loaded from Vulkan, it will still be populated, but with a stub implementation that panics at runtime. For some history: This was introduced in #137, because loaders previously errored out when failing to load functions that had additional requirements in order to be loaded (#136).

I think we should consider storing the function pointers in Options instead. Yes, it will result in a runtime null-check every time a Vulkan function gets called, but that check has no real-world impact in the runtime performance of a Vulkan program. I previously benchmarked this: https://gitlab.com/Friz64/erupt/-/issues/27.

It makes it really obvious and explicit when a function pointer was not loaded, to no real disadvantage. The current runtime panic solution feels weird.

I originally alluded to this in this comment: #734 (comment).

[Friz64]

Or, even better: Only store extension function pointers in an Option that are not guaranteed to succeed loading (read: that have special requirements). So for VK_KHR_swapchain, that would look like this:

pub struct KhrSwapchainFn {
    // no requirements
    pub create_swapchain_khr: PFN_vkCreateSwapchainKHR,
    pub destroy_swapchain_khr: PFN_vkDestroySwapchainKHR,
    pub get_swapchain_images_khr: PFN_vkGetSwapchainImagesKHR,
    pub acquire_next_image_khr: PFN_vkAcquireNextImageKHR,
    pub queue_present_khr: PFN_vkQueuePresentKHR,
    // If Version 1.1 is supported:
    pub get_device_group_present_capabilities_khr: Option<PFN_vkGetDeviceGroupPresentCapabilitiesKHR>,
    pub get_device_group_surface_present_modes_khr: Option<PFN_vkGetDeviceGroupSurfacePresentModesKHR>,
    pub get_physical_device_present_rectangles_khr: Option<PFN_vkGetPhysicalDevicePresentRectanglesKHR>,
    pub acquire_next_image2_khr: Option<PFN_vkAcquireNextImage2KHR>,
}

[Ralith]

that check has no real-world impact in the runtime performance of a Vulkan program.

I agree. I'm more interested in which one compiles faster, since ash currently imposes a disproportionate buildtime cost for many users. My intuition is that an indirect function call is faster to compile than an unwrap followed by an indirect function call, but perhaps our real costs are elsewhere and the difference is insignificant.

It makes it really obvious and explicit when a function pointer was not loaded

I disagree. The overwhelming majority users will be using the high-level wrappers, which will panic in exactly the same way regardless of the internal representation. In that light, the proposed change seems like unnecessary churn.

Only store extension function pointers in an Option that are not guaranteed to succeed loading

Does the registry have enough information to do this easily?

[Friz64]

Does the registry have enough information to do this easily?

Yes, this should be easy to do. The XML literally looks like this:

<extension name="VK_KHR_swapchain">
    <require>
        <command name="vkCreateSwapchainKHR"/>
        <command name="vkDestroySwapchainKHR"/>
        <command name="vkGetSwapchainImagesKHR"/>
        <command name="vkAcquireNextImageKHR"/>
        <command name="vkQueuePresentKHR"/>
    </require>
    <require feature="VK_VERSION_1_1">
        <command name="vkGetDeviceGroupPresentCapabilitiesKHR"/>
        <command name="vkGetDeviceGroupSurfacePresentModesKHR"/>
        <command name="vkGetPhysicalDevicePresentRectanglesKHR"/>
        <command name="vkAcquireNextImage2KHR"/>
    </require>
</extension>

There's just one thing at the back of mind: I once did some testing with erupt on my Android phone 3-4 years ago (Pixel 4), and its Vulkan loader failed loading a function which it should have, according to the requirements. I don't have the details anymore, but that's something to be potentially wary of.

[Ralith]

I recall @kvark reported something similar here once, but IIRC it went away. I don't think we should go far out of our way for the benefit of severely broken drivers.

[MarijnS95]

The only relevance I see here to #734 is ash's "wrong" 1:1 mapping between function pointers (and relevant types/typedefs to some extent) and necessary extensionS/features. As seen in #734 some function pointers are only available on extension x AND y, and others are available on extension x OR y (and sometimes even more complex expressions). We currently have no good/clear way to represent them besides Option or the current panicking behaviour, as well as vague duplication (some functions are loaded by both Fn structs).

I have yet to come up with a better representation than Option and a doc-comment as suggested above, but that doesn't defeat the duplication incurred by x OR y functions. Again: where two extensions enable or give access to the same function: if you search for Also available as in the codebase, you'll find my generic doc-comment that covers this duplication.

Seems we're tracking that in #878 though?

[Ralith]

#567 is also relevant reading, in which we found that mucking with the repr/getting rid of the stubs doesn't seem to help build times.

[MarijnS95]

The tracking issue for multiple extensions referencing the same commands is at #635.