Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VK_KHR_deferred_host_operations needs to be implemented before adding Raytracing Support #1953

Closed
AntarticCoder opened this issue Jun 16, 2023 · 13 comments

Comments

@AntarticCoder
Copy link
Contributor

AntarticCoder commented Jun 16, 2023

In issue #427 there has been discussion about the implementation of VK_KHR_ray_tracing_pipeline or VK_KHR_ray_query. Both of these extensions need the VK_KHR_acceleration_structure extension to work. To implement acceleration structures, we need the VK_KHR_deferred_host_operation, which allows for heavy operations that can take longer to do to be deferred until later. So as a small step towards raytracing, deferred_host_operations could be implemented.

This issue is dependent on issue #427

Edit: I'd like to try and implement this, could somebody point me to which files would need to be modified as I'm not quite sure where to start

@natevm
Copy link

natevm commented Jun 16, 2023

I’m not that familiar with MoltenVK, but I’m guessing the best first steps here would be to add a vkCreateDeferredOperationKHRand a vkDestroyDeferredOperationKHR that allocate / free an opaque VkDeferredOperationKHR object.

@AntarticCoder
Copy link
Contributor Author

Would an opaque handle of VkDeferredOperationKHR fit into GPUObjects/MVKSync.h?

@natevm
Copy link

natevm commented Jun 17, 2023

Personally I think that’s a good fit. This is a synchronization mechanism for sure, just a bit unique in that it’s about host-side asynchronous operations.

Only thing I wonder about is if this is part of “GPUObjects” or something else, since it’s a host-side thing, not necessarily a GPU side thing.

Worst case we can move it around later on in a pull request.

@AntarticCoder
Copy link
Contributor Author

AntarticCoder commented Jun 17, 2023

Thanks for the insight, I plan to start a PR on Monday/Tuesday

@AntarticCoder
Copy link
Contributor Author

AntarticCoder commented Jun 19, 2023

Another thing, how do we store the function pointer? There seems to be a variety of different functions from different extensions that can take a deferred operation. This means that the deferred operation needs to be able to refer to and execute the original operation. Issue is that each of these function has different parameters, so std::function or raw function pointers wouldn't work

Maybe we could create a union with each type of function, would that be all right?

Example:

union MVKDeferableOperations
{
   std::function<VkResult(
                        VkDevice,
                        VkDeferredOperationKHR
                        VkPipelineCache
                        uint32_t
                        const VkRayTracingPipelineCreateInfoKHR*
                        const VkAllocationCallbacks*
                        VkPipeline*
   ), // vkCreateRayTracingPipelinesKHR

   std::function<VkResult(
                        VkDevice
                        VkDeferredOperationKHR
                        uint32_t,
                        const VkMicromapBuildInfoEXT*
    )>, // vkBuildMicromapsEXT

    // ect ...
};

For each and every function that uses VkDeferredOperationKHR

@AntarticCoder
Copy link
Contributor Author

Opened a pull request, please link the issue with it: #1954

@natevm
Copy link

natevm commented Jun 19, 2023

IMO, a union seems like a decent way to do it. So long as there is a clear organization that can be extended.

@AntarticCoder
Copy link
Contributor Author

AntarticCoder commented Jun 19, 2023

That's how I implemented it, everytime a new vulkan command is implemented requiring deferred operations, you add the operations functions pointer to the union and also add the function to an enum that will manage with variable in the union should be used.

@natevm
Copy link

natevm commented Jun 19, 2023

Indirectly related to this issue, does Metal RT support host-side acceleration structure construction? (iiuc, that’s the reason this is a dependency of that, right?) If it doesn’t, then is it possible to still support vk_khr_acceleration_structure?

@cdavis5e
Copy link
Collaborator

Indirectly related to this issue, does Metal RT support host-side acceleration structure construction? (iiuc, that’s the reason this is a dependency of that, right?)

Nope. It only has device-side builds.

If it doesn’t, then is it possible to still support vk_khr_acceleration_structure?

Yes, because host-side builds are an optional feature.

@AntarticCoder
Copy link
Contributor Author

This issue can be closed, because it has been resolved in PR #1954 that has now been merged. The next step to raytracing would be to implement acceleration structures, I plan to create an issue soon.

@natevm
Copy link

natevm commented Jun 22, 2023

Nice work!! Looking forward to the next issue / PR.

Acceleration structures will be a tougher to implement I’d imagine, but if helps at all, I have a Vulkan ray tracing framework where I could test certain features as they’re added.

@AntarticCoder
Copy link
Contributor Author

Thanks I just opened an issue #1956

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants