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

Add VulkanLibrary as first initialization step before Instance #1932

Merged
merged 3 commits into from
Jul 30, 2022

Conversation

Rua
Copy link
Contributor

@Rua Rua commented Jul 24, 2022

Changelog:

- **Breaking** Changes to `Instance` and Vulkan initialization:
  - `FunctionPointers` is renamed to `VulkanLibrary`, and now resides in a separate `library` module. It is re-exported from the crate root.
  - The `Loader` trait is now in the `library` module.
  - `Instance` now requires a `VulkanLibrary` object, which you must create beforehand.
  - The `auto_loader` function is removed.
  - Supported extensions and layers are now retrieved from the `VulkanLibrary` object. The old `layers_list` and `InstanceExtensions` methods are removed.
  - The deprecated methods of `DeviceExtensions` are removed.
  - Vulkano-win: `required_extensions` now requires a reference to `VulkanLibrary`.
- **Breaking** Changes to Vulkano-util:
  - Required instance extensions for surface creation aren't added until `VulkanoContext` is created.
  - The `instance`, `device`, `graphics_queue` and `compute_queue` methods of `VulkanoContext` now return a reference instead of an owned `Arc`.

This closes #1876 by partially reimplementing it, but differently.

This is still a draft, because I ran into a problem making Vulkano-util conform to the new API. VulkanoConfig has a Default implementation, but that no longer works now that the user must load the library themselves. @hakolao do you know of a good solution to this? If you think it's better, I can leave this PR as is, and give you the opportunity to fix it yourself later.

@hakolao
Copy link
Contributor

hakolao commented Jul 24, 2022

I’ll try to come up with something later today, cant think of a good solution right away.

@hakolao
Copy link
Contributor

hakolao commented Jul 24, 2022

@Rua How about something like this?

So the required extensions can be appended to the inputs from config. That way Default can remain None, and user can still add the extensions that they want, which are not required extensions.

On top of that, I think that the VulkanLibrary should probably be accessible from VulkanoContext? Unless it can be retrieved through instance. Either would be fine I think.

@Rua Rua marked this pull request as ready for review July 25, 2022 11:09
@Rua
Copy link
Contributor Author

Rua commented Jul 25, 2022

This is now ready for review.

Copy link
Member

@AustinJ235 AustinJ235 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@AustinJ235 AustinJ235 merged commit 6a755cd into vulkano-rs:master Jul 30, 2022
AustinJ235 added a commit that referenced this pull request Jul 30, 2022
@Rua Rua deleted the VulkanLibrary branch September 17, 2022 11:55
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

Successfully merging this pull request may close these issues.

3 participants