extensions: Always return pipeline/shaders, even on error

[MarijnS95]

In ash::Device create_compute_pipeline() and create_graphics_pipeline() already return the list of pipelines regardless of the error code, as documented in the Multiple Pipeline Creation chapter. Callers are expected to scan the returned array for valid pipeline handles and either use or destroy them (when handling an error). Furthermore, the caller is guaranteed that all handles will be NULL after the first returned NULL handle when VK_PIPELINE_CREATE_EARLY_RETURN_ON_FAILURE_BIT is set.

An upstream Khronos/Vulkan request which has since been merged to the documentation makes all pipeline creation functions across the core and extensions point to this Multiple Pipeline Creation chapter which has also been improved, clarifying that they too return impartial results. This documentation reference has been embedded in ash.

VK_EXT_shader_object is similar in nature, but has its own documentation to account for the difference in naming.

[Ralith]

It's tempting to push for (Vec<T>, Result<()>) here, but it does make it a lot easier to forget to check the error condition.

[MarijnS95]

It's tempting to push for (Vec<T>, Result<()>) here, but it does make it a lot easier to forget to check the error condition.

Yeah it's unfortunate that there's no lint to catch an unused Result when it is returned as part of a tuple.

[MarijnS95]

https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap10.html#pipelines-multiple

Turns out there's actually documentation for this, and the references+language are being updated upstream.

I'd still like to think of a better way to represent the validity of the return values (and necessity to clean up non-NULL handles) before merging this in.

[Ralith]

We could do Vec<Result<T>>? Might cause some confusion for errors due to one pipeline getting broadcast to multiple, but has a lot going for it otherwise.

[MarijnS95]

I've been contemplating that but it sets the false precedent that every pipeline individually has an error code. It seems better than the status-quo, though!

[Ralith]

(Vec<Option<T>>, VkResult)?

[MarijnS95]

Also considered, but that might be ambiguous with None vs Some(vk::Handle::null()) 😅

[MarijnS95]

We're gonna write high-level bindings at this point, but how about an OwnedPipeline that has an fn take(self) -> vk::Pipeline, which drops itself if it wasn't taken (via std::mem::take())?

It wouldn't have access to the device to do so though if the type is #[repr(transparent)].

[Ralith]

Anything like RAII ownership of Vulkan objects starts feeling out of scope for ash. We could panic rather than destroy, but I'm not sure that'll do much good compared to a static mechanism.

I feel like the original proposal here (Result<Vec<T>, (Vec<T>, VkResult)>) is fine, honestly. It's weird looking but gives a pretty strong static cue that you have to do some work on error.

[MarijnS95]

Anything like RAII ownership of Vulkan objects starts feeling out of scope for ash.

It'd also start implying that similar mechanisms are available or supposed to be available on other types, which wouldn't be the case by for ash in any shape or form.

[MarijnS95]

Upstream documentation improvements have been merged, command pages now contain this:

Pipelines are created and returned as described for Multiple Pipeline Creation.

I'll adopt and update the relevant portions of that doc here in ash on all functions, before merging this. While the function interface indeed shows that users have to do some work on error, it's better to explain and/or link what explicitly.

[MarijnS95]

Perhaps that doc is a bit excessive. I pruned it down somewhat, but we could also boil it down to just the link, or perhaps at the very least (because of the return signature) state that valid references may always be returned - corresponding to create_infos[i] - and should either be used or cleaned up.

[Ralith]

Yeah, I think something a lot more terse will suffice. No need to duplicate upstream docs at length. Maybe something like:

If multiple pipelines are being created in one call, a subset of pipelines might be created even in the event of an error. Care should be taken not to leak these.