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

The Vulkan allocator #614

Merged
merged 2 commits into from
Jul 7, 2022
Merged

The Vulkan allocator #614

merged 2 commits into from
Jul 7, 2022

Conversation

i509VCB
Copy link
Member

@i509VCB i509VCB commented Jun 9, 2022

Scope

First is a thin lightweight wrapper around ash to get an Instance and PhysicalDevices. This helper is kept intentionally lightweight to prevent essentially replicating a crate like vulkano. Creation of a logical Device is intentionally not covered since extensions to DeviceCreateInfo and queue creation are hard to model in a public API. The Vulkan allocator and a future renderer are expected to initialize their own logical device(s) and queues.

The Vulkan allocator is modeled around the Allocator trait. Implementing memory import/export and dmabuf import could be done, but it involves much more complex lifetimes (since memory import/export requires copy commands, which means command execution and needing to delay destruction of images until command execution finishes). In the future we could expand the allocator api to handle this as an alternative to gbm's memory import/export functions. This may be harder to do but will allow custom Vulkan renderers to be build more easily (since all the external memory logical will be in one type you can just pull in).

There are a few things we do need to do before merging:

  • A new ash release (Release ash 0.37.1 ash-rs/ash#632). 0.37.1 will not eliminate the builders, so if we do not want to wait for 0.38.0 then we will need to readd the builders. Will use 0.37.1 pre-release with a set revision for now.
  • Double check we do not have validation errors.

@i509VCB i509VCB requested a review from Drakulix June 9, 2022 05:36
@i509VCB i509VCB changed the base branch from master to rework-wayland-rs-0.30 June 14, 2022 06:52
@i509VCB i509VCB changed the base branch from rework-wayland-rs-0.30 to master June 14, 2022 06:52
@i509VCB i509VCB force-pushed the vulkan/allocator branch from a226371 to 02e904f Compare June 14, 2022 07:01
@i509VCB i509VCB changed the base branch from master to rework-wayland-rs-0.30 June 14, 2022 07:01
@i509VCB i509VCB changed the base branch from rework-wayland-rs-0.30 to master June 16, 2022 20:33
@i509VCB i509VCB force-pushed the vulkan/allocator branch 6 times, most recently from ffd08b2 to 7e05e25 Compare June 19, 2022 05:59
@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2022

Codecov Report

Merging #614 (1967a3e) into master (01ffd33) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #614      +/-   ##
==========================================
+ Coverage   31.04%   31.06%   +0.01%     
==========================================
  Files          84       84              
  Lines       13770    13770              
==========================================
+ Hits         4275     4277       +2     
+ Misses       9495     9493       -2     
Flag Coverage Δ
wlcs-core 28.50% <ø> (+0.01%) ⬆️
wlcs-output 10.47% <ø> (ø)
wlcs-pointer-input 30.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/backend/allocator/mod.rs 0.00% <ø> (ø)
src/desktop/space/mod.rs 59.23% <0.00%> (+0.36%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01ffd33...1967a3e. Read the comment docs.

@i509VCB i509VCB force-pushed the vulkan/allocator branch 2 times, most recently from a7f97af to c0adddf Compare June 19, 2022 21:56
Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Looks mostly good, some questions:

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/backend/allocator/vulkan/mod.rs Outdated Show resolved Hide resolved
@i509VCB i509VCB force-pushed the vulkan/allocator branch 3 times, most recently from 557e21b to fa43594 Compare June 26, 2022 22:06
@i509VCB i509VCB marked this pull request as ready for review June 26, 2022 22:08
@i509VCB i509VCB requested a review from Drakulix June 26, 2022 22:08
@i509VCB i509VCB force-pushed the vulkan/allocator branch 4 times, most recently from f88ea80 to 80421ef Compare June 27, 2022 15:30
Cargo.toml Outdated Show resolved Hide resolved
@i509VCB i509VCB force-pushed the vulkan/allocator branch from 80421ef to af8f746 Compare July 7, 2022 19:22
@i509VCB i509VCB force-pushed the vulkan/allocator branch from af8f746 to 8991f4a Compare July 7, 2022 19:24
@Drakulix Drakulix merged commit 82308c8 into Smithay:master Jul 7, 2022
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.

4 participants