Unify all image types

[marc0246]

It is here, the prophecy foretold long ago, of the image unification. Things went a bit too smoothly, though I did make some decisions while consolidating features of the different image types that undermine some previous functionality, which are as follows.

Constructors from iter/buffer

• ImmutableImage::from_iter

  Constructing an image from data on the host will always require a staging buffer, unless the image is useless linear. Hiding that this is going on is not helping anyone, quite the opposite. Creating a new staging buffer for each upload of an image for instance, when you have thousands, is insanity.

• ImmutableImage::from_buffer

  This is less tame, since there is no hidden buffer allocation. Yet it is still obstructing what is going on under the hood for no good reason. The image isn't created from the buffer. The function creates an uninitialized image. It's really awkward in my eyes, but worse still, recording a copy command is one line of code! This function has no utility.

Exporting memory

• {AttachmentImage, StorageImage}::new_with_exportable_fd

  My issue with this is first and foremost that it is platform-specific, and if we indulged things like this we would very quickly have an explosion of constructors. Instead, I think whether or not memory is exportable is something that should be handled by the memory allocator or when allocating DeviceMemory manually.

  What one would do now is:

  1. Configure StandardMemoryAllocator to allocate memory with the desired external handle types: https://github.com/marc0246/vulkano/blob/bd68a6f601ec3e718ab3eb04f865d4d9fe27d6f4/examples/src/bin/gl-interop.rs#L576-L597
  2. Create an Image with the desired external handle types, using the regular Image::new constructor: https://github.com/marc0246/vulkano/blob/bd68a6f601ec3e718ab3eb04f865d4d9fe27d6f4/examples/src/bin/gl-interop.rs#L117-L137
     Using MemoryAllocatePreference::AlwaysAllocate, one can also ensure the image gets its own separate allocation.

  Alternatively one could allocate DeviceMemory with the desired external handle types, create a MemoryAlloc out of the DeviceMemory, and bind it to a RawImage.

• {AttachmentImage, StorageImage}::export_posix_fd

  Exporting memory is not specific to images either and therefore this functionality should be present on DeviceMemory only IMO.

• {AttachmentImage, StorageImage}::mem_size

  For suballocated images, having this be a method on the image itself doesn't make much sense. Instead one can get the allocation of the image and then the size of the DeviceMemory from that, albeit a bit more clunky.

Since @hakolao originally added these, hopefully he can chime in and tell me whether this sounds adequate. Also tagging @fayalalebrun because they added the GL interop example, so maybe they have a perspective on this as well.

Importing memory

This only concerns StorageImage::new_from_dma_buf_fd.

Most important issues I see with this are same as above, namely that this is platform-specific and importing memory is not specific to images, and so encouraging these constructors could quickly spiral out of control. Instead I would like to err on the side of caution and request that the user imports memory using DeviceMemory directly. That said I wouldn't be completely opposed to the idea of adding platform-specific constructors to Image and Buffer for memory-importing, if such a shortcut is truly wanted, but it would add a lot to the maintenance burden especially with the amount of external handle types that there are.

What one would do now is:

1. Use DeviceMemory::import together with the file descriptor(s) to import the memory.
2. Create a RawImage with the desired explicit DRM format modifier(s).
3. Create MemoryAlloc(s) from the DeviceMemory and bind them to the RawImage.

Since @DavidR86 added this constructor, hopefully they can give me their opinion on the matter.

Fixes #1988, fixes #1854, fixes #1446, fixes #1154, fixes #715, fixes #108.

Changelog:
Remove:

### Breaking changes
Changes to images:
- Removed the `ImageInner` type. The `inner` method of the `ImageAccess` trait now returns a reference to the inner image directly.
- Removed the `descriptor_layouts` method on the `ImageAccess` trait. All images by default now use the `General` layout for storage image descriptors, and `ShaderReadOnlyOptimal` for all other image descriptors.

Add:

### Breaking changes
Changes to images:
- There is now only the single type `Image` to represent images. `ImageAccess`, `ImageInner`, `AttachmentImage`, `ImmutableImage`, `StorageImage` and `SwapchainImage` were removed.
- `ImageView` no longer has a type parameter, `ImageViewAbstract` was removed.
- Removed `ImageAccessFromUndefinedLayout`, `ImmutableImageCreationError`, `ImmutableImageInitialization` and `MipmapsCount`.
- `Image` was moved to the `image` module.

[hakolao]

I think what you said about the exportable memory for images sounds good 👍🏻

[Rua]

Do I understand correctly that, if the allocator is configured with external memory, then all buffers and images using it will also use external memory? It should probably be made clear in this case that you should use multiple allocators, one for normal resources and one for exportable ones. Having every single resource be exportable doesn't seem like a good idea.

[marc0246]

Do I understand correctly that, if the allocator is configured with external memory, then all buffers and images using it will also use external memory?

Each memory block allocated from a memory type that has external handle types configured will be allocated with those external handle types. Whether or not your resources are exportable depends on whether the resources were created with external handle types as well. Ideally those blocks could house both regular resources and ones that are exported, but of course since the spec is so anal about this, in practice you have to put each such resource in its own block of DeviceMemory. At least until that mess of a VU is resolved.

[marc0246]

@Rua what do you think about me leaving out some of the previous constructors? Do you think my reasoning is justified? I wouldn't be completely opposed to adding those back if they actually serve a purpose.

[marc0246]

Also I forgot to mention the myriad of other constructors that AttachmentImage previously had, as well as StorageImage::general_purpose_image_view, but I think that speaks for itself...

[Rua]

Given how little those constructors actually did, I'm not opposed. For buffers though (which you haven't touched here), it may be a bit annoying for the user to have to calculate the size manually. A possible source of errors.

[marc0246]

When would one need to calculate the size manually?

[Rua]

The gl-interop example is giving me a validation error with this PR, that I don't get with the latest master:

VUID-VkBindBufferMemoryInfo-memory-02726(ERROR / SPEC): msgNum: -1616898404 - Validation Error: [ VUID-VkBindBufferMemoryInfo-memory-02726 ] Object 0: handle = 0xcb3ee80000000007, type = VK_OBJECT_TYPE_BUFFER; Object 1: handle = 0xead9370000000008, type = VK_OBJECT_TYPE_DEVICE_MEMORY; | MessageID = 0x9fa0169c | vkBindBufferMemory2() pBindInfos[0]: The VkDeviceMemory (VkDeviceMemory 0xead9370000000008[]) has an external handleType of VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT which does not include at least one handle from VkBuffer (VkBuffer 0xcb3ee80000000007[]) handleType VkExternalMemoryHandleTypeFlags(0). The Vulkan spec states: If the value of VkExportMemoryAllocateInfo::handleTypes used to allocate memory is not 0, it must include at least one of the handles set in VkExternalMemoryBufferCreateInfo::handleTypes when buffer was created (https://vulkan.lunarg.com/doc/view/1.3.250.1/linux/1.3-extensions/vkspec.html#VUID-VkBindBufferMemoryInfo-memory-02726)

Oddly it's complaining about something related to buffers rather than images.

[marc0246]

No that's what I would expect actually, I forgot a buffer is allocated as well. I'll just make the example allocate DeviceMemory directly for the image instead of using the allocator, as is the best bet.

[marc0246]

Fixed.

[Rua]

Thanks, there's no more validation error now. Was it caused by the example code misusing unsafe code, or is it a bug in Vulkano itself?

[marc0246]

It's a bug in vulkano, because we're using bind_memory_unchecked. I noticed that, but that's for a future PR of mine. It's an issue with buffers as well. We're also missing validation for VUID-VkExportMemoryAllocateInfo-handleTypes-00656 when binding memory for both buffers and images.

[marc0246]

One thing that we could do so that the user doesn't have to bind memory manually is to always bind a separate block of DeviceMemory when the resource has external handle types. Since that's pretty much the only viable thing to do anyway, it seems like a sensible thing to do in Image::new and Buffer::new? Pretty much what StorageImage::new_with_exportable_fd and DeviceLocalBuffer::raw_with_exportable_fd were doing before, except that there would be no additional constructors, and that it would work with any combination of external handle types.