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

Explicit 'count' field in AccelerationStructureBuildGeometryInfoKHR #239

Closed
sheaf opened this issue Dec 11, 2020 · 3 comments
Closed

Explicit 'count' field in AccelerationStructureBuildGeometryInfoKHR #239

sheaf opened this issue Dec 11, 2020 · 3 comments

Comments

@sheaf
Copy link
Collaborator

sheaf commented Dec 11, 2020

I'm wondering why AccelerationStructureBuildGeometryInfoKHR has an explicit geometryCount field, as this is just supposed to be the size of the geometries vector. In other places in this library, this field is usually elided.
I assume that this ended up being a special case, because the original Vulkan function can work with either a pointer to an array of geometries or a pointer to an array of pointers to geometries?

  • Only one of pGeometries or ppGeometries can be a valid pointer, the other must be NULL
  • If geometryCount is not 0, and pGeometries is not NULL, pGeometries must be a valid pointer to an array of geometryCount valid AccelerationStructureGeometryKHR structures
  • If geometryCount is not 0, and ppGeometries is not NULL, ppGeometries must be a valid pointer to an array of geometryCount valid pointers to valid AccelerationStructureGeometryKHR structures
@expipiplus1
Copy link
Owner

This is a deficiency in the bindings.

The cause is:

Lengths are elided if they can be uniquely determined by the lengths of Vectors being passed. In this case both arrays which are sized by this length are optional:

https://github.com/KhronosGroup/Vulkan-Docs/blob/8f718b4194ed1e0a572d37072e5558dd9ceabcb0/xml/vk.xml#L4772-L4774

This means that (according to just the XML spec) one could pass a length and NULL for both vectors. The XML spec lacks the language to say that only one of these arrays can be NULL.

Usually there's a 1-1 mapping between arrays and Vectors, however in this specific case we ditch ppGeometries and make pGeometries mandatory.

The easiest solution would be to assign it ElidedLength ["pGeometries"] [] (or something like that) here:

accelerationStructureGeometry :: BespokeScheme
accelerationStructureGeometry = BespokeScheme $ \case
"VkAccelerationStructureBuildGeometryInfoKHR" -> \case
(p :: a) | "ppGeometries" <- name p, Ptr Const (Ptr Const _) <- type' p ->
Just $ ElidedUnivalued "nullPtr"
_ -> Nothing
_ -> const Nothing
. I can take a look at this next week

For now it's safe to set it to zero and it'll be inferred:

geometryCount'' <- lift $ if (geometryCount) == 0
then pure $ fromIntegral pGeometriesLength

@sheaf
Copy link
Collaborator Author

sheaf commented Dec 11, 2020

This obviously isn't causing me any trouble at all so you don't need to worry about it.

@expipiplus1
Copy link
Owner

Fixed with #251

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

No branches or pull requests

2 participants