Fix compiler errors on newer vulkan implementations

[Weckyy702]

Hey there, I was interested in using this library but found out that it didn't compile on my Arch machine with newest version of everything.

There were two errors:

1. delayed.hpp:96:22: error: ‘unique_ptr’ in namespace ‘std’ does not name a template type: This is simply fixed by including <memory>
2. device.cpp: In member function ‘vk::Pipeline vuh::Device::createPipeline(vk::PipelineLayout, vk::PipelineCache, const vk::PipelineShaderStageCreateInfo&, vk::PipelineCreateFlags)’: /home/weckyy702/Documents/dev/vuh/src/device.cpp:221:45: error: could not convert ‘((vuh::Device*)this)->vuh::Device::<anonymous>.vk::Device::createComputePipeline<>(pipe_cache, pipelineCI, vk::Optional<const vk::AllocationCallbacks>(nullptr), (*(const vk::DispatchLoaderStatic*)(& vk::getDispatchLoaderStatic())))’ from ‘vk::ResultValue<vk::Pipeline>’ to ‘vk::Pipeline’: With this one, createComputePipeline returns a ResultValue that must be unwrapped first. I used an assert here, but there is likely a better way to handle the possible error. I'd appreciate feedback on that, thanks.

[Glavnokoman]

1. delayed.hpp:96:22: error: ‘unique_ptr’ in namespace ‘std’ does not name a template type: This is simply fixed by including <memory>

Ah, my bad!

2. `device.cpp: In member function ‘vk::Pipeline .... I used an assert here, but there is likely a better way to handle the possible error.

Yes, assert is not a good way to handle the failure to create the pipeline. The best way would probably be to comply with the change in the vk interface and return a vk::ResultValue. That would trigger a wave of changes throughout the codebase though and I am not sure how large is the set or required changes. Could you maybe invest some effort into this and see if we can do it right?

[plexx-dev]

LGTM

[Weckyy702]

1. delayed.hpp:96:22: error: ‘unique_ptr’ in namespace ‘std’ does not name a template type: This is simply fixed by including <memory>

Ah, my bad!

2. `device.cpp: In member function ‘vk::Pipeline .... I used an assert here, but there is likely a better way to handle the possible error.

Yes, assert is not a good way to handle the failure to create the pipeline. The best way would probably be to comply with the change in the vk interface and return a vk::ResultValue. That would trigger a wave of changes throughout the codebase though and I am not sure how large is the set or required changes. Could you maybe invest some effort into this and see if we can do it right?

Sure, I'll look into it!

[Weckyy702]

Maybe we could do something like the TRY-macro? Where we return early if a ResultValue does not contain eSuccess. That would be quite elegant

like in program.hpp:330: _pipeline = VUH_TRY(_device.createPipeline(...));

EDIT: It looks like MSVC does not support Statement Expressions. And since we probably don't want to drop MSVC compatibility, I will have to go for a less elegant solution. The line from above would look like

VUH_TRY(_device.createPipeline(...), pipeline);
_pipeline = std::move(pipeline);

[Weckyy702]

Ah, thanks for approving but the current system silently swallows errors. I'm currently testing a system that uses a custom Result class so we can surface errors to the user. I'll commit that once all the shenanigans are figured out (e.g vuh::Delayed is move-only)

[Weckyy702]

So, just finished the overhaul.

The new Result<T> contains an optional T and a vk::Result to indicate the potential error. The VUH_TRY* macros allow to return early from a function if there is an error. The errors now bubble up to the user through the normal return paths and the user will get a warning if they ignore a Result since it is declared [[nodiscard]].

[plexx-dev]

LGTM