GpuFuture - Redesign RFC

[LeonMatthes]

Hi,

I'm currently using vulkano in a personal project as my main access to Vulkan. In many ways it's so much nicer than normal ash or other direct vulkan access, whilst retaining a tight level of control. Especially coming from OpenGL, whilst Vulkan means more boilerplate, I also finally understand how everything ties together.

One aspect of Vulkano that I've avoided in my project until now is the use of GpuFuture. Whilst the API is simple to use, I also feel it oversimplifies vulkan synchronization and results in some strange behavior like #1705 .

I've spent some time trying to narrow down this disconnect between Vulkan <-> GpuFuture recently, as I'd like to be comfortable using the simpler GpuFuture API (especially as the raw Submit API has been changed after vulkano 0.31).

From my research the main issue seems to be that GpuFuture actually takes care of multiple concerns, that may be better adressed individually:

1. Buffering multiple queue operations (anything before calling flush).
2. Handling synchronization between queue operations (then_execute, then_signal_semaphore, then_signal_fence).
3. Representing an event that will happen on the GPU in the future and cleaning up resources when the event has happened. (cleanup_finished, signal_finished, etc.).

Because GpuFuture takes care of all three of these, it ends up handling none of them perfectly. GpuFuture also ends up creating a tree, instead of a DAG, which is what synchronization with multiple queues actually is.

Of course I'm willing to contribute myself but don't want to waste time implementing something that other maintainers don't agree on. This is why I'm asking for comments and suggestions in this issue before I start implementing.

My current thoughts look something like this:

• Outsource concerns 1&2 into a submission builder, similar to the previous SubmitCommandBufferBuilder. This approach seems to be even outlined in DESIGN.md.
• Change GpuFuture to only represent an event that has been submitted and will happen in the future on a GPU.

We can create futures for the vulkan synchronization primitives to allow for manual synchronization. This would also allow us to use type restrictions for inter-future dependencies. e.g.:

• A SubmissionBuilder can only wait for SemaphoreFutures or TimelineSemaphoreFutures.
• Only the CPU can wait on a Fence, not another queue operation.

Proposed API - triangle example

let (fence, present) = previous_frame_end.take();
fence.wait();

// Result always needs to include either a SemaphoreFuture, FenceFuture or both so that there is some synchronization primitive to wait on.
let acquisition = swapchain.acquire_next_image(SwapchainAcquireInfo {
    signal_semaphore: true,
    ..Default::default()
});

let mut submission_builder = render_queue.submit();
let draw_semaphore = submission_builder
    .after(acquisition.semaphore.take())
    .commands(command_buffer)
    .signal_semaphore();
let draw_fence = submission_builder.submit_with_fence(); // returns a `FenceFuture`

let present_future = present_queue.present(SwapchainPresentInfo {
    wait_for: draw_semaphore,
    ..SwapchainPresentInfo::swapchain_image_index(swapchain, acquisition.image_index)
});
let previous_frame_end = (draw_fence, present_future);

Compare this to the old API:

let acquire_future = acquire_next_image(swapchain.clone(), None); // simplified

let future = previous_frame_end
    .take()
    .unwrap()
    .join(acquire_future)
    .then_execute(queue.clone(), command_buffer)
    .unwrap()
    .then_swapchain_present(
        queue.clone(),
        SwapchainPresentInfo::swapchain_image_index(swapchain.clone(), image_index),
    )
    .then_signal_fence_and_flush();

Whilst the old API is shorter, it also tries to hide some important details:

• the command_buffer queue might be a different one compared to the swapchain present queue, which would require a semaphore. This is currently missing.
• swapchain_present cannot signal a fence, so then_signal_fence_and_flush has to add an empty queue submission (possibly on the wrong queue).
• Signalling multiple semaphores after command buffer execution isn't possible. The documentation of GpuFuture even lists this as a ToDo.

What this tries to solve

• 1705

• Synchronization between multiple queues (multiple calls to signal_semaphore possible)
  • Futures will then represent a dependency-DAG, which is the correct representation.
• Potentially: #1882 (Add TimelineSemaphoreFuture and appropriate functions to SubmissionBuilder)

Open Questions

• What happens if a SemaphoreFuture is just dropped? Nothing ever waits for the Semaphore... Do we just panic? - I believe this is already an issue today.
• How exactly would futures share the "source event". E.g. a SemaphoreFuture and a FenceFuture would refer to the same submission. If any of them finish, the others would be finished as well.
• Exact GpuFuture trait API (draft see below)

New GpuFuture trait:

Should include the following:

unsafe fn signal_finished(&self);
fn cleanup_finished(&self);
fn finished(&self) -> bool;
// blocking wait until this future has finished (basically drop impl)
// Ideally should only be used with fences.
fn wait(&mut self);

[Rua]

GpuFuture is likely to be deprecated and removed sometime in the future. In Vulkano 0.32, new methods were added to Queue that mirror the plain Vulkan API functions more directly. There are currently only _unchecked variants of these functions (which only show up in the documentation if you enable the document_unchecked feature), there is no safe implementation yet. A safe implementation requires a much more thorough representation of Vulkan synchronization. This is in the works, but it has to start at the bottom, in command buffers. There is currently a new CommandBufferBuilder in development for that purpose, but it will take a while before it's done.

[LeonMatthes]

@Rua thanks for the answer, this makes a lot of sense. I've already found these functions on Queue and the fact that queue takes care of a lot of resource handling already. I'll start using the unchecked functions then for now.

Is there some sort of milestone or issue that tracks these upcoming changes? I'd like to contribute but don't really know what to implement and what the next issues are. 😊 Let me know what some good first issues would be.

[Rua]

I don't think there is anything you can do for the moment. The synchronization code in the new command buffer is quite difficult, and I'm studying how the official validation layer does it, so that I can get an idea what to do for Vulkano. I can list the things I want to do at some point though:

• Make the new command buffer track all the synchronization state. Currently some things are missing, including render pass transitions, render pass barriers (subpass dependencies), clear_attachments, execute_commands and set_event/wait_events.
• Implement PrimaryCommandBufferAbstract for it, so that it can actually be used for queue submissions. I would like to have this done for the 0.33 release of Vulkano. This will also deprecate UnsafeCommandBuffer.
• Implement synchronization validation for all the functions, using the state tracking that was previously implemented.
• Replace UnsafeCommandBufferBuilder with CommandBufferBuilder as the inner type of SyncCommandBufferBuilder.
• Deprecate PrimaryAutoCommandBuffer, SecondaryAutoCommandBuffer and SyncCommandBuffer, and use PrimaryCommandBuffer/SecondaryCommandBuffer as the only built command buffer types.
• Merge AutoCommandBufferBuilder and SyncCommandBufferBuilder, with the methods of the latter becoming _unchecked versions of the former.
• Make AutoCommandBufferBuilder re-use the validation methods of CommandBufferBuilder.
• Using the now-finished new command buffer stuff, start adding state tracking and validation to Queue.

[LeonMatthes]

Taking inspiration from the validation layers makes a lot of sense :)

Alright, if there are any concrete issues apart from CommandBuffers, which you're already working on, let me know. I'll keep an eye out for new issues marked as "easy" or "good first issue"