From 46402d4ff7acb92d8c6cc3895bbb1887ae203162 Mon Sep 17 00:00:00 2001 From: Sasha Szpakowski Date: Sat, 22 Jun 2024 16:32:10 -0300 Subject: [PATCH 1/2] vulkan: fix missing resource validation in Shader:send --- src/modules/graphics/opengl/Shader.cpp | 1 - src/modules/graphics/vulkan/Shader.cpp | 66 +++++++++++++++++++++----- src/modules/graphics/vulkan/Vulkan.cpp | 2 +- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/modules/graphics/opengl/Shader.cpp b/src/modules/graphics/opengl/Shader.cpp index 99568ebfc..42e4828b0 100644 --- a/src/modules/graphics/opengl/Shader.cpp +++ b/src/modules/graphics/opengl/Shader.cpp @@ -655,7 +655,6 @@ void Shader::sendBuffers(const UniformInfo *info, love::graphics::Buffer **buffe count = std::min(count, info->count); - // Bind the textures to the texture units. for (int i = 0; i < count; i++) { love::graphics::Buffer *buffer = buffers[i]; diff --git a/src/modules/graphics/vulkan/Shader.cpp b/src/modules/graphics/vulkan/Shader.cpp index 54be10bd5..57671e210 100644 --- a/src/modules/graphics/vulkan/Shader.cpp +++ b/src/modules/graphics/vulkan/Shader.cpp @@ -362,39 +362,80 @@ void Shader::updateUniform(const UniformInfo *info, int count) void Shader::sendTextures(const UniformInfo *info, graphics::Texture **textures, int count) { + bool issampler = info->baseType == UNIFORM_SAMPLER; + bool isstoragetex = info->baseType == UNIFORM_STORAGETEXTURE; + + count = std::min(count, info->count); + if (current == this) Graphics::flushBatchedDrawsGlobal(); for (int i = 0; i < count; i++) { + love::graphics::Texture *tex = textures[i]; + bool isdefault = tex == nullptr; + + if (tex != nullptr) + { + if (!validateTexture(info, tex, false)) + continue; + } + else + { + auto gfx = Module::getInstance(Module::M_GRAPHICS); + tex = gfx->getDefaultTexture(info->textureType, info->dataBaseType, info->isDepthSampler); + } + int resourceindex = info->resourceIndex + i; auto prevtexture = activeTextures[resourceindex]; - activeTextures[resourceindex] = textures[i]; + activeTextures[resourceindex] = tex; activeTextures[resourceindex]->retain(); if (prevtexture) prevtexture->release(); - if (textures[i] != prevtexture) - setTextureDescriptor(info, textures[i], i); + if (tex != prevtexture) + setTextureDescriptor(info, (isdefault && (info->access & ACCESS_WRITE) != 0) ? nullptr : tex, i); } } void Shader::sendBuffers(const UniformInfo *info, love::graphics::Buffer **buffers, int count) { + bool texelbinding = info->baseType == UNIFORM_TEXELBUFFER; + bool storagebinding = info->baseType == UNIFORM_STORAGEBUFFER; + + count = std::min(count, info->count); + if (current == this) Graphics::flushBatchedDrawsGlobal(); for (int i = 0; i < count; i++) { + love::graphics::Buffer *buffer = buffers[i]; + bool isdefault = buffer == nullptr; + + if (buffer != nullptr) + { + if (!validateBuffer(info, buffer, false)) + continue; + } + else + { + auto gfx = Module::getInstance(Module::M_GRAPHICS); + if (texelbinding) + buffer = gfx->getDefaultTexelBuffer(info->dataBaseType); + else + buffer = gfx->getDefaultStorageBuffer(); + } + int resourceindex = info->resourceIndex + i; auto prevbuffer = activeBuffers[resourceindex]; - activeBuffers[resourceindex] = buffers[i]; + activeBuffers[resourceindex] = buffer; activeBuffers[resourceindex]->retain(); if (prevbuffer) prevbuffer->release(); - if (buffers[i] != prevbuffer) - setBufferDescriptor(info, buffers[i], i); + if (buffer != prevbuffer) + setBufferDescriptor(info, (isdefault && (info->access & ACCESS_WRITE) != 0) ? nullptr : buffer, i); } } @@ -1070,7 +1111,8 @@ void Shader::setMainTex(graphics::Texture *texture) if (u != nullptr) { auto prevtexture = activeTextures[u->resourceIndex]; - texture->retain(); + if (texture != nullptr) + texture->retain(); if (prevtexture) prevtexture->release(); activeTextures[u->resourceIndex] = texture; @@ -1088,8 +1130,8 @@ void Shader::setTextureDescriptor(const UniformInfo *info, love::graphics::Textu // Samplers may change after this call, so they're set just before the // descriptor set is used instead of here. - imageInfo.imageLayout = vkTexture->getImageLayout(); - imageInfo.imageView = (VkImageView)vkTexture->getRenderTargetHandle(); + imageInfo.imageLayout = vkTexture != nullptr ? vkTexture->getImageLayout() : VK_IMAGE_LAYOUT_UNDEFINED; + imageInfo.imageView = vkTexture != nullptr ? (VkImageView)vkTexture->getRenderTargetHandle() : VK_NULL_HANDLE; resourceDescriptorsDirty = true; } @@ -1099,13 +1141,13 @@ void Shader::setBufferDescriptor(const UniformInfo *info, love::graphics::Buffer if (info->baseType == UNIFORM_STORAGEBUFFER) { VkDescriptorBufferInfo &bufferInfo = descriptorBuffers[info->bindingStartIndex + index]; - bufferInfo.buffer = (VkBuffer)buffer->getHandle(); + bufferInfo.buffer = buffer != nullptr ? (VkBuffer)buffer->getHandle() : VK_NULL_HANDLE; bufferInfo.offset = 0; - bufferInfo.range = buffer->getSize(); + bufferInfo.range = buffer != nullptr ? buffer->getSize() : 0; } else if (info->baseType == UNIFORM_TEXELBUFFER) { - descriptorBufferViews[info->bindingStartIndex + index] = (VkBufferView)buffer->getTexelBufferHandle(); + descriptorBufferViews[info->bindingStartIndex + index] = buffer != nullptr ? (VkBufferView)buffer->getTexelBufferHandle() : VK_NULL_HANDLE; } resourceDescriptorsDirty = true; diff --git a/src/modules/graphics/vulkan/Vulkan.cpp b/src/modules/graphics/vulkan/Vulkan.cpp index dea2523a7..7a6a4275e 100644 --- a/src/modules/graphics/vulkan/Vulkan.cpp +++ b/src/modules/graphics/vulkan/Vulkan.cpp @@ -772,7 +772,7 @@ VkDescriptorType Vulkan::getDescriptorType(graphics::Shader::UniformType type) case graphics::Shader::UniformType::UNIFORM_INT: case graphics::Shader::UniformType::UNIFORM_UINT: case graphics::Shader::UniformType::UNIFORM_BOOL: - return VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + return VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; case graphics::Shader::UniformType::UNIFORM_SAMPLER: return VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; case graphics::Shader::UniformType::UNIFORM_STORAGETEXTURE: From f548430f06b59f182dacd713ed5af845f688a902 Mon Sep 17 00:00:00 2001 From: Sasha Szpakowski Date: Sat, 22 Jun 2024 16:32:29 -0300 Subject: [PATCH 2/2] remove more obsolete code --- src/modules/graphics/Shader.cpp | 11 +++++------ src/modules/graphics/Shader.h | 3 --- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/modules/graphics/Shader.cpp b/src/modules/graphics/Shader.cpp index 14fb6f63d..0cdb96068 100644 --- a/src/modules/graphics/Shader.cpp +++ b/src/modules/graphics/Shader.cpp @@ -1678,12 +1678,11 @@ static StringMap languages(language static StringMap::Entry builtinNameEntries[] = { - { "MainTex", Shader::BUILTIN_TEXTURE_MAIN }, - { "love_VideoYChannel", Shader::BUILTIN_TEXTURE_VIDEO_Y }, - { "love_VideoCbChannel", Shader::BUILTIN_TEXTURE_VIDEO_CB }, - { "love_VideoCrChannel", Shader::BUILTIN_TEXTURE_VIDEO_CR }, - { "love_UniformsPerDraw", Shader::BUILTIN_UNIFORMS_PER_DRAW }, - { "love_UniformsPerDraw2", Shader::BUILTIN_UNIFORMS_PER_DRAW_2 }, + { "MainTex", Shader::BUILTIN_TEXTURE_MAIN }, + { "love_VideoYChannel", Shader::BUILTIN_TEXTURE_VIDEO_Y }, + { "love_VideoCbChannel", Shader::BUILTIN_TEXTURE_VIDEO_CB }, + { "love_VideoCrChannel", Shader::BUILTIN_TEXTURE_VIDEO_CR }, + { "love_UniformsPerDraw", Shader::BUILTIN_UNIFORMS_PER_DRAW }, }; static StringMap builtinNames(builtinNameEntries, sizeof(builtinNameEntries)); diff --git a/src/modules/graphics/Shader.h b/src/modules/graphics/Shader.h index 7eaff628c..3fcddcbd8 100644 --- a/src/modules/graphics/Shader.h +++ b/src/modules/graphics/Shader.h @@ -64,7 +64,6 @@ public: BUILTIN_TEXTURE_VIDEO_CB, BUILTIN_TEXTURE_VIDEO_CR, BUILTIN_UNIFORMS_PER_DRAW, - BUILTIN_UNIFORMS_PER_DRAW_2, BUILTIN_MAX_ENUM }; @@ -188,8 +187,6 @@ public: Vector4 scaleParams; Vector4 clipSpaceParams; Colorf constantColor; - - // Pixel shader-centric variables past this point. Vector4 screenSizeParams; };