I would like to create simple vectors of shaders, pipelines, textures, and other objects used with the Vulkan API, but I am struggling with understanding how to use copy-constructors, move-constructors, copy-assignment, and move-assignment operations.
My program builds a vector of shader structures to pass to a pipeline for creation:
std::vector<vk::Shader> testShaders{
{"vertex.vert", VK_SHADER_STAGE_VERTEX_BIT},
{"vertex.frag", VK_SHADER_STAGE_FRAGMENT_BIT}
};
vk::GraphicsPPL<triangleList> testPPL(testShaders);
The above method returns a validation layer error where the shader modules are VK_NULL_HANDLE. In another question on Stack Overflow, the solution to loading the vectors was to implement the Rule of Three, but I am now struggling with how to do so without triggering other errors. I tried a few attempts to create the copy-assignment operator and copy-constructor utilizing the cpp-reference page, but I can't seem to avoid crashing my program. Here are some of my attempts:
Shader(const Shader& other) // First attempt: copy constructor
{
std::memcpy(this, other.shaderModule, sizeof(other.shaderModule));
}// exited with code -1073741819.
Shader(const Shader& other) // Second attempt
{
vkDestroyShaderModule(GPU::device, shaderModule, nullptr);
//delete[] shaderModule; //With or without, does not change the outcome
std::memcpy(shaderModule, other.shaderModule, sizeof(other.shaderModule));
} // Couldn't find VkShaderModule Object 0xcdcdcdcdcdcdcdcd and VK_NULL_HANDLE
/* Third attempt */
Shader(const Shader& other)
: shaderStage(other.shaderStage){}
Shader& operator=(const Shader& other) // III. copy assignment
{
if (this == &other)
return *this;
VkShaderModule new_shaderModule{};
//sizeof(new_shaderModule) returns 8
//sizeof(other.shaderModule) returns 8
std::memcpy(new_shaderModule, other.shaderModule, sizeof(other.shaderModule));
vkDestroyShaderModule(GPU::device, shaderModule, nullptr); // No difference with or without, neither replacing or adding "delete[] shaderModule"
shaderModule = new_shaderModule;
return *this;
}// VK_NULL_HANDLE error
Shader(const Shader& other) // Fourth attempt - forgot to add this one
{
std::memcpy(this, &other, sizeof(Shader));
}
My shader structure is below:
struct Shader {
VkShaderModule shaderModule;
VkShaderStageFlagBits shaderStage;
Shader(std::string const& filename, VkShaderStageFlagBits stage)
: shaderStage(stage)
{
auto code = readFile(".\\shaders\\" + filename + ".spv"); // Function omitted from post for brevity, please don't hesitate to ask if needed
createShaderModule(code, filename);
}
~Shader() {
vkDestroyShaderModule(GPU::device, shaderModule, nullptr);
// Delete[] shaderModule // With or without, did not change the outcome
}
void createShaderModule(const std::vector<char>& code, const std::string& filename) {
VkShaderModuleCreateInfo createInfo
{ VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO };
createInfo.codeSize = code.size();
createInfo.pCode = reinterpret_cast<const uint32_t*>(code.data());
if (vkCreateShaderModule(GPU::device, &createInfo, nullptr, &shaderModule) != VK_SUCCESS) {
throw std::runtime_error("failed to create " + filename + "!");
}
}
/* My failed attempts here */
}
Just in case, this is how Vulkan defines the vkCreateShaderModule function:
// Provided by VK_VERSION_1_0
VkResult vkCreateShaderModule(
VkDevice device,
const VkShaderModuleCreateInfo* pCreateInfo,
const VkAllocationCallbacks* pAllocator,
VkShaderModule* pShaderModule);
Edit:
Additions to the shader struct based on the recommendations from the comments:
Shader(const Shader& other) = delete;
Shader& operator=(const Shader& other) = delete;
Shader(Shader&& other) noexcept // move constructor
: shaderModule(std::exchange(other.shaderModule, nullptr)), shaderStage(std::exchange(other.shaderStage, nullptr)) {}
Shader& operator=(Shader&& other) noexcept // move assignment
{
std::swap(shaderModule, other.shaderModule);
return *this;
}
and some new errors, yay...
Error C2440
'=': cannot convert from 'nullptr' to '_Ty'
/* path */ \MSVC\14.37.32822\include\utility
Error (active) E0304
no instance of function template "std::construct_at" matches the argument list
/* path */ MSVC\14.37.32822\include\xmemory
Edit 2:
Changing the move definitions:
Shader(const Shader& other) = delete;
Shader& operator=(const Shader& other) = delete;
Shader(Shader&& other) noexcept // move constructor
{
std::swap(*this, other);
// std::exchange(*this, other); // no dice either
}
Shader& operator=(Shader&& other) noexcept // move assignment
{
std::swap(*this, other);
return *this;
}
New error:
Error C2280 'vk::Shader::Shader(const vk::Shader &)'
attempting to reference a deleted function
/* path */\MSVC\14.37.32822\include\xutility
I tried the shaderModule(nullptr) solution in addition to commenting out the two delete lines without a difference to the error.
I can't help but think you are making this more difficult for yourself than it needs to be. "I would like to create simple vectors of shaders, pipelines, textures, and other objects used with the Vulkan API" - is the std::vector really necessary? Notably, the std::vector will move objects around in memory whenever the container is resized.
In the case of constructing a Vulkan graphics pipeline object, what you need is a vector of vkShaderModule, and these can be stored inside a single "Shader" object. And as soon as the pipeline is created, you are free to destroy those shader modules. (unless pipeline re-creation is a thing)
In my own Vulkan library (cannot open-source atm., unfortunately!) I use opaque pointers to represent shader objects, like following:
// shaders.hpp
struct Shader; // opaque handle defined here
Shader* create_shader(std::string_view vert, std::string_view frag);
void destroy_shader(Shader* shader);
std::vector<vkShaderModule> get_shader_modules(Shader* shader);
// shaders.cpp
struct Shader {
// definition here
};
Shader* create_shader(std::string_view vert, std::string_view frag) {
auto shader = new Shader;
// ...
return shader;
}
void destroy_shader(Shader* shader) {
// ...
delete shader;
}
// main.cpp
auto shader = create_shader("vertex.vert", "vertex.frag");
if (shader == nullptr) { /* FAIL */ }
// use shader here...
destroy_shader(shader);
shader = nullptr;
The introduction of object-oriented programming mechanisms has a tendency of over-complicating things, like here where you are struggling with move constructors and memcpy, mostly because memory allocations/frees are usually placed inside constructors/destructors. In some cases this is a good pattern, but it can lead to a lot of unanswered questions.
Notably, if you now want to move the shader handle around, pass it to other functions, etc., you are only moving a pointer. So you can eliminate the extra complexity of remembering to pass by const ref to avoid copies, or verifying that the move/copy constructors/assignment ops are actually called correctly.
Now this may be subjective, but it seems a bit like inventing a very complicated problem in order to solve an easy problem. (except nothing in Vulkan is "easy"). I hope this helps.