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

[Henri Verbeet]

On Wed, 23 Oct 2019 at 13:47, Hans-Kristian Arntzen
<post at arntzen-software.no> wrote:
> On 10/22/19 3:04 PM, Henri Verbeet wrote:
> > 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().
>
> Consistently applying heap_offset would be worse though, because it
> would be wrong in the case where placed buffers are not used. When
> looking up VA, you get the buffer for the heap.
>
Would it? For non-placed resources the heap offset should always be 0.
Put a different way, after this patch the invariant is that the
VkBuffer always starts at offset 0 of the underlying VkDeviceMemory
allocation.

> I fixed the copy/clear commands, but I'm a bit concerned the test suite
> did not catch this. I changed the UAV clear test to use placed resources
> at least in a follow-up patch.
>
Support for placed resources is a relatively recent addition to vkd3d,
so most of the tests currently use committed resources. It would be
good to increase the coverage, but that's something to be aware of,
yes.

> > Do we really need VKD3D_RESOURCE_PLACED_BUFFER? It seems largely
> > equivalent to "!(resource->flags & VKD3D_RESOURCE_DEDICATED_HEAP) &&
> > d3d12_resource_is_buffer(resource)".
>
> Yes, there is some confusion who owns the heap if DEDICATED_HEAP is
> used. The heap creates the resource in this case, and not the other way
> around. FWIW, I tried reverting back, but ended up with issues which
> would likely need some awkward flag to work around anyways, so
> PLACED_BUFFER is the best option I think.
>
What is the exact issue there?