Add support for VK_EXT_host_image_copy extension.

[billhollings]

• Add support for VK_EXT_host_image_copy extension.
• MVKResource::getHostMemoryAddress() return nullptr if MVKDeviceMemory::getHostMemoryAddress() returns nullptr, regardless of local offset.
• Remove unnecessary enum value kMVKVkFormatFeatureFlagsTexTransfer to reduce redundancy between read and transfer feature flag options
• MVKResource remove unnecessary inline qualifiers (unrelated).
• MVKDevice remove some obsolete commentary (unrelated).

Fixes #2197.

[ovvldc]

Why include unrelated changes in a PR, why not separate them in a housekeeping PR or such? Doesn't that make the commit history difficult to trace, or I am missing something?

[billhollings]

Why include unrelated changes in a PR, why not separate them in a housekeeping PR or such? Doesn't that make the commit history difficult to trace, or I am missing something?

A fair question.

To help keep the code tidy over the long run, we try to have a "clean as you go" approach, when it comes to non-functional maintenance, since those kind of maintenance projects never seem to get prioritized.

I've removed one of the "unrelated" notes, as it actually was functional as part of this change.

[cdavis5e]

To help keep the code tidy over the long run, we try to have a "clean as you go" approach, when it comes to non-functional maintenance, since those kind of maintenance projects never seem to get prioritized.

Fair point. Coming from Wine, however, which is extremely strict about this, I actually prefer to at least split the housekeeping changes into their own commits. That way, they show up separately in the history, which makes bisection easier. The PR could still include the main change and the unrelated cleanups together.

[billhollings]

That way, they show up separately in the history, which makes bisection easier. The PR could still include the main change and the unrelated cleanups together.

This makes sense. I'll try to remember that approach on future PR's.

[billhollings]

You forgot to add this to the list of all supported extensions in the User's Guide.

D'oh! Good catch. Fixed.

Also, FYI, it looks like GitHub doesn't recognize the keyword "completes" as indicating that an issue should be closed after a PR is merged. You may have to close the issue manually.

Yeah. I noticed that after the fact, too. I've modified it above now.

I'm also worried about the implications of the VK_HOST_IMAGE_COPY_MEMCPY_EXT flag. I don't think we can support that for nonlinear images. Managed textures on Intel are where this would crop up.

I don't know why it didn't make it into the spec proper, but the extension proposal includes:

...flags may be VK_HOST_IMAGE_COPY_MEMCPY_EXT, in which case the data in host memory should have the same swizzling layout as the image. This is mainly useful for embedded systems where this swizzling is known and well defined outside of Vulkan.

This leads me to interpret VK_HOST_IMAGE_COPY_MEMCPY_EXT as a nice-to-have optimization, but that falling back to a linear extraction for both linear and optimal layouts (basically ignoring VK_HOST_IMAGE_COPY_MEMCPY_EXT) is acceptable.

The CTS tests all seem to test both with and without VK_HOST_IMAGE_COPY_MEMCPY_EXT, and so far so good. However, I have not tested on Intel GPU's. Why do you feel any issues would be limited to that platform?

[cdavis5e]

The CTS tests all seem to test both with and without VK_HOST_IMAGE_COPY_MEMCPY_EXT, and so far so good. However, I have not tested on Intel GPU's. Why do you feel any issues would be limited to that platform?

Because on Intel Macs, the GPUs support Managed textures which are host-accessible but do not have to be linear. I suppose you could get around that by putting the texture in a placement heap and aliasing it with a buffer, but I'm not sure Metal will let you create a Managed MTLHeap... I suppose we could also forbid setting VK_IMAGE_USAGE_HOST_TRANSFER_BIT_EXT with Managed textures, which we expose as Vulkan HOST_CACHED memory, but then we'd lose out on being able to use VK_EXT_host_image_copy with these images.

[billhollings]

Maybe I should run the CTS against this change. I have a couple of Intel Macs.

If you could, that would be helpful. Without an understanding of what may/will go wrong, it's hard to develop and test a solution for how to handle it.

I've been testing using...

cd <mvk-parent-dir>/MoltenVK/Scripts
./runcts <cts-parent-dir>/VK-GL-CTS/external/vulkancts/mustpass/main/vk-default/image/host-image-copy.txt

[billhollings]

Maybe I should run the CTS against this change. I have a couple of Intel Macs.

If you could, that would be helpful.

NM. I've dug out my old Intel/Radeon and have managed to get things running on it.

The integrated Intel GPU is fine, but it's definitely misbehaving on the discrete Radeon GPU, as you predicted.

Looking into it.

[billhollings]

The changes so far LGTM, but I won't approve this until you've answered my objections about VK_HOST_IMAGE_COPY_MEMCPY_EXT with Managed textures on Intel Macs. Maybe I should run the CTS against this change. I have a couple of Intel Macs.

If you could, that would be helpful.

NM. I've dug out my old Intel/Radeon and have managed to get things running on it.

The integrated Intel GPU is fine, but it's definitely misbehaving on the discrete Radeon GPU, as you predicted.

Looking into it.

This is now working properly on both Radeon & Intel.

It turns out the failures were not a linear/optimal issue, but a managed-memory texture sync issue. I've added a synchronizeTexture: where appropriate for managed-memory textures on discrete GPUs.