EvergineTeam / Vulkan.NET

This repository contains low-level bindings for Vulkan used in Evergine.
MIT License
199 stars 18 forks source link

VkBool32 noncompliance. #7

Closed stromkos closed 2 years ago

stromkos commented 2 years ago

https://github.com/EvergineTeam/Vulkan.NET/blob/d4117431b7efd79d12c263756e2c4daae4651b09/VulkanGen/Evergine.Bindings.Vulkan/VKBool32.cs#L31-L34

To quote from 3.5 of the spec: https://www.khronos.org/registry/vulkan/specs/1.3/pdf/vkspec.pdf

Applications must not pass any other values than VK_TRUE or VK_FALSE into a Vulkan implementation where a VkBool32 is expected.

The restriction may be an internal and with 1, thus making any odd value true.

I would add a line before line 33:

if (value > 1) throw new System.Exception("Invalid Value for VkBool32");
Value = value;`

Or

Change line 33 to: Value = (value==1)?1:0; and add the validation to the implicit cast on line 65:

public static implicit operator VkBool32(uint value)
{
   if (value > 1) throw new System.Exception("Invalid Value for VkBool32");
   return new VkBool32(value);
}

https://github.com/EvergineTeam/Vulkan.NET/blob/d4117431b7efd79d12c263756e2c4daae4651b09/VulkanGen/Evergine.Bindings.Vulkan/VKBool32.cs#L65

The problem with the implicit cast is that anything implicitly castable to a uint is also implicitly castable to a Vk_Bool32.


Without one of these changes, if I place method parameters in the wrong order, they may be silently coerced.

I realize this project is a low-level shim, but C# type safety should be maintained wherever possible and the API's accepted value ranges honored.

jcant0n commented 2 years ago

Hi,

Yes, this is a low level api and we try to add only code autogenerated.

But this is a simple change to add in a struct created manually, so I have added this:

https://github.com/EvergineTeam/Vulkan.NET/commit/d58fe3d536782fe5a69767e039556ec2542f9bd7

Hope this helps!

stromkos commented 1 year ago

Thank you,

I understand the auto-generation aspect.

The struct in question was created manually, so thank you very much.