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.3.240 #48

Merged
merged 11 commits into from
Oct 6, 2023
Merged

Update to Vulkan 1.3.240 #48

merged 11 commits into from
Oct 6, 2023

Conversation

serenity4
Copy link
Member

@serenity4 serenity4 commented Feb 10, 2023

This uses the latest Clang version, which showed no difference in the generated wrapper (tested for the Linux wrapper) before the update to 1.3.240.

I also removed a bunch of const annotations to facilitate tweaking the generator while prototyping.

TODO:

@serenity4 serenity4 self-assigned this Feb 10, 2023
@serenity4
Copy link
Member Author

serenity4 commented Feb 10, 2023

It seems like there are a few issues with finding libvulkan on MacOS, I don't know if that's a file permission issue or a wrong library name or a missing library include path. This guide (though perhaps a bit outdated) seems to indicate the use of libvulkan.1.dylib as library name, but setting this CI had no effect.

EDIT: I just realized it was already captured in #37.

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (97959a1) 34.42% compared to head (861ecb6) 33.87%.
Report is 2 commits behind head on master.

❗ Current head 861ecb6 differs from pull request most recent head f09d780. Consider uploading reports for the commit f09d780 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
- Coverage   34.42%   33.87%   -0.56%     
==========================================
  Files           3        3              
  Lines          61       62       +1     
==========================================
  Hits           21       21              
- Misses         40       41       +1     
Files Coverage Δ
src/LibVulkan.jl 72.72% <0.00%> (-7.28%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@serenity4
Copy link
Member Author

Windows CI timed out during the installation of the SDK, it might be that the .exe requires interaction of some sort, though it used to be working fine before.

@serenity4
Copy link
Member Author

Alright, it got me some time (with a distraction under the form of https://github.com/serenity4/VulkanSpec.jl) but this PR is tested and ready now :)

FYI, the Vulkan Video bindings are only for decode operations, which are no longer provisional, and encode operations (which are still provisional) are not included. Therefore it should be in line with our discussion in #43.

I may update to v1.3.266 (or whatever will be the latest) soon after this PR, but if that is the case, it's anyways good to make incremental changes.

@serenity4 serenity4 marked this pull request as ready for review October 5, 2023 17:29
@serenity4
Copy link
Member Author

(@Gnimuc if you have time to review, that would be fantastic, but otherwise it's alright - I am reasonably confident that I can merge this in the current state)

Copy link
Member

@Gnimuc Gnimuc left a comment

Choose a reason for hiding this comment

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

The generator script looks good to me.

@Gnimuc
Copy link
Member

Gnimuc commented Oct 6, 2023

We may fix the Windows CI in the future though. (The last time I tried, it was still hard to debug the CI environment...)

@serenity4 serenity4 merged commit 1936f24 into JuliaGPU:master Oct 6, 2023
1 of 2 checks passed
@serenity4
Copy link
Member Author

Thanks! Yep, eventually we should try to get Vulkan to work reliably on all platforms. I've been trying to put the validation layers in Yggdrasil, but it's not an easy one.

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