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 CreateInfo for Event, Fence and Semaphore. #1841

Merged
merged 5 commits into from
Feb 26, 2022

Conversation

Rua
Copy link
Contributor

@Rua Rua commented Feb 25, 2022

Changelog:

- **Breaking** `Fence` and `Semaphore` no longer have a type parameter.
- **Breaking** `Event` creation parameters are given using `EventCreateInfo`.
- **Breaking** `Fence` creation parameters are given using `FenceCreateInfo`.
- **Breaking** `Semaphore` creation parameters are given using `SemaphoreCreateInfo`.
- **Breaking** `Semaphore::export_opaque_fd` is now `unsafe`.
- Added `PhysicalDevice::external_semaphore_properties`.

Another relatively simple one.

@Rua
Copy link
Contributor Author

Rua commented Feb 25, 2022

The MacOS run keeps failing on this one, but it's failing when trying to load the Vulkan library at the start of a test. I didn't change anything about the library loading process in this PR, so it's quite strange that it fails so consistently on this. I wonder what's going on.

@Rua
Copy link
Contributor Author

Rua commented Feb 25, 2022

Now the issue also appears on Windows, and I'm even more confused.

@Rua
Copy link
Contributor Author

Rua commented Feb 25, 2022

I've tracked it down and it seems to actually be a possible issue with all other tests except this one. All other tests use the instance! macro, which is internal to Vulkano and generates a basic Instance to use for tests. If this macro can't create an instance then it just aborts the test and shows it as passed. The semaphore_export test on the other hand uses InstanceExtensions::supported_by_core() and then explicitly creates an instance, unwrapping on errors in both. The CI environments seem to be missing the Vulkan library on both Windows and Mac, so this one test panics while all the other tests... get silently skipped on CI.

@AustinJ235
Copy link
Member

I've tracked it down and it seems to actually be a possible issue with all other tests except this one. All other tests use the instance! macro, which is internal to Vulkano and generates a basic Instance to use for tests. If this macro can't create an instance then it just aborts the test and shows it as passed. The semaphore_export test on the other hand uses InstanceExtensions::supported_by_core() and then explicitly creates an instance, unwrapping on errors in both. The CI environments seem to be missing the Vulkan library on both Windows and Mac, so this one test panics while all the other tests... get silently skipped on CI.

Not sure if there is any real implementations for Windows or MacOS on Github actions. There is this https://github.com/humbletim/setup-vulkan-sdk which I don't know if that would provide enough? Think these platforms may have to remain build only for most tests.

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 42b4bd9 into vulkano-rs:master Feb 26, 2022
AustinJ235 added a commit that referenced this pull request Feb 26, 2022
@Rua Rua deleted the event-fence-semaphore branch May 31, 2022 18:36
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.

2 participants