copyMemoryToImage function added by CrabExtra · Pull Request #1079 · Devsh-Graphics-Programming/Nabla

CrabExtra · 2026-05-26T17:37:28Z

Add hostImageCopy as MOVE_TO_LIMIT in device_features.json.
Query and enable VkPhysicalDeviceHostImageCopyFeaturesEXT through limits.hostImageCopy.
Add EUF_HOST_TRANSFER_BIT and map it to VK_IMAGE_USAGE_HOST_TRANSFER_BIT_EXT.
Wire VK_FORMAT_FEATURE_2_HOST_IMAGE_TRANSFER_BIT_EXT into hostImageTransfer format usage.
Add host image copy flags and SMemoryToImageCopy.
Add ILogicalDevice::copyMemoryToImage validation and CVulkanLogicalDevice backend.
Validate host-transfer image creation and EHICF_MEMCPY_BIT full-subresource rules.

Description

Testing

TODO list:

- Add hostImageCopy as MOVE_TO_LIMIT in device_features.json. - Query and enable VkPhysicalDeviceHostImageCopyFeaturesEXT through limits.hostImageCopy. - Add EUF_HOST_TRANSFER_BIT and map it to VK_IMAGE_USAGE_HOST_TRANSFER_BIT_EXT. - Wire VK_FORMAT_FEATURE_2_HOST_IMAGE_TRANSFER_BIT_EXT into hostImageTransfer format usage. - Add host image copy flags and SMemoryToImageCopy. - Add ILogicalDevice::copyMemoryToImage validation and CVulkanLogicalDevice backend. - Validate host-transfer image creation and EHICF_MEMCPY_BIT full-subresource rules.

devshgraphicsprogramming · 2026-05-27T14:05:34Z

+        //Not sure to remove this?
+        //        retval.hostImageTransfer = anyFlag(features, VK_FORMAT_FEATURE_2_HOST_IMAGE_TRANSFER_BIT);
+        retval.hostImageTransfer = anyFlag(features, VK_FORMAT_FEATURE_2_HOST_IMAGE_TRANSFER_BIT_EXT);


we need to report because not every format suports

…ImageCopyProperties`, including `identicalMemoryTypeRequirements`, `copySrcLayouts`, and `copyDstLayouts`. - Updated `ILogicalDevice` to validate source and destination image layouts for host image copy operations in `copyMemoryToImage`, `copyImageToMemory`, `copyImageToImage`, and `transitionImageLayout`. - Extended `device_limits.json` to include new fields for `hostImageCopySrcLayouts`, `hostImageCopyDstLayouts`, and `identicalMemoryTypeRequirements`.

Erfan-Ahmadi · 2026-06-15T06:20:39Z

+        {
+            "comment": [
+                "VK_EXT_host_image_copy",
+                "HostImageCopyPropertiesEXT"            
+            ],
+             "entries": [
+                {
+                    "type": "uint16_t",
+                    "name": "hostImageCopySrcLayouts",
+                    "value": 0
+                },
+                {
+                    "type": "uint16_t",
+                    "name": "hostImageCopyDstLayouts",
+                    "value": 0
+                },
+                {
+                    "type": "bool",
+                    "name": "identicalMemoryTypeRequirements",
+                    "value": false
+                }
+            ]
+        },


Ok so this is wrong:
it's not a "limit" per se, or an extension/feature.
limits are either vulkan limits or vulkan features promoted to limits.
this fits neither. it's a physical device property.
Similar to SMemoryProperties you should have a SHostImageCopyProperties and function to query it, and member functions to fill.
it should be equivalent to https://docs.vulkan.org/refpages/latest/refpages/source/VkPhysicalDeviceHostImageCopyProperties.html

Look at how these work:

SMemoryProperties

SFormatImageUsages

SQueueFamilyProperties

or basically anything in SInitData

Erfan-Ahmadi · 2026-06-15T06:21:06Z

+            /* VK_EXT_host_image_copy */
+            uint8_t optimalTilingLayoutUUID[VK_UUID_SIZE];


nope, should be in dedicate SHostImageCopyProperties

Erfan-Ahmadi · 2026-06-15T06:29:21Z

+            layouts.copySrcLayoutCount = hostImageCopyProperties.copySrcLayoutCount;
+            layouts.pCopySrcLayouts = copySrcLayouts.data();
+            layouts.copyDstLayoutCount = hostImageCopyProperties.copyDstLayoutCount;
+            layouts.pCopyDstLayouts = copyDstLayouts.data();


bug,
vectors go out of scope and you're holding onto stale pointer.
if you fix this previous issue first you'll realize why we used smart_refctd_dynamic_array for these types of things. it's basically a fixed sized dynamic allocation (no resize like vector + consistent pointers). it's also refcounted to ensure the memory doesn't get freed.

ok I mistunderstood, It's ok.
but try to use smart_refctd_dynamic_array

Erfan-Ahmadi · 2026-06-15T06:29:50Z

            properties.limits.minAccelerationStructureScratchOffsetAlignment = accelerationStructureProperties.minAccelerationStructureScratchOffsetAlignment;
        }

+        if (isExtensionSupported(VK_EXT_HOST_IMAGE_COPY_EXTENSION_NAME))


this whole thing needs to fill initData's host image copy properties
(don't forget getters for this property)

Erfan-Ahmadi · 2026-06-15T06:35:31Z

+    if (regions.size() > 0xffff'ffffull)
+    {
+        NBL_LOG_ERROR("`regions.size()` exceeds `uint32_t` range");
+        return false;
+    }


ugh, I feel like this is the super protective stuff that AI generates, cool but we'd be having to do it everywhere if it was a real issue

Erfan-Ahmadi · 2026-06-15T06:37:13Z

+    if (regions.empty())
+    {
+        NBL_LOG_ERROR("`regions` must not be empty");
+        return false;
+    }


why return false?
user wants to copy 0 regions? done, you're good to go :D

move that to the first check

Erfan-Ahmadi · 2026-06-15T06:50:16Z

+            for (uint32_t i=0u; i<=static_cast<uint32_t>(layout_t::SHARED_PRESENT); i++)
+            {
+                const VkImageLayout vkLayout = getVkImageLayoutFromImageLayout(static_cast<layout_t>(i));
+                for (const VkImageLayout l : copySrcLayouts)
+                    if (l==vkLayout) { properties.limits.hostImageCopySrcLayouts |= 1u<<i; break; }
+                for (const VkImageLayout l : copyDstLayouts)
+                    if (l==vkLayout) { properties.limits.hostImageCopyDstLayouts |= 1u<<i; break; }
+            }


ok I see what you're doing, instead of an array like vulkan's, you're creating a bitflag. that's actually a good idea for faster checks.

This double for loop seems weird af, you're iterating on all allowed src/dst layouts, per layout.

Why not do this instead:
loop over all copy[Src/Dst]Layouts, getImageLayout from Vk layout. check whether it's != UNDEFINED, and then put it in the correct place in the bitfield uint64_t seems good enough for the bitfield, we won't be dealing with 64+ layouts.

If there is no getImageLayout from Vk layout, implement it

also good to assert that whatever you're doing with the bitshifts are less than the size of the storage. (e.g. i < 64)

btw be careful about UNDEFINED because it is 0 in the enum and 1 << 0 is 1.
so we should never see the first bit set. because that would indicate host image copy is valid to/from undefined layout?

Erfan-Ahmadi · 2026-06-15T06:52:05Z

+            VkPhysicalDeviceProperties2 layouts2 = { VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2,&layouts };
+            vkGetPhysicalDeviceProperties2(vk_physicalDevice,&layouts2);
+            using layout_t = asset::IImage::LAYOUT;
+            for (uint32_t i=0u; i<=static_cast<uint32_t>(layout_t::SHARED_PRESENT); i++)


very risky, what if someone added a new layout to image layout, then SHARED_PRESENT wouldn't be the last item.
but anyways, this whole thing could be done with a single for loop

Erfan-Ahmadi · 2026-06-15T07:05:16Z

+    bool validLayout = false;
+    switch (dstImageLayout)
+    {
+        case IGPUImage::LAYOUT::GENERAL:
+        case IGPUImage::LAYOUT::READ_ONLY_OPTIMAL:
+        case IGPUImage::LAYOUT::ATTACHMENT_OPTIMAL:
+        case IGPUImage::LAYOUT::TRANSFER_SRC_OPTIMAL:
+        case IGPUImage::LAYOUT::TRANSFER_DST_OPTIMAL:
+        case IGPUImage::LAYOUT::PRESENT_SRC:
+        case IGPUImage::LAYOUT::SHARED_PRESENT:
+            validLayout = true;
+            break;
+        default:
+            break;
+    }
+    if (!validLayout)
+    {
+        NBL_LOG_ERROR("Invalid `dstImageLayout`");
+        return false;
+    }


vulkan has to make this validation because their flags are simple integers. we have explicit type for LAYOUT.
Just check if it's != UNDEFINED ? no need for switch case

Erfan-Ahmadi · 2026-06-15T07:08:40Z

+    if ((flags.value & ~static_cast<decltype(flags.value)>(IGPUImage::EHICF_MEMCPY_BIT)) != 0u)
+    {
+        NBL_LOG_ERROR("Invalid host image copy flags");
+        return false;
+    }


again, no need for this, vulkan has to do these types of validations, cause their flags are just uint32_t's

typedef uint32_t VkFlags; typedef VkFlags VkFormatFeatureFlags;

you don't have to make this validation. we have explicit enum class type for host image copy. and we shouldn't be resposnible for user doing anything weird with it. (we just translate this to vulkan flag and pass it down)
if vulkan gives validation error about this, we know it's us who did the translation incorrectly and passed incorrect flags

Erfan-Ahmadi · 2026-06-15T07:20:04Z

+        return false;
+    }
+    // device `pCopyDstLayouts` membership: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkCopyMemoryToImageInfo-dstImageLayout-09060
+    if (!(getPhysicalDevice()->getLimits().hostImageCopyDstLayouts & (1u<<static_cast<uint32_t>(dstImageLayout))))


good check against bitflag, but it needs to change to physdev->gethostimagecopyproperties

devshgraphicsprogramming reviewed May 27, 2026

View reviewed changes

Comment thread include/nbl/video/ILogicalDevice.h

devshgraphicsprogramming reviewed May 27, 2026

View reviewed changes

Comment thread src/nbl/video/utilities/CAssetConverter.cpp

CrabExtra added 5 commits June 9, 2026 01:49

Add transitionImageLayout function

129964d

one small dublicated bug fix

641821d

Add Vulkan image copy and layout transition support

4d8e7e5

Added optimalTilingLayoutUUID to IPhysicalDevice

e0ea817

Erfan-Ahmadi reviewed Jun 15, 2026

View reviewed changes

		/* VK_EXT_host_image_copy */
		uint8_t optimalTilingLayoutUUID[VK_UUID_SIZE];

Conversation

CrabExtra commented May 26, 2026

Description

Testing

TODO list:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Erfan-Ahmadi Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Erfan-Ahmadi Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Erfan-Ahmadi Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Erfan-Ahmadi Jun 15, 2026 •

edited

Loading

Erfan-Ahmadi Jun 15, 2026 •

edited

Loading

Erfan-Ahmadi Jun 15, 2026 •

edited

Loading