[PATCH v2 2/2] vkd3d: Allocate one large buffer for a heap and offset into it.

[Henri Verbeet]

On Tue, 8 Oct 2019 at 12:00, Hans-Kristian Arntzen
<post at arntzen-software.no> wrote:
> @@ -4076,6 +4076,7 @@ static void d3d12_command_list_set_root_cbv(struct d3d12_command_list *list,
>
>      resource = vkd3d_gpu_va_allocator_dereference(&list->device->gpu_va_allocator, gpu_address);
>      buffer_info.buffer = resource->u.vk_buffer;
> +
>      buffer_info.offset = gpu_address - resource->gpu_address;
>      buffer_info.range = resource->desc.Width - buffer_info.offset;
>      buffer_info.range = min(buffer_info.range, vk_info->device_limits.maxUniformBufferRange);
Stray whitespace change.

What I'm guessing happened here is that you changed the code to
account for resource->heap_offset, but then realised it's always 0 for
anything that has its own VA. That's convenient here, but also a
little annoying to have to reason about; I'd probably prefer to
consistently apply the heap offset anywhere we use a buffer offset.
Worse, I think there are a few places that do need the heap offset
applied, but don't in this patch. E.g.,
d3d12_command_list_CopyTextureRegion(),
d3d12_command_list_ResourceBarrier(),
d3d12_command_list_ClearUnorderedAccessViewUint().

> @@ -1334,6 +1334,8 @@ static HRESULT vkd3d_init_device_caps(struct d3d12_device *device,
>      device->feature_options.CrossAdapterRowMajorTextureSupported = FALSE;
>      /* SPV_EXT_shader_viewport_index_layer */
>      device->feature_options.VPAndRTArrayIndexFromAnyShaderFeedingRasterizerSupportedWithoutGSEmulation = FALSE;
> +
> +    /* FIXME: Does this actually work on NV which has 64k bufferImage alignment quirks with VkDeviceMemory? */
>      device->feature_options.ResourceHeapTier = D3D12_RESOURCE_HEAP_TIER_2;
Well, does it?

> @@ -546,6 +557,10 @@ static HRESULT d3d12_heap_init(struct d3d12_heap *heap,
>      VkDeviceSize vk_memory_size;
>      HRESULT hr;
>      int rc;
> +    bool buffers_allowed;
> +    D3D12_RESOURCE_DESC resource_desc;
> +    D3D12_RESOURCE_STATES initial_resource_state;
> +    const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs;
"vk_procs" is unused.

> @@ -1003,13 +1078,16 @@ static void d3d12_resource_destroy(struct d3d12_resource *resource, struct d3d12
>      if (resource->flags & VKD3D_RESOURCE_EXTERNAL)
>          return;
>
> -    if (resource->gpu_address)
> -        vkd3d_gpu_va_allocator_free(&device->gpu_va_allocator, resource->gpu_address);
> +    if (!(resource->flags & VKD3D_RESOURCE_PLACED_BUFFER))
> +    {
> +        if (resource->gpu_address)
> +            vkd3d_gpu_va_allocator_free(&device->gpu_va_allocator, resource->gpu_address);
>
> -    if (d3d12_resource_is_buffer(resource))
> -        VK_CALL(vkDestroyBuffer(device->vk_device, resource->u.vk_buffer, NULL));
> -    else
> -        VK_CALL(vkDestroyImage(device->vk_device, resource->u.vk_image, NULL));
> +        if (d3d12_resource_is_buffer(resource))
> +            VK_CALL(vkDestroyBuffer(device->vk_device, resource->u.vk_buffer, NULL));
> +        else
> +            VK_CALL(vkDestroyImage(device->vk_device, resource->u.vk_image, NULL));
> +    }
>
>      if (resource->flags & VKD3D_RESOURCE_DEDICATED_HEAP)
>          d3d12_heap_destroy(resource->heap);
Do we really need VKD3D_RESOURCE_PLACED_BUFFER? It seems largely
equivalent to "!(resource->flags & VKD3D_RESOURCE_DEDICATED_HEAP) &&
d3d12_resource_is_buffer(resource)".