mellinoe / vk

Low-level Vulkan bindings for .NET.
Other
176 stars 35 forks source link

Update to vulkan 1.1 SDK #14

Open amerkoleci opened 6 years ago

amerkoleci commented 6 years ago

Any plans to update to newest 1.1 release? I can submit PR in case.

Thanks

mellinoe commented 6 years ago

Yeah, I just haven't gotten around to it yet. Unfortunately there's some new stuff in the XML spec that needs to be parsed out. There's now a bunch of "alias" definitions that I'm not sure how to handle. Maybe they can be ignored for now, I haven't actually looked too closely.

amerkoleci commented 6 years ago

I tried quickly and indeed the "alias" is something we need to solve tho, all KHR, Ext methods now are standard,waiting for your info :)

Thanks

mellinoe commented 6 years ago

I guess the right thing to do is just ignore the aliases. There is no type aliasing in C#, so there's no way to cleanly represent them. There have been breaking changes like this in the past, there just weren't aliases to replace them at the time. Maintaining backwards compatibility for this will be pretty hard, so maybe it's not worth it. Function aliases could be easier to represent with another overload, but it wouldn't make sense to have those and nothing else.

Thoughts?

amerkoleci commented 6 years ago

WE can keep both old and new function, but will increase library size, or maybe we can route the new function without KHR to call the legacy KHR functions, Types does not alias tho.

mellinoe commented 6 years ago

It might be tricky to keep the old function signatures around. The reason is that they are now simply defined as aliases to the regular functions. However, for backwards compatibility, we also need all of the parameters to be their aliased versions too. For example:

New core version:

void vkGetPhysicalDeviceProperties2(
    VkPhysicalDevice physicalDevice,
    VkPhysicalDeviceProperties2* pProperties);

Old version, now aliased:

void vkGetPhysicalDeviceProperties2KHR(
    VkPhysicalDevice physicalDevice,
    // This parameter needs to have the aliased "KHR" type.
    VkPhysicalDeviceProperties2KHR* pProperties)

This might not actually be that hard. When generating an aliased function, we can automatically use the aliased versions of any parameters, if they exist. Practically speaking, that should always work. I don't expect that aliases will be used to redirect random types around for no reason -- it will probably just be for extension->core promotions.

It will bloat the library a little bit, so we can put the aliased stuff behind an #if for folks who really care. This way we can keep the NuGet package fully backwards compatible.

FacticiusVir commented 6 years ago

So the aliases only exist for mapping 1.0 extensions to 1.1 core functions, and some cases where the 1.1 version of an extension is an amalgam of 1.0 extensions. You'll find some of the extension enums duplicated (for not very obvious reasons) as well. I'm looking at splitting SharpVk into Api1_0 & Api1_1 namespaces, so the aliases are used to generate extension methods that only apply to Api1_0, but unless you're looking at specifically supporting 1.0 you can ignore them.

mellinoe commented 6 years ago

You'll find some of the extension enums duplicated (for not very obvious reasons) as well

Yeah, I noticed that... 🙁 . The XML spec is an absolute mess, TBH.

some cases where the 1.1 version of an extension is an amalgam of 1.0 extensions.

Do you know which ones this was done for?

I'm looking at splitting SharpVk into Api1_0 & Api1_1 namespaces

Hmm, it makes some sense, but it's still a breaking change. Supporting the aliased functions would probably only make sense if everything was still backwards compatible, IMO. If you need to make changes downstream, you might as well just remove the suffixes in your types/function calls -- it's not that much work.

mimvdb commented 6 years ago

For reference here is my quick and dirty port to 1.1 https://github.com/mimvdb/vk/tree/vulkan1.1
Ignores aliases, all samples work for me.

amerkoleci commented 6 years ago

@mimvdb can you submit PR and maybe @mellinoe can merge it and we can start from there?

mellinoe commented 6 years ago

The branch above looks like it's going in the right direction, but there's still some key things discussed above that aren't fully figured out. Ignoring the aliases entirely is an okay stopgap to use to test things out, but we will need to come up with some sort of plan if the changes are to be merged. Some level of care needs to be given to backwards compatibility. I'm especially concerned about bringing forward Veldrid, which is going to continue using primarily 1.0-level features for the foreseeable future.

I think this comment still fairly accurately describes what a working solution would look like. I think it would involve parsing both 1.0 and 1.1 spec files, and using the "alias" information to map between them in some way (if that's even necessary).

Unfortunately, this is pretty far down my list of priorities at the moment. Like I said, Veldrid is going to continue using 1.0-level features, so I don't have an urgent need to update on my end.

mellinoe commented 6 years ago

I should also mention that I've been thinking a lot about re-designing this library into something closer to flextgl/flextvk. Instead of the library as it exists today, this repo would produce a code generator plugin that you could install and which would inject the vulkan loader code directly into your project.

That way, there is no longer any "public API" to maintain or break -- it all becomes an internal detail of your project. It would also allow you to define a small subset of functionality that you are actually interested in using, which would speed up load time.

amerkoleci commented 6 years ago

Or something similar to https://github.com/zeux/volk as well?