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 Vulkan-Headers to 1.3.206 #563

Merged
merged 2 commits into from
Feb 19, 2022
Merged

Conversation

MarijnS95
Copy link
Collaborator

Vulkan 1.3 is out, with many extensions stabilized. This'll have to remain draft until the next spec version (hopefully next week) where (again, hopefully) KhronosGroup/Vulkan-Docs#1746 is incorporated - otherwise we'll have to work around in our generator :(

@MarijnS95
Copy link
Collaborator Author

Can't update to 1.3.205 either, vk.xml got broken there as well: KhronosGroup/Vulkan-Headers#258 (comment)

@Rua
Copy link
Contributor

Rua commented Feb 5, 2022

Seems to have been fixed in KhronosGroup/Vulkan-Docs#1764.

@MarijnS95 MarijnS95 marked this pull request as ready for review February 6, 2022 06:10
@MarijnS95 MarijnS95 changed the title Update Vulkan-Headers to 1.3.204 Update Vulkan-Headers to 1.3.205 Feb 6, 2022
@MarijnS95 MarijnS95 requested a review from Ralith February 6, 2022 17:26
Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

If it wasn't clear, I think this is a breaking change, because e.g. Extends* for structs that were made core have been renamed.

@MarijnS95
Copy link
Collaborator Author

@Ralith Do we think that's a problem, or an unsafe implementation detail? After all, callers are not supposed to implement these on their own. However, crates may also decide to use these trait bounds for their own purposes (perhaps something that "wraps around" push_next).

I'm thinking about getting a patch release out now then, and release a breaking release with Vulkan 1.3 and the rest shortly after. It's a large update that "deserves some attention" anyway.

@Ralith
Copy link
Collaborator

Ralith commented Feb 18, 2022

It's definitely a technical semver break, since it is legal (regardless of unsafe) for downstream code to reference the changed identifiers. I think the real-world impact is probably low, but I also don't see a need to play fast and loose with semver.

@MarijnS95
Copy link
Collaborator Author

MarijnS95 commented Feb 18, 2022

Should have probably not mentioned unsafe implementing the traits by clients at all (which I can't see any valid reason for), since being able to use this trait bound in a safe wrapper is already well enough of a possibility to risk an API break.

I don't want to go figure out how to add a bunch of type aliases for these either, at least :)

@Ralith
Copy link
Collaborator

Ralith commented Feb 18, 2022

Yeah, I don't think that's worth the trouble; this only comes up on Vulkan minor version bumps anyway.

@MarijnS95 MarijnS95 changed the title Update Vulkan-Headers to 1.3.205 Update Vulkan-Headers to 1.3.206 Feb 19, 2022
@MarijnS95 MarijnS95 merged commit 955cb28 into ash-rs:master Feb 19, 2022
@MarijnS95 MarijnS95 deleted the update branch February 19, 2022 09:18
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