gfx-rs / gfx

[maintenance mode] A low-overhead Vulkan-like GPU API for Rust.
http://gfx-rs.github.io/
Apache License 2.0
5.36k stars 547 forks source link

RFC: Clone-ability Guidelines #2457

Open kvark opened 6 years ago

kvark commented 6 years ago

This is an RFC for #2454, also related to #2452

There has been a lot of discussion about gfx-hal types having move semantics. More often than not, it stands in a way of working with the API. The type system integration that the move semantics provides isn't helping much because of the overall level unsafety we expose. The reason for types to not being Clone at the API level is because it allows the backends to use non-clonable data in the implementation. In practice, dealing with native API often involves some sort of sharing: Vulkan handles are just copyable machine words, D3D12 has ComPtr, and Metal's objects are refcounted internally. At the end of the day, if none of the backends require the move semantics for a particular type, there is one less reason to not have Clone on it. One particularly annoying aspect of the move semantics is that it doesn't play well with Rust's drop(&mut self). Thus, if a structure owns some graphics objects, and it wants to clean up after itself, it needs to resort to some run-time owning tricks like Option or ManuallyDrop.

This aspect of the API affects both gfx-hal and wgpu-rs, and it's very important to get right.

Proposed Guideline

Enable Clone on objects (at the API level) if all the conditions are met:

  1. All the backend implementations can (or already) do Clone
  2. Unique borrowing doesn't make sense or is not used

In order to see how this would be applied, let's split all graphics objects in the following groups:

Note: this is a proposal! comments are welcome.

zakarumych commented 6 years ago

Command buffer is only clonable type and you want to make it nonclonnable and other types clonnable...

kvark commented 6 years ago

Command buffers being clon-eable is a very unfortunate artifact, as I've been complaining about it since the beginning. Why do you want them to be clone-able?

zakarumych commented 6 years ago

Because you want them in at least two places. Near command pool to reuse next time. Near queue to submit when all command buffers for submission are ready. Maybe even store submission objects and resubmit them each frame after recording.

bbrown683 commented 6 years ago

Being able to clone synchronizers would allow something like below without using std::iter::repeat_with.

let mut acquire_semaphores = 
        iter::repeat(device.borrow().get_device().create_semaphore().unwrap())
    .take(image_count as _)
    .collect();

I think a lot of other types could benefit from similar initialization.

zakarumych commented 6 years ago

@bbrown683 Cloning resource gives you new value that represent the very same resource. So it would be invalid to use cloned semaphore for synchronizing multiple image acquisition.