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

Update to vulkan 1.1 SDK #14

Open
amerkoleci opened this issue Mar 13, 2018 · 12 comments
Open

Update to vulkan 1.1 SDK #14

amerkoleci opened this issue Mar 13, 2018 · 12 comments

Comments

@amerkoleci
Copy link
Contributor

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

Thanks

@mellinoe
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link

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
Copy link
Owner

mellinoe commented Mar 14, 2018

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
Copy link

mimvdb commented Sep 18, 2018

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
Copy link
Contributor Author

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

@mimvdb mimvdb mentioned this issue Sep 18, 2018
@mellinoe
Copy link
Owner

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
Copy link
Owner

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
Copy link
Contributor Author

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

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

4 participants