vulkano icon indicating copy to clipboard operation
vulkano copied to clipboard

`ImmutableImage` isn't actually immutable

Open Rua opened this issue 2 years ago • 2 comments

ImmutableImage is immutable in the sense that the user can't submit commands that modify the image contents. But there is one other kind of operation that is not considered in this: layout transitions. A layout transition is a write operation, and while ImmutableImage will normally be in the ShaderReadOnlyOptimal layout, doing a copy_image with the image as the source will transition it to TransferSrcOptimal. For the user this is invisible, but this matters a lot for synchronisation within the GPU, because such a write operation can't happen while another queue is accessing the image. This could create unexpected errors for the user when submitting command buffers, because the user thinks the image isn't ever written and thus concurrent access is possible, when it actually is written to, just invisibly to the user.

I have been thinking for a while about whether the immutable buffer and image types are even really needed in Vulkano. I suppose they signal the intent of the user, that they never intend to ever write to the buffer or image again, but to call it "immutable" is also misleading as demonstrated above. There may be a speed argument to be made, since the synchronisation for immutable resources is a little simpler, but that needs to be tested by comparing performance with the mutable resource variants.

Rua avatar Mar 04 '22 16:03 Rua

Here are some ideas or thoughts, that could be unsound or not valid, but overall I think it might be a good idea to generalize images into a single type. This would mean that users would have to create the own staging buffers unless we provide helper methods to do that.

  • All layout transitions are done manually.
    • Images could be initialized with an initial layout.
    • Command buffers would provide a method to transition the image.
    • All command would check the layout and error out if it isn't in a valid layout.
  • Create a ImageLayoutUsage type that would handle transitions automatically.
    • Would be supplied at image creation.
    • Any method that requires a transition to be made requires the image to be used exclusively.
    • This could maybe be automatically derived from the ImageUsage?
    • A general use image that would require only one transition.
      • Used anywhere, but slow.
      ImageLayoutUsage {
          initial: ImageLayout::Undefined,
          transfer_src: ImageLayout::General,
          transfer_dst: ImageLayout::General,
          attachment: ImageLayout::General,
          descriptor: ImageLayout::General,
      }
      
    • A more optimized version that would require more transitions.
      • Used for sampled images & input attachments
      ImageLayoutUsage {
          initial: ImageLayout::Undefined,
          transfer_src: ImageLayout::TransferSrcOptimal,
          transfer_dst: ImageLayout::TransferDstOptimal,
          attachment: ImageLayout::ShaderReadOnlyOptimal,
          descriptor: ImageLayout::ShaderReadOnlyOptimal,
      }
      
    • A transient attachment image
      ImageLayoutUsage {
          initial: ImageLayout::ColorAttachmentOptimal,
          transfer_src: ImageLayout::None, // Not allowed
          transfer_dst: ImageLayout::None, // Not allowed
          attachment: ImageLayout::ColorAttachmentOptimal,
          descriptor: ImageLayout::None, // Not allowed
      }
      

AustinJ235 avatar Mar 04 '22 23:03 AustinJ235

Your second point already exists, it's the descriptor_layouts method on the ImageAccess trait. It doesn't contain all the items you mentioned, but the basic idea is there. I've been thinking that it would be nice if the user could specify these layouts themselves when creating an image, but that would require some extra code to check that the specified layouts are valid for each use and for the image.

Rua avatar Mar 05 '22 12:03 Rua