fix: VkTexture move/destroy ownsSampler_ flag, extract finalizeSampler

- Fix move constructor and move assignment: set other.ownsSampler_ to
  false after transfer (was incorrectly set to true, leaving moved-from
  object claiming ownership of a null sampler)
- Fix destroy(): reset ownsSampler_ to false after clearing sampler
  handle (was set to true, inconsistent with null handle state)
- Extract finalizeSampler() from 3 duplicated cache-or-create blocks
  in createSampler() overloads and createShadowSampler() (-24 lines)
- Add SPIR-V alignment why-comment in vk_shader.cpp
This commit is contained in:
Kelsi 2026-03-30 14:24:41 -07:00
parent 28e5cd9281
commit 683e171fd1
3 changed files with 27 additions and 51 deletions

View file

@ -68,6 +68,8 @@ public:
private: private:
void generateMipmaps(VkContext& ctx, VkFormat format, uint32_t width, uint32_t height); void generateMipmaps(VkContext& ctx, VkFormat format, uint32_t width, uint32_t height);
// Shared sampler finalization: prefer the global cache, fall back to direct creation
bool finalizeSampler(VkDevice device, const VkSamplerCreateInfo& samplerInfo);
AllocatedImage image_{}; AllocatedImage image_{};
VkSampler sampler_ = VK_NULL_HANDLE; VkSampler sampler_ = VK_NULL_HANDLE;

View file

@ -32,6 +32,7 @@ bool VkShaderModule::loadFromFile(VkDevice device, const std::string& path) {
} }
size_t fileSize = static_cast<size_t>(file.tellg()); size_t fileSize = static_cast<size_t>(file.tellg());
// SPIR-V is a stream of 32-bit words — file size must be a multiple of 4
if (fileSize == 0 || fileSize % 4 != 0) { if (fileSize == 0 || fileSize % 4 != 0) {
LOG_ERROR("Invalid SPIR-V file size (", fileSize, "): ", path); LOG_ERROR("Invalid SPIR-V file size (", fileSize, "): ", path);
return false; return false;

View file

@ -17,7 +17,8 @@ VkTexture::VkTexture(VkTexture&& other) noexcept
ownsSampler_(other.ownsSampler_) { ownsSampler_(other.ownsSampler_) {
other.image_ = {}; other.image_ = {};
other.sampler_ = VK_NULL_HANDLE; other.sampler_ = VK_NULL_HANDLE;
other.ownsSampler_ = true; // Source no longer owns the sampler — ownership transferred to this instance
other.ownsSampler_ = false;
} }
VkTexture& VkTexture::operator=(VkTexture&& other) noexcept { VkTexture& VkTexture::operator=(VkTexture&& other) noexcept {
@ -28,7 +29,7 @@ VkTexture& VkTexture::operator=(VkTexture&& other) noexcept {
ownsSampler_ = other.ownsSampler_; ownsSampler_ = other.ownsSampler_;
other.image_ = {}; other.image_ = {};
other.sampler_ = VK_NULL_HANDLE; other.sampler_ = VK_NULL_HANDLE;
other.ownsSampler_ = true; other.ownsSampler_ = false;
} }
return *this; return *this;
} }
@ -196,6 +197,23 @@ bool VkTexture::createDepth(VkContext& ctx, uint32_t width, uint32_t height, VkF
return true; return true;
} }
// Shared sampler finalization: try the global cache first (avoids duplicate Vulkan
// sampler objects), fall back to direct creation if no VkContext is available.
bool VkTexture::finalizeSampler(VkDevice device, const VkSamplerCreateInfo& samplerInfo) {
auto* ctx = VkContext::globalInstance();
if (ctx) {
sampler_ = ctx->getOrCreateSampler(samplerInfo);
ownsSampler_ = false;
return sampler_ != VK_NULL_HANDLE;
}
if (vkCreateSampler(device, &samplerInfo, nullptr, &sampler_) != VK_SUCCESS) {
LOG_ERROR("Failed to create texture sampler");
return false;
}
ownsSampler_ = true;
return true;
}
bool VkTexture::createSampler(VkDevice device, bool VkTexture::createSampler(VkDevice device,
VkFilter minFilter, VkFilter magFilter, VkFilter minFilter, VkFilter magFilter,
VkSamplerAddressMode addressMode, float maxAnisotropy) VkSamplerAddressMode addressMode, float maxAnisotropy)
@ -217,22 +235,7 @@ bool VkTexture::createSampler(VkDevice device,
samplerInfo.mipLodBias = 0.0f; samplerInfo.mipLodBias = 0.0f;
samplerInfo.minLod = 0.0f; samplerInfo.minLod = 0.0f;
samplerInfo.maxLod = static_cast<float>(mipLevels_); samplerInfo.maxLod = static_cast<float>(mipLevels_);
return finalizeSampler(device, samplerInfo);
// Use sampler cache if VkContext is available.
auto* ctx = VkContext::globalInstance();
if (ctx) {
sampler_ = ctx->getOrCreateSampler(samplerInfo);
ownsSampler_ = false;
return sampler_ != VK_NULL_HANDLE;
}
// Fallback: no VkContext (shouldn't happen in normal use).
if (vkCreateSampler(device, &samplerInfo, nullptr, &sampler_) != VK_SUCCESS) {
LOG_ERROR("Failed to create texture sampler");
return false;
}
ownsSampler_ = true;
return true;
} }
bool VkTexture::createSampler(VkDevice device, bool VkTexture::createSampler(VkDevice device,
@ -258,22 +261,7 @@ bool VkTexture::createSampler(VkDevice device,
samplerInfo.mipLodBias = 0.0f; samplerInfo.mipLodBias = 0.0f;
samplerInfo.minLod = 0.0f; samplerInfo.minLod = 0.0f;
samplerInfo.maxLod = static_cast<float>(mipLevels_); samplerInfo.maxLod = static_cast<float>(mipLevels_);
return finalizeSampler(device, samplerInfo);
// Use sampler cache if VkContext is available.
auto* ctx = VkContext::globalInstance();
if (ctx) {
sampler_ = ctx->getOrCreateSampler(samplerInfo);
ownsSampler_ = false;
return sampler_ != VK_NULL_HANDLE;
}
// Fallback: no VkContext (shouldn't happen in normal use).
if (vkCreateSampler(device, &samplerInfo, nullptr, &sampler_) != VK_SUCCESS) {
LOG_ERROR("Failed to create texture sampler");
return false;
}
ownsSampler_ = true;
return true;
} }
bool VkTexture::createShadowSampler(VkDevice device) { bool VkTexture::createShadowSampler(VkDevice device) {
@ -290,22 +278,7 @@ bool VkTexture::createShadowSampler(VkDevice device) {
samplerInfo.mipmapMode = VK_SAMPLER_MIPMAP_MODE_NEAREST; samplerInfo.mipmapMode = VK_SAMPLER_MIPMAP_MODE_NEAREST;
samplerInfo.minLod = 0.0f; samplerInfo.minLod = 0.0f;
samplerInfo.maxLod = 1.0f; samplerInfo.maxLod = 1.0f;
return finalizeSampler(device, samplerInfo);
// Use sampler cache if VkContext is available.
auto* ctx = VkContext::globalInstance();
if (ctx) {
sampler_ = ctx->getOrCreateSampler(samplerInfo);
ownsSampler_ = false;
return sampler_ != VK_NULL_HANDLE;
}
// Fallback: no VkContext (shouldn't happen in normal use).
if (vkCreateSampler(device, &samplerInfo, nullptr, &sampler_) != VK_SUCCESS) {
LOG_ERROR("Failed to create shadow sampler");
return false;
}
ownsSampler_ = true;
return true;
} }
void VkTexture::destroy(VkDevice device, VmaAllocator allocator) { void VkTexture::destroy(VkDevice device, VmaAllocator allocator) {
@ -313,7 +286,7 @@ void VkTexture::destroy(VkDevice device, VmaAllocator allocator) {
vkDestroySampler(device, sampler_, nullptr); vkDestroySampler(device, sampler_, nullptr);
} }
sampler_ = VK_NULL_HANDLE; sampler_ = VK_NULL_HANDLE;
ownsSampler_ = true; ownsSampler_ = false;
destroyImage(device, allocator, image_); destroyImage(device, allocator, image_);
} }