GPUOpen-Archive / Anvil

Anvil is a cross-platform framework for Vulkan
MIT License
594 stars 62 forks source link

Why does MemoryBlock::create_derived require parent memory to not be mapped? #76

Closed Silverlan closed 6 years ago

Silverlan commented 6 years ago

I have a parent buffer with a large chunk of memory. Since I need to make frequent changes to it, I keep the entire buffer permanently mapped. However, I also want to split this buffer into many smaller derived sub-buffers. Since the parent buffer is already mapped at this point, this will trigger an assertion failure in _MemoryBlock::createderived:

Anvil::MemoryBlockUniquePtr Anvil::MemoryBlock::create_derived(MemoryBlock* in_parent_memory_block_ptr,
                                                               VkDeviceSize in_start_offset,
                                                               VkDeviceSize in_size)
{
    MemoryBlockUniquePtr result_ptr(nullptr,
                                    std::default_delete<MemoryBlock>() );

    if (in_parent_memory_block_ptr == nullptr)
    {
        anvil_assert(in_parent_memory_block_ptr != nullptr); // Assertion failure!

        goto end;
    }

    result_ptr.reset(
        new Anvil::MemoryBlock(in_parent_memory_block_ptr,
                               in_start_offset,
                               in_size)
    );

end:
    return result_ptr;
}

The question is, is that assert really necessary? Writing to (or mapping) a sub-buffer redirects to the parent buffer anyway, so I don't see any harm in the parent buffer being already mapped at the time the sub-buffer is created.

DominikWitczakAMD commented 6 years ago

That assertion check acts more of a safe-guard against use cases we currently do not have test coverage for internally. (yes, we actually do have a fairly thorough test coverage for Anvil but it can't be shared for various reasons). I believe it dates back to the old times where buffer chaining used to be a mess and use cases like the one you mention here would basically explode.

If you can confirm everything works fine for you, I'm happy to ditch that assertion check.

DominikWitczakAMD commented 6 years ago

Addressed internally.