Strange VkBuffer adress

[laurent7601]

Hi, I create a vertex buffer an I copy it into a std::vector.

He calls the destructor of my vertex buffer class on this function :

void RenderTarget::createUniformBuffers() {

            VkDeviceSize bufferSize = sizeof(UniformBufferObject);
            std::vector<VkBuffer> ubos;
            std::vector<VkDeviceMemory> ubosMemory;
            ubos.resize(getMaxFramesInFlight());
            ubosMemory.resize(getMaxFramesInFlight());

            for (size_t i = 0; i < getMaxFramesInFlight(); i++) {
                createBuffer(bufferSize, VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, ubos[i], ubosMemory[i]);
            }
            uniformBuffers.push_back(ubos);
            uniformBuffersMemory.push_back(ubosMemory);
            VertexBuffer vb(vkDevice);
            vertexBuffers.push_back(vb);
        }

But it tells me that the adress of the vertex buffer is invalid in the vkDestroy function or I don’t create the vertex buffer because I only create the vertex buffer in the update function of my vertex buffer class so it should be null but he’s not null but he have a strange adress :

VertexBuffer::VertexBuffer(window::Device& vkDevice) : vkDevice(vkDevice), vertexBufferCreated(false), indexBufferCreated(false),
            needToUpdateVertexBuffer(false), needToUpdateIndexBuffer(false) {
                createCommandPool();
            }
            void VertexBuffer::clear() {
                m_vertices.clear();
            }
            void VertexBuffer::clearIndexes() {
                indices.clear();
            }
            void VertexBuffer::addIndex(uint16_t index) {
                indices.push_back(index);
                needToUpdateIndexBuffer = true;
            }
            void VertexBuffer::append(const Vertex& vertex) {
                m_vertices.push_back(vertex);
                needToUpdateVertexBuffer = true;
            }
            void VertexBuffer::createBuffer(VkDeviceSize size, VkBufferUsageFlags usage, VkMemoryPropertyFlags properties, VkBuffer& buffer, VkDeviceMemory& bufferMemory) {
                VkBufferCreateInfo bufferInfo{};
                bufferInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO;
                bufferInfo.size = size;
                bufferInfo.usage = usage;
                bufferInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE;

                if (vkCreateBuffer(vkDevice.getDevice(), &bufferInfo, nullptr, &buffer) != VK_SUCCESS) {
                    throw std::runtime_error("failed to create buffer!");
                }

                VkMemoryRequirements memRequirements;
                vkGetBufferMemoryRequirements(vkDevice.getDevice(), buffer, &memRequirements);

                VkMemoryAllocateInfo allocInfo{};
                allocInfo.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO;
                allocInfo.allocationSize = memRequirements.size;
                allocInfo.memoryTypeIndex = findMemoryType(memRequirements.memoryTypeBits, properties);

                if (vkAllocateMemory(vkDevice.getDevice(), &allocInfo, nullptr, &bufferMemory) != VK_SUCCESS) {
                    throw std::runtime_error("failed to allocate buffer memory!");
                }

                vkBindBufferMemory(vkDevice.getDevice(), buffer, bufferMemory, 0);
            }
            void VertexBuffer::copyBuffer(VkBuffer srcBuffer, VkBuffer dstBuffer, VkDeviceSize size) {
                VkCommandBufferAllocateInfo allocInfo{};
                allocInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO;
                allocInfo.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY;
                allocInfo.commandPool = commandPool;
                allocInfo.commandBufferCount = 1;


                VkCommandBuffer commandBuffer;
                vkAllocateCommandBuffers(vkDevice.getDevice(), &allocInfo, &commandBuffer);

                VkCommandBufferBeginInfo beginInfo{};
                beginInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
                beginInfo.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;

                vkBeginCommandBuffer(commandBuffer, &beginInfo);

                    VkBufferCopy copyRegion{};
                    copyRegion.size = size;
                    vkCmdCopyBuffer(commandBuffer, srcBuffer, dstBuffer, 1, &copyRegion);

                vkEndCommandBuffer(commandBuffer);

                VkSubmitInfo submitInfo{};
                submitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
                submitInfo.commandBufferCount = 1;
                submitInfo.pCommandBuffers = &commandBuffer;

                vkQueueSubmit(vkDevice.getGraphicsQueue(), 1, &submitInfo, VK_NULL_HANDLE);
                vkQueueWaitIdle(vkDevice.getGraphicsQueue());

                vkFreeCommandBuffers(vkDevice.getDevice(), commandPool, 1, &commandBuffer);
            }
            uint32_t VertexBuffer::findMemoryType(uint32_t typeFilter, VkMemoryPropertyFlags properties) {
                VkPhysicalDeviceMemoryProperties memProperties;
                vkGetPhysicalDeviceMemoryProperties(vkDevice.getPhysicalDevice(), &memProperties);
                for (uint32_t i = 0; i < memProperties.memoryTypeCount; i++) {
                    if ((typeFilter & (1 << i)) && (memProperties.memoryTypes[i].propertyFlags & properties) == properties) {
                        return i;
                    }
                }
                throw std::runtime_error("aucun type de memoire ne satisfait le buffer!");
            }
            VkBuffer VertexBuffer::getVertexBuffer() {
                return vertexBuffer;
            }
            size_t VertexBuffer::getSize() {
                return m_vertices.size();
            }
            size_t VertexBuffer::getVertexCount() {
                return m_vertices.size();
            }
            size_t VertexBuffer::getIndicesSize() {
                return indices.size();
            }
            VkBuffer VertexBuffer::getIndexBuffer() {
                return indexBuffer;
            }
            ////////////////////////////////////////////////////////////
            void VertexBuffer::setPrimitiveType(PrimitiveType type)
            {
                m_primitiveType = type;
            }


            ////////////////////////////////////////////////////////////
            PrimitiveType VertexBuffer::getPrimitiveType() const
            {
                return m_primitiveType;
            }
            void VertexBuffer::update() {
                if (needToUpdateVertexBuffer) {
                    VkDeviceSize bufferSize = sizeof(Vertex) * m_vertices.size();

                    VkBuffer stagingBuffer;
                    VkDeviceMemory stagingBufferMemory;
                    createBuffer(bufferSize, VK_BUFFER_USAGE_TRANSFER_SRC_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, stagingBuffer, stagingBufferMemory);

                    void* data;
                    vkMapMemory(vkDevice.getDevice(), stagingBufferMemory, 0, bufferSize, 0, &data);
                        memcpy(data, m_vertices.data(), (size_t) bufferSize);
                    vkUnmapMemory(vkDevice.getDevice(), stagingBufferMemory);

                    createBuffer(bufferSize, VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_VERTEX_BUFFER_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, vertexBuffer, vertexBufferMemory);

                    copyBuffer(stagingBuffer, vertexBuffer, bufferSize);

                    vkDestroyBuffer(vkDevice.getDevice(), stagingBuffer, nullptr);
                    vkFreeMemory(vkDevice.getDevice(), stagingBufferMemory, nullptr);
                    vertexBufferCreated = true;
                    needToUpdateVertexBuffer = false;
                }
                if (needToUpdateIndexBuffer) {
                    VkDeviceSize bufferSize = sizeof(indices[0]) * indices.size();

                    VkBuffer stagingBuffer;
                    VkDeviceMemory stagingBufferMemory;
                    createBuffer(bufferSize, VK_BUFFER_USAGE_TRANSFER_SRC_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT, stagingBuffer, stagingBufferMemory);

                    void* data;
                    vkMapMemory(vkDevice.getDevice(), stagingBufferMemory, 0, bufferSize, 0, &data);
                    memcpy(data, indices.data(), (size_t) bufferSize);
                    vkUnmapMemory(vkDevice.getDevice(), stagingBufferMemory);

                    createBuffer(bufferSize, VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_INDEX_BUFFER_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, indexBuffer, indexBufferMemory);

                    copyBuffer(stagingBuffer, indexBuffer, bufferSize);

                    vkDestroyBuffer(vkDevice.getDevice(), stagingBuffer, nullptr);
                    vkFreeMemory(vkDevice.getDevice(), stagingBufferMemory, nullptr);
                    indexBufferCreated = true;
                    needToUpdateIndexBuffer  = false;
                }
            }
            Vertex& VertexBuffer::operator [](unsigned int index)
            {
                return m_vertices[index];
            }
            void VertexBuffer::createCommandPool() {

                window::Device::QueueFamilyIndices queueFamilyIndices = vkDevice.findQueueFamilies(vkDevice.getPhysicalDevice(), VK_NULL_HANDLE);

                VkCommandPoolCreateInfo poolInfo{};
                poolInfo.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO;
                poolInfo.queueFamilyIndex = queueFamilyIndices.graphicsFamily.value();
                poolInfo.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT; // Optionel
                if (vkCreateCommandPool(vkDevice.getDevice(), &poolInfo, nullptr, &commandPool) != VK_SUCCESS) {
                    throw core::Erreur(0, "échec de la création d'une command pool!", 1);
                }
            }
            VertexBuffer::~VertexBuffer() {
                /*if (indexBufferCreated) {
                    vkDestroyBuffer(vkDevice.getDevice(), indexBuffer, nullptr);
                    vkFreeMemory(vkDevice.getDevice(), indexBufferMemory, nullptr);
                }
                if (vertexBufferCreated) {
                    vkDestroyBuffer(vkDevice.getDevice(), vertexBuffer, nullptr);
                    vkFreeMemory(vkDevice.getDevice(), vertexBufferMemory, nullptr);
                }*/
            }

Even if I initialize the vkBuffer to null he’s not null in the destructor…

Why ?

Tanks.

[laurent7601]

Ok I’ve found what’s wrong, the std::vector class make copies so I needed to add a copy constructor in my VertexBuffer class and recreate the vulkan vertex buffers otherwise I have several vertex buffers pointing to the same VkBuffer :

VertexBuffer::VertexBuffer(const VertexBuffer& vb) : vkDevice(vb.vkDevice) {
                m_vertices = vb.m_vertices;
                indices = vb.indices;
                needToUpdateVertexBuffer = (m_vertices.size() > 0) ? true : false;
                needToUpdateIndexBuffer = (indices.size() > 0) ? true : false;
                update();
            }

Solved.

[carsten_neumann]

Please read and understand this description of common pitfalls when applying RAII principles to OpenGL objects - the same applies to Vulkan.
In fact, for Vulkan the issue is more severe: many Vulkan objects have lifetime requirements that are outside of the lifetime of your C++ wrapper objects. For example, you must not destroy a VkBuffer while the GPU is processing commands that refer to that buffer. In other words, you now have a situation where the destructor of your wrapper object must not destroy the underlying Vulkan resource until some synchronization mechanism says it is safe to do so (e.g. the VkFence passed to a submit is signaled).

[laurent7601]

Ok and if we recreate a buffer, the old one is not destroyed…, so we must destroy the buffer and than create it again. I didn’t know about that so I was searching for hours why I had the message that a buffer was not destroyed.

And when the buffers are in use, this is even worth…, we need a synchronisation mechanism like you said…
I’ve found a solution using push bind descriptors for datas I update at every frame.

[Alfonse_Reinheart]

Or you can just follow good C++ practices and make the buffer type move-only. When you move a buffer, the new object adopts the resources of the old one and leaves it empty. So when the old one gets destroyed, it doesn’t have a Vulkan buffer to destroy. Delete the copy constructor and you’re fine.

[laurent7601]

Yes but I have a vertex buffer class, which destroy the vulkan buffer in the desctructor. When I move an object of my vertex buffer class into a std::vector by example, the push_back function copies or moves my vertex buffer objects and destroys the old one. And if the old buffer is still used in a command buffer I’ve an error. So I had to create an std::vector class which pointers to my vertex buffer class so buffer is not copied or moved and no buffers are destoryed by the destructor of my vertex buffer class until I call the operator delete in the destructor of my render target class I solved this problem like this.

Sometimes moving buffers may cause issues, or I need a sychronisation mechanim maybe but it can slow down the appli if I’ve to wait. for a fence…

[Alfonse_Reinheart]

@laurent7601

Yes but I have a vertex buffer class, which destroy the vulkan buffer in the desctructor. When I move an object of my vertex buffer class into a std::vector by example, the push_back function copies or moves my vertex buffer objects and destroys the old one. And if the old buffer is still used in a command buffer I’ve an error.

I am well aware of the problem. But making the type non-copyable and implementing a proper move constructor solves the problem.

If the type is not copyable, it cannot be copied and therefore it cannot have more than one instance that refers to the same object. The move constructor should null out the pointers on the original object so that, if it is deleted, it won’t delete the Vulkan buffer since it doesn’t point to it anymore.

This is basic C++11 stuff here; it’s one of the main reasons move constructors were added to the language.

I don’t know what types all of your VertexBuffer members are, but here’s a first crack at a move constructor:

VertexBuffer::VertexBuffer(const VertexBuffer& vb) = delete;

VertexBuffer::VertexBuffer(VertexBuffer &&vb)
  : vkDevice(std::exchange(vb.vkDevice, nullptr))
  , buffer(std::exchange(buffer, nullptr))
  , m_vertices(std::move(vb.m_vertices) //Is this a vector?
  , indices(std::exchange(vb.indices, nullptr) //Is this a pointer or integer?
  , needToUpdateVertexBuffer((m_vertices.size() > 0) ? true : false)
  , needToUpdateIndexBuffer((indices.size() > 0) ? true : false)
{
  update();
}

Again, I don’t know all of your members or what they are, but this is generally what it would look like. Note the use of std::exchange instead of copying the buffer pointer.

Also, you really should be consistent on your use of reverse-Polish notation. Either all of your members should use m_, or none of them should.

[laurent7601]

Ok thanks I didn’t implemented the move constructor correctly, I didn’t set the moved buffer to nullptr…

Thanks!

[laurent7601]

I tried your code but the problem is that vulkan buffers are read only, your code :

VertexBuffer::VertexBuffer(const VertexBuffer&& vb) : vkDevice(vb.vkDevice)
            , vertexBuffer(std::exchange(vb.vertexBuffer, nullptr))
            , indexBuffer(std::exchange(vb.indexBuffer, nullptr))
            , m_vertices(std::move(vb.m_vertices))
            , indices(std::move(vb.indices))
            , needToUpdateVertexBuffer((m_vertices.size() > 0) ? true : false)
            , needToUpdateIndexBuffer((indices.size() > 0) ? true : false) {
                createCommandPool();
                update();
            }

gives me this compile time error :

[ 38%] Building CXX object src/odfaeg/Graphics/CMakeFiles/odfaeg-graphics.dir/vertexBuffer.cpp.obj
In file included from C:/mingw64/include/c++/14.2.0/bits/exception_ptr.h:41,
                 from C:/mingw64/include/c++/14.2.0/exception:166,
                 from C:/mingw64/include/c++/14.2.0/ios:41,
                 from C:/mingw64/include/c++/14.2.0/ostream:40,
                 from C:/mingw64/include/c++/14.2.0/iostream:41,
                 from C:/Users/Laurent et Christian/ODFAEG-master/ODFAEG/include/odfaeg/Math/vec4.h:3,
                 from C:/Users/Laurent et Christian/ODFAEG-master/ODFAEG/include/odfaeg/Graphics/vertex.h:31,
                 from C:/Users/Laurent et Christian/ODFAEG-master/ODFAEG/include/odfaeg/Graphics/vertexBuffer.hpp:8,
                 from C:\Users\Laurent et Christian\ODFAEG-master\ODFAEG\src\odfaeg\Graphics\vertexBuffer.cpp:2:
C:/mingw64/include/c++/14.2.0/bits/move.h: In instantiation of 'constexpr _Tp std::__exchange(_Tp&, _Up&&) [with _Tp = VkBuffer_T* const; _Up = std::nullptr_t]':
C:/mingw64/include/c++/14.2.0/utility:110:29:   required from 'constexpr _Tp std::exchange(_Tp&, _Up&&) [with _Tp = VkBuffer_T* const; _Up = std::nullptr_t]'
  110 |     { return std::__exchange(__obj, std::forward<_Up>(__new_val)); }
      |              ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Users\Laurent et Christian\ODFAEG-master\ODFAEG\src\odfaeg\Graphics\vertexBuffer.cpp:27:41:   required from here
   27 |             , vertexBuffer(std::exchange(vb.vertexBuffer, nullptr))
      |                            ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
C:/mingw64/include/c++/14.2.0/bits/move.h:177:13: error: assignment of read-only reference '__obj'
  177 |       __obj = std::forward<_Up>(__new_val);
      |       ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mingw32-make[2]: *** [src\odfaeg\Graphics\CMakeFiles\odfaeg-graphics.dir\build.make:469: src/odfaeg/Graphics/CMakeFiles/odfaeg-graphics.dir/vertexBuffer.cpp.obj] Error 1
mingw32-make[1]: *** [CMakeFiles\Makefile2:417: src/odfaeg/Graphics/CMakeFiles/odfaeg-graphics.dir/all] Error 2
mingw32-make: *** [makefile:135: all] Error 2

But I don’t think it’ll work this error seems to be logical : if you set a buffer which is used (I mean which is bound with vkCmd) to nullptr it’ll not work.

I think the only way to solve this, is to set a boolean when the buffer is moved, and if this boolean is true we don’t destroy the buffer because it’s the same buffer it’s just the owner of the buffer which change.

EDIT : Ha ok the error was because I declared const the VertexBuffer in the move constructor, I removed the const keyword and now it compiles.

[laurent7601]

Ok it works no more errors messages with that.
Thanks!

[laurent7601]

Strange, now I have errors messages. (invalid buffer while recording commands)
I guess this is because move set the old buffer invalid.

So I had to use a pointer to remove all these error messages.

[Alfonse_Reinheart]

@laurent7601

I guess this is because move set the old buffer invalid.

You shouldn’t use an object after it has been moved-from.

You really should take some time to learn modern C++ before getting into something complex like Vulkan.

[laurent7601]

You shouldn’t use an object after it has been moved-from.

You mean I shoudn’t use it in a command buffer when it was moved ?
I haven’t any choise, I need to draw everything at once, and then submit all the work…, because begin erase the command buffer.

And I can’t use several submits (one submit per draw) otherwise I have errors I can’t separate graphics queue submit and presentation.

In this tutoriel he submit the graphics queue only once and after he do the presentation :

https://vulkan-tutorial.com/Drawing_a_triangle/Drawing/Frames_in_flight
I tried to separate the graphics queue submit from the presentation but it didn’t worked, I had a bunch of errors.
And there’s an error in the tutoriel, you have to create a command buffer per swapchain image and not per frame in flight otherwise I have the windows how’s flickering like if a swapchain image is empty…

[Alfonse_Reinheart]

@laurent7601

You mean I shoudn’t use it in a command buffer when it was moved ?

You have a C++ object. You move from that C++ object to a different C++ object. The previous C++ object should be destroyed, not used for any purpose.

Moving is not copying. If you move all of your stuff from one house to another, and you want to sleep in your bed, you’ll have to go to the current house, not your previous one. Same idea.

I haven’t any choise, I need to draw everything at once, and then submit all the work…, because begin erase the command buffer.

… I have no idea what that has to do with ownership of your vertex buffer class object. Your C++ vertex buffer class should not be formally owned by your C++ command buffer class. You can pass a reference to the object to a command buffer member function for binding purposes or whatever, but it shouldn’t maintain ownership of it.

So how your command buffer works should have nothing to do with how you maintain ownership of your vertex buffer.

[laurent7601]

Yes but we can only set fences for submitting and not for command recording so if I bind the verte xbuffer to the command buffer and then I draw another vertex buffer vulkan tells me the first vertex buffer is still in use by the command buffer. And I don’t know how to solve this, I don’t thing using one command buffer per draw call’ll solve this.

I don’t know how to tell to vulkan that I don’t want my vertexbuffer is still used by the command buffer recording when I move it…and I don’t thing vulkan have this feature…

[Alfonse_Reinheart]

@laurent7601

if I bind the verte xbuffer to the command buffer and then I draw another vertex buffer vulkan tells me the first vertex buffer is still in use by the command buffer.

You are doing something in that sequence of operations that you are not describing which is causing an error that you are misattributing to something else. If you were doing only the sequence of commands you describe, it would work. There is no prohibition on using multiple vertex buffers in the same CB.

Your code is buggy.

@laurent7601

I don’t thing using one command buffer per draw call’ll solve this.

Yet that’s what everyone else does.

[laurent7601]

Ok there is a bug somewhere but I have still to find where…

[carsten_neumann]

@Alfonse_Reinheart

@laurent7601

I don’t thing using one command buffer per draw call’ll solve this.

Yet that’s what everyone else does.

Maybe I’m misreading this, but I don’t think having separate command buffers for each draw call is a common approach? I would include as many draw calls in a single command buffer as I reasonably can (especially if the draw calls are related, e.g. using the same pipeline and/or resources).
I don’t want to derail the thread from the resource ownership issues that seem to be (part of) the problem, but it sounded like advice opposite to what I (falsely?) thought was common practice.

[Alfonse_Reinheart]

@carsten_neumann

Maybe I’m misreading this, but I don’t think having separate command buffers for each draw call is a common approach?

Sorry; I misread the comment I was replying to. Using multiple draw calls in the same CB the thing that “everyone else does”.

[Alfonse_Reinheart]

One way to do that is to just remove your code from the equation. There are tools that can just spit out the sequence of Vulkan calls your program is making. This shows you exactly what you’re telling the Vulkan API and in what order, without any regard for how your code generated those functions.

This should make it relatively easy to see where you’re misusing the API. Tracking down how those function calls got into that order requires looking at your codebase, but I’m sure you know how to debug that.