diff --git a/Common/GPU/OpenGL/GLFeatures.cpp b/Common/GPU/OpenGL/GLFeatures.cpp index 18aa1bd714..6877dbd76c 100644 --- a/Common/GPU/OpenGL/GLFeatures.cpp +++ b/Common/GPU/OpenGL/GLFeatures.cpp @@ -306,6 +306,13 @@ bool CheckGLExtensions() { // Otherwise, let's trust GL_MAJOR_VERSION. Note that Mali is intentionally not banned here. if (gl_extensions.ver[0] >= 3) { gl_extensions.GLES3 = gl3stubInit(); + if (!gl_extensions.GLES3) { + // We failed to load the ES3 entry points, so we can't use any of them - including + // glGetStringi below, which is one of the ones gl3stubInit() checks for. Drop back + // to 2.0 so nothing downstream keys off the version and calls into a null pointer. + gl_extensions.ver[0] = 2; + gl_extensions.ver[1] = 0; + } } } #else @@ -331,6 +338,9 @@ bool CheckGLExtensions() { g_set_gl_extensions.clear(); for (GLint i = 0; i < numExtensions; ++i) { const char *ext = (const char *)glGetStringi(GL_EXTENSIONS, i); + if (!ext) { + continue; + } g_set_gl_extensions.insert(ext); g_all_gl_extensions += ext; g_all_gl_extensions += " "; diff --git a/Common/GPU/OpenGL/GLSLProgram.cpp b/Common/GPU/OpenGL/GLSLProgram.cpp index c8c93dc0e9..2de61f8b0c 100644 --- a/Common/GPU/OpenGL/GLSLProgram.cpp +++ b/Common/GPU/OpenGL/GLSLProgram.cpp @@ -1,19 +1,12 @@ -#include - #include #include -#include -#include "Common/File/VFS/VFS.h" -#include "Common/File/FileUtil.h" #include "Common/GPU/OpenGL/GLSLProgram.h" #include "Common/Log.h" -static std::set active_programs; - -bool CompileShader(const char *source, GLuint shader, const char *filename, std::string *error_message) { - glShaderSource(shader, 1, &source, NULL); +static bool CompileShader(const char *source, GLuint shader, const char *stageName, std::string *error_message) { + glShaderSource(shader, 1, &source, nullptr); glCompileShader(shader); GLint success; glGetShaderiv(shader, GL_COMPILE_STATUS, &success); @@ -23,9 +16,9 @@ bool CompileShader(const char *source, GLuint shader, const char *filename, std: GLsizei len; glGetShaderInfoLog(shader, MAX_INFO_LOG_SIZE, &len, infoLog); infoLog[len] = '\0'; - ERROR_LOG(Log::G3D, "Error in shader compilation of %s!\n", filename); + ERROR_LOG(Log::G3D, "Error compiling %s shader!\n", stageName); ERROR_LOG(Log::G3D, "Info log: %s\n", infoLog); - ERROR_LOG(Log::G3D, "Shader source:\n%s\n", (const char *)source); + ERROR_LOG(Log::G3D, "Shader source:\n%s\n", source); if (error_message) *error_message = infoLog; return false; @@ -33,104 +26,17 @@ bool CompileShader(const char *source, GLuint shader, const char *filename, std: return true; } -GLSLProgram *glsl_create_source(const char *vshader_src, const char *fshader_src, std::string *error_message) { - GLSLProgram *program = new GLSLProgram(); - program->program_ = 0; - program->vsh_ = 0; - program->fsh_ = 0; - program->vshader_source = vshader_src; - program->fshader_source = fshader_src; - strcpy(program->name, "[srcshader]"); - strcpy(program->vshader_filename, ""); - strcpy(program->fshader_filename, ""); - if (glsl_recompile(program, error_message)) { - active_programs.insert(program); - } else { - ERROR_LOG(Log::G3D, "Failed compiling GLSL program from source strings"); - delete program; - return 0; - } - return program; -} - -// Not wanting to change ReadLocalFile semantics. -// TODO: Use C++11 unique_ptr, remove delete[] -struct AutoCharArrayBuf { - AutoCharArrayBuf(char *buf = nullptr) : buf_(buf) { - } - ~AutoCharArrayBuf() { - delete [] buf_; - buf_ = nullptr; - } - void reset(char *buf) { - delete[] buf_; - buf_ = buf; - } - operator char *() { - return buf_; - } - -private: - char *buf_; -}; - -bool glsl_recompile(GLSLProgram *program, std::string *error_message) { - struct stat vs, fs; - AutoCharArrayBuf vsh_src, fsh_src; - - if (strlen(program->vshader_filename) > 0 && 0 == stat(program->vshader_filename, &vs)) { - program->vshader_mtime = vs.st_mtime; - if (!program->vshader_source) { - size_t sz; - vsh_src.reset((char *)File::ReadLocalFile(Path(program->vshader_filename), &sz)); - } - } else { - program->vshader_mtime = 0; - } - - if (strlen(program->fshader_filename) > 0 && 0 == stat(program->fshader_filename, &fs)) { - program->fshader_mtime = fs.st_mtime; - if (!program->fshader_source) { - size_t sz; - fsh_src.reset((char *)File::ReadLocalFile(Path(program->fshader_filename), &sz)); - } - } else { - program->fshader_mtime = 0; - } - - if (!program->vshader_source && !vsh_src) { - size_t sz; - vsh_src.reset((char *)g_VFS.ReadFile(program->vshader_filename, &sz)); - } - if (!program->vshader_source && !vsh_src) { - ERROR_LOG(Log::G3D, "File missing: %s", program->vshader_filename); - if (error_message) { - *error_message = std::string("File missing: ") + program->vshader_filename; - } - return false; - } - if (!program->fshader_source && !fsh_src) { - size_t sz; - fsh_src.reset((char *)g_VFS.ReadFile(program->fshader_filename, &sz)); - } - if (!program->fshader_source && !fsh_src) { - ERROR_LOG(Log::G3D, "File missing: %s", program->fshader_filename); - if (error_message) { - *error_message = std::string("File missing: ") + program->fshader_filename; - } - return false; - } - +static bool glsl_compile(GLSLProgram *program, const char *vshader_src, const char *fshader_src, std::string *error_message) { GLuint vsh = glCreateShader(GL_VERTEX_SHADER); - const GLchar *vsh_str = program->vshader_source ? program->vshader_source : (const GLchar *)(vsh_src); - if (!CompileShader(vsh_str, vsh, program->vshader_filename, error_message)) { + if (!CompileShader(vshader_src, vsh, "vertex", error_message)) { + glDeleteShader(vsh); return false; } - const GLchar *fsh_str = program->fshader_source ? program->fshader_source : (const GLchar *)(fsh_src); GLuint fsh = glCreateShader(GL_FRAGMENT_SHADER); - if (!CompileShader(fsh_str, fsh, program->fshader_filename, error_message)) { + if (!CompileShader(fshader_src, fsh, "fragment", error_message)) { glDeleteShader(vsh); + glDeleteShader(fsh); return false; } @@ -146,63 +52,47 @@ bool glsl_recompile(GLSLProgram *program, std::string *error_message) { GLint bufLength = 0; glGetProgramiv(prog, GL_INFO_LOG_LENGTH, &bufLength); if (bufLength) { - char* buf = new char[bufLength + 1]; // safety - glGetProgramInfoLog(prog, bufLength, NULL, buf); - INFO_LOG(Log::G3D, "vsh: %i fsh: %i", vsh, fsh); + char *buf = new char[bufLength + 1]; // safety + glGetProgramInfoLog(prog, bufLength, nullptr, buf); ERROR_LOG(Log::G3D, "Could not link shader program (linkstatus=%i):\n %s \n", linkStatus, buf); if (error_message) { *error_message = buf; } - delete [] buf; + delete[] buf; } else { - INFO_LOG(Log::G3D, "vsh: %i fsh: %i", vsh, fsh); ERROR_LOG(Log::G3D, "Could not link shader program (linkstatus=%i). No OpenGL error log was available.", linkStatus); if (error_message) { *error_message = "(no error message available)"; } } + glDeleteProgram(prog); glDeleteShader(vsh); glDeleteShader(fsh); return false; } - // Destroy the old program, if any. - if (program->program_) { - glDeleteProgram(program->program_); - } - program->program_ = prog; program->vsh_ = vsh; program->fsh_ = fsh; program->sampler0 = glGetUniformLocation(program->program_, "sampler0"); - program->sampler1 = glGetUniformLocation(program->program_, "sampler1"); - - program->a_position = glGetAttribLocation(program->program_, "a_position"); - program->a_color = glGetAttribLocation(program->program_, "a_color"); - program->a_normal = glGetAttribLocation(program->program_, "a_normal"); - program->a_texcoord0 = glGetAttribLocation(program->program_, "a_texcoord0"); - program->a_texcoord1 = glGetAttribLocation(program->program_, "a_texcoord1"); - - program->u_worldviewproj = glGetUniformLocation(program->program_, "u_worldviewproj"); - program->u_world = glGetUniformLocation(program->program_, "u_world"); program->u_viewproj = glGetUniformLocation(program->program_, "u_viewproj"); - program->u_fog = glGetUniformLocation(program->program_, "u_fog"); - program->u_sundir = glGetUniformLocation(program->program_, "u_sundir"); - program->u_camerapos = glGetUniformLocation(program->program_, "u_camerapos"); - - //INFO_LOG(Log::G3D, "Shader compilation success: %s %s", - // program->vshader_filename, - // program->fshader_filename); + program->a_position = glGetAttribLocation(program->program_, "a_position"); + program->a_texcoord0 = glGetAttribLocation(program->program_, "a_texcoord0"); return true; } -int glsl_attrib_loc(const GLSLProgram *program, const char *name) { - return glGetAttribLocation(program->program_, name); -} - -int glsl_uniform_loc(const GLSLProgram *program, const char *name) { - return glGetUniformLocation(program->program_, name); +GLSLProgram *glsl_create_source(const char *vshader_src, const char *fshader_src, std::string *error_message) { + GLSLProgram *program = new GLSLProgram(); + program->program_ = 0; + program->vsh_ = 0; + program->fsh_ = 0; + if (!glsl_compile(program, vshader_src, fshader_src, error_message)) { + ERROR_LOG(Log::G3D, "Failed compiling GLSL program from source strings"); + delete program; + return nullptr; + } + return program; } void glsl_destroy(GLSLProgram *program) { @@ -210,28 +100,19 @@ void glsl_destroy(GLSLProgram *program) { glDeleteShader(program->vsh_); glDeleteShader(program->fsh_); glDeleteProgram(program->program_); - active_programs.erase(program); } else { ERROR_LOG(Log::G3D, "Deleting null GLSL program!"); } delete program; } -static const GLSLProgram *curProgram; - void glsl_bind(const GLSLProgram *program) { if (program) glUseProgram(program->program_); else glUseProgram(0); - curProgram = program; } void glsl_unbind() { glUseProgram(0); - curProgram = nullptr; -} - -const GLSLProgram *glsl_get_program() { - return curProgram; } diff --git a/Common/GPU/OpenGL/GLSLProgram.h b/Common/GPU/OpenGL/GLSLProgram.h index fff73a957b..9b1cd8754e 100644 --- a/Common/GPU/OpenGL/GLSLProgram.h +++ b/Common/GPU/OpenGL/GLSLProgram.h @@ -1,40 +1,24 @@ // Utility code for loading GLSL shaders. -// Has support for auto-reload, see glsl_refresh +// +// This is a small leftover from the old "native" library, only used by the Win32 GE debugger's +// preview windows, which talk to GL directly rather than going through thin3d. Everything here +// works on shader source strings - the file loading and auto-reload support that used to live +// here was never used in PPSSPP and is gone. #pragma once #include -#include #include "Common/GPU/OpenGL/GLCommon.h" // Represent a compiled and linked vshader/fshader pair. -// A just-constructed object is valid but cannot be used as a shader program, meaning that -// yes, you can declare these as globals if you like. struct GLSLProgram { - char name[16]; - char vshader_filename[256]; - char fshader_filename[256]; - const char *vshader_source; - const char *fshader_source; - time_t vshader_mtime; - time_t fshader_mtime; - - // Locations to some common uniforms. Hardcoded for speed. + // Locations of the uniforms and attributes the callers use, looked up once at link time. + // Add to these as needed - each one costs a lookup per link. GLint sampler0; - GLint sampler1; - GLint u_worldviewproj; - GLint u_world; GLint u_viewproj; - GLint u_fog; // rgb = color, a = density - GLint u_sundir; - GLint u_camerapos; - GLint a_position; - GLint a_color; - GLint a_normal; GLint a_texcoord0; - GLint a_texcoord1; // Private to the implementation, do not touch GLuint vsh_; @@ -42,19 +26,10 @@ struct GLSLProgram { GLuint program_; }; -// C API, old skool. Not much point either... - -// From files (VFS) -GLSLProgram *glsl_create(const char *vshader_file, const char *fshader_file, std::string *error_message = 0); -// Directly from source code -GLSLProgram *glsl_create_source(const char *vshader_src, const char *fshader_src, std::string *error_message = 0); +// Compiles and links a program. Returns nullptr on failure, having logged the reason and, +// if error_message is non-null, copied it there. +GLSLProgram *glsl_create_source(const char *vshader_src, const char *fshader_src, std::string *error_message = nullptr); void glsl_destroy(GLSLProgram *program); -// If recompilation of the program fails, the program is untouched and error messages -// are logged and the function returns false. -bool glsl_recompile(GLSLProgram *program, std::string *error_message = 0); void glsl_bind(const GLSLProgram *program); -const GLSLProgram *glsl_get_program(); void glsl_unbind(); -int glsl_attrib_loc(const GLSLProgram *program, const char *name); -int glsl_uniform_loc(const GLSLProgram *program, const char *name); diff --git a/Common/GPU/OpenGL/thin3d_gl.cpp b/Common/GPU/OpenGL/thin3d_gl.cpp index 1947cf6cf2..1c27c6e2b8 100644 --- a/Common/GPU/OpenGL/thin3d_gl.cpp +++ b/Common/GPU/OpenGL/thin3d_gl.cpp @@ -534,26 +534,6 @@ private: PresentMode requestedPresentMode_{}; }; -static constexpr int MakeIntelSimpleVer(int v1, int v2, int v3) { - return (v1 << 16) | (v2 << 8) | v3; -} - -static bool HasIntelDualSrcBug(const int versions[4]) { - // Intel uses a confusing set of at least 3 version numbering schemes. This is the one given to OpenGL. - switch (MakeIntelSimpleVer(versions[0], versions[1], versions[2])) { - case MakeIntelSimpleVer(9, 17, 10): - case MakeIntelSimpleVer(9, 18, 10): - return false; - case MakeIntelSimpleVer(10, 18, 10): - return versions[3] < 4061; - case MakeIntelSimpleVer(10, 18, 14): - return versions[3] < 4080; - default: - // Older than above didn't support dual src anyway, newer should have the fix. - return false; - } -} - OpenGLContext::OpenGLContext(bool canChangeSwapInterval) : renderManager_(frameTimeHistory_) { if (gl_extensions.IsGLES) { if (gl_extensions.OES_packed_depth_stencil || gl_extensions.OES_depth24) { @@ -647,17 +627,6 @@ OpenGLContext::OpenGLContext(bool canChangeSwapInterval) : renderManager_(frameT if (!gl_extensions.VersionGEThan(3, 0, 0)) { // Don't use this extension on sub 3.0 OpenGL versions as it does not seem reliable. bugs_.Infest(Bugs::DUAL_SOURCE_BLENDING_BROKEN); - } else if (caps_.vendor == GPUVendor::VENDOR_INTEL) { - // Note: this is for Intel drivers with GL3+. - // Also on Intel, see https://github.com/hrydgard/ppsspp/issues/10117 - // TODO: Remove entirely sometime reasonably far in driver years after 2015. - const std::string ver = OpenGLContext::GetInfoString(Draw::InfoField::APIVERSION); - int versions[4]{}; - if (sscanf(ver.c_str(), "Build %d.%d.%d.%d", &versions[0], &versions[1], &versions[2], &versions[3]) == 4) { - if (HasIntelDualSrcBug(versions)) { - bugs_.Infest(Bugs::DUAL_SOURCE_BLENDING_BROKEN); - } - } } #if PPSSPP_ARCH(ARMV7) diff --git a/Common/GPU/Vulkan/VulkanBarrier.cpp b/Common/GPU/Vulkan/VulkanBarrier.cpp index abdd50ca2e..b91596c73d 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.cpp +++ b/Common/GPU/Vulkan/VulkanBarrier.cpp @@ -212,7 +212,7 @@ void VulkanBarrierBatch::TransitionDepthStencilImageAuto( dstStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; break; case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; dstStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; break; case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: diff --git a/Common/GPU/Vulkan/VulkanContext.cpp b/Common/GPU/Vulkan/VulkanContext.cpp index 9a4e4c8fd4..4b6604a2ef 100644 --- a/Common/GPU/Vulkan/VulkanContext.cpp +++ b/Common/GPU/Vulkan/VulkanContext.cpp @@ -907,9 +907,13 @@ VkResult VulkanContext::CreateDevice(int physical_device, const std::vector lock(del.mutex_); _dbg_assert_(cmdPools_.empty()); _dbg_assert_(descPools_.empty()); _dbg_assert_(modules_.empty()); @@ -1862,6 +1871,7 @@ void VulkanDeleteList::Take(VulkanDeleteList &del) { _dbg_assert_(framebuffers_.empty()); _dbg_assert_(pipelineLayouts_.empty()); _dbg_assert_(descSetLayouts_.empty()); + _dbg_assert_(queryPools_.empty()); _dbg_assert_(callbacks_.empty()); cmdPools_ = std::move(del.cmdPools_); descPools_ = std::move(del.descPools_); @@ -1879,12 +1889,14 @@ void VulkanDeleteList::Take(VulkanDeleteList &del) { framebuffers_ = std::move(del.framebuffers_); pipelineLayouts_ = std::move(del.pipelineLayouts_); descSetLayouts_ = std::move(del.descSetLayouts_); + queryPools_ = std::move(del.queryPools_); callbacks_ = std::move(del.callbacks_); del.cmdPools_.clear(); del.descPools_.clear(); del.modules_.clear(); del.buffers_.clear(); del.buffersWithAllocs_.clear(); + del.bufferViews_.clear(); del.imageViews_.clear(); del.imagesWithAllocs_.clear(); del.deviceMemory_.clear(); @@ -1895,10 +1907,33 @@ void VulkanDeleteList::Take(VulkanDeleteList &del) { del.framebuffers_.clear(); del.pipelineLayouts_.clear(); del.descSetLayouts_.clear(); + del.queryPools_.clear(); del.callbacks_.clear(); } void VulkanDeleteList::PerformDeletes(VulkanContext *vulkan, VmaAllocator allocator) { + // Drain into a local list before destroying anything - a callback is allowed to queue more deletes + // (~VKFramebuffer does, via ~VKRFramebuffer), and we mustn't append to a vector we're iterating. + // Anything queued back onto this list gets picked up by the next lap, so keep going until a lap + // comes up empty. In the normal per-frame case that's a single extra lap, since callbacks queue + // onto the global list rather than the frame's own list - the loop matters for + // VulkanContext::PerformPendingDeletes(), which drains the global list itself just before the + // device goes away, with no later pass to catch the stragglers. + int deleteCount = 0; + for (;;) { + VulkanDeleteList taken; + taken.Take(*this); + int count = taken.PerformDeletesInternal(vulkan, allocator); + if (!count) { + break; + } + deleteCount += count; + } + deleteCount_ = deleteCount; +} + +// See the note on Take() - every vector in the class must be handled here too, or it leaks. +int VulkanDeleteList::PerformDeletesInternal(VulkanContext *vulkan, VmaAllocator allocator) { int deleteCount = 0; for (auto &callback : callbacks_) { @@ -1993,7 +2028,7 @@ void VulkanDeleteList::PerformDeletes(VulkanContext *vulkan, VmaAllocator alloca deleteCount++; } queryPools_.clear(); - deleteCount_ = deleteCount; + return deleteCount; } void VulkanContext::GetImageMemoryRequirements(VkImage image, VkMemoryRequirements *mem_reqs, bool *dedicatedAllocation) { diff --git a/Common/GPU/Vulkan/VulkanContext.h b/Common/GPU/Vulkan/VulkanContext.h index 72e1576093..a8e9e7b03f 100644 --- a/Common/GPU/Vulkan/VulkanContext.h +++ b/Common/GPU/Vulkan/VulkanContext.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -70,6 +71,19 @@ enum class PerfClass { typedef std::function DeleteCallback; // This is a bit repetitive... +// +// Thread safety: The queueing functions are locked, because the global delete list gets written from +// more than one thread. Most callers are on the main thread, but not all: +// * VulkanDescSetPool::Recreate, when a descriptor pool has to grow, runs from FlushDescSets on +// the render thread. This one really happens - some games go past the initial 1024 descriptors. +// * VulkanQueueRunner::ResizeReadbackBuffer runs from PerformReadback on the render thread. For +// blocking readbacks the main thread is parked in FlushSync so it can't collide, and the delayed +// ones never actually resize (the readback key contains the dimensions), but it's not worth +// relying on that staying true. +// Meanwhile the main thread moves the global list into the current frame's list in EndFrame(). +// +// PerformDeletes needs no lock of its own: it drains into a private local list (under Take's lock) and +// destroys from that, so the destruction never touches a list another thread can reach. class VulkanDeleteList { struct BufferWithAlloc { VkBuffer buffer; @@ -91,36 +105,40 @@ class VulkanDeleteList { public: // NOTE: These all take reference handles so they can zero the input value. - void QueueDeleteCommandPool(VkCommandPool &pool) { _dbg_assert_(pool != VK_NULL_HANDLE); cmdPools_.push_back(pool); pool = VK_NULL_HANDLE; } - void QueueDeleteDescriptorPool(VkDescriptorPool &pool) { _dbg_assert_(pool != VK_NULL_HANDLE); descPools_.push_back(pool); pool = VK_NULL_HANDLE; } - void QueueDeleteShaderModule(VkShaderModule &module) { _dbg_assert_(module != VK_NULL_HANDLE); modules_.push_back(module); module = VK_NULL_HANDLE; } - void QueueDeleteBuffer(VkBuffer &buffer) { _dbg_assert_(buffer != VK_NULL_HANDLE); buffers_.push_back(buffer); buffer = VK_NULL_HANDLE; } - void QueueDeleteBufferView(VkBufferView &bufferView) { _dbg_assert_(bufferView != VK_NULL_HANDLE); bufferViews_.push_back(bufferView); bufferView = VK_NULL_HANDLE; } - void QueueDeleteImageView(VkImageView &imageView) { _dbg_assert_(imageView != VK_NULL_HANDLE); imageViews_.push_back(imageView); imageView = VK_NULL_HANDLE; } - void QueueDeleteDeviceMemory(VkDeviceMemory &deviceMemory) { _dbg_assert_(deviceMemory != VK_NULL_HANDLE); deviceMemory_.push_back(deviceMemory); deviceMemory = VK_NULL_HANDLE; } - void QueueDeleteSampler(VkSampler &sampler) { _dbg_assert_(sampler != VK_NULL_HANDLE); samplers_.push_back(sampler); sampler = VK_NULL_HANDLE; } - void QueueDeletePipeline(VkPipeline &pipeline) { _dbg_assert_(pipeline != VK_NULL_HANDLE); pipelines_.push_back(pipeline); pipeline = VK_NULL_HANDLE; } - void QueueDeletePipelineCache(VkPipelineCache &pipelineCache) { _dbg_assert_(pipelineCache != VK_NULL_HANDLE); pipelineCaches_.push_back(pipelineCache); pipelineCache = VK_NULL_HANDLE; } - void QueueDeleteRenderPass(VkRenderPass &renderPass) { _dbg_assert_(renderPass != VK_NULL_HANDLE); renderPasses_.push_back(renderPass); renderPass = VK_NULL_HANDLE; } - void QueueDeleteFramebuffer(VkFramebuffer &framebuffer) { _dbg_assert_(framebuffer != VK_NULL_HANDLE); framebuffers_.push_back(framebuffer); framebuffer = VK_NULL_HANDLE; } - void QueueDeletePipelineLayout(VkPipelineLayout &pipelineLayout) { _dbg_assert_(pipelineLayout != VK_NULL_HANDLE); pipelineLayouts_.push_back(pipelineLayout); pipelineLayout = VK_NULL_HANDLE; } - void QueueDeleteDescriptorSetLayout(VkDescriptorSetLayout &descSetLayout) { _dbg_assert_(descSetLayout != VK_NULL_HANDLE); descSetLayouts_.push_back(descSetLayout); descSetLayout = VK_NULL_HANDLE; } - void QueueDeleteQueryPool(VkQueryPool &queryPool) { _dbg_assert_(queryPool != VK_NULL_HANDLE); queryPools_.push_back(queryPool); queryPool = VK_NULL_HANDLE; } - void QueueCallback(DeleteCallback func) { callbacks_.push_back(func); } + void QueueDeleteCommandPool(VkCommandPool &pool) { _dbg_assert_(pool != VK_NULL_HANDLE); std::lock_guard lock(mutex_); cmdPools_.push_back(pool); pool = VK_NULL_HANDLE; } + void QueueDeleteDescriptorPool(VkDescriptorPool &pool) { _dbg_assert_(pool != VK_NULL_HANDLE); std::lock_guard lock(mutex_); descPools_.push_back(pool); pool = VK_NULL_HANDLE; } + void QueueDeleteShaderModule(VkShaderModule &module) { _dbg_assert_(module != VK_NULL_HANDLE); std::lock_guard lock(mutex_); modules_.push_back(module); module = VK_NULL_HANDLE; } + void QueueDeleteBuffer(VkBuffer &buffer) { _dbg_assert_(buffer != VK_NULL_HANDLE); std::lock_guard lock(mutex_); buffers_.push_back(buffer); buffer = VK_NULL_HANDLE; } + void QueueDeleteBufferView(VkBufferView &bufferView) { _dbg_assert_(bufferView != VK_NULL_HANDLE); std::lock_guard lock(mutex_); bufferViews_.push_back(bufferView); bufferView = VK_NULL_HANDLE; } + void QueueDeleteImageView(VkImageView &imageView) { _dbg_assert_(imageView != VK_NULL_HANDLE); std::lock_guard lock(mutex_); imageViews_.push_back(imageView); imageView = VK_NULL_HANDLE; } + void QueueDeleteDeviceMemory(VkDeviceMemory &deviceMemory) { _dbg_assert_(deviceMemory != VK_NULL_HANDLE); std::lock_guard lock(mutex_); deviceMemory_.push_back(deviceMemory); deviceMemory = VK_NULL_HANDLE; } + void QueueDeleteSampler(VkSampler &sampler) { _dbg_assert_(sampler != VK_NULL_HANDLE); std::lock_guard lock(mutex_); samplers_.push_back(sampler); sampler = VK_NULL_HANDLE; } + void QueueDeletePipeline(VkPipeline &pipeline) { _dbg_assert_(pipeline != VK_NULL_HANDLE); std::lock_guard lock(mutex_); pipelines_.push_back(pipeline); pipeline = VK_NULL_HANDLE; } + void QueueDeletePipelineCache(VkPipelineCache &pipelineCache) { _dbg_assert_(pipelineCache != VK_NULL_HANDLE); std::lock_guard lock(mutex_); pipelineCaches_.push_back(pipelineCache); pipelineCache = VK_NULL_HANDLE; } + void QueueDeleteRenderPass(VkRenderPass &renderPass) { _dbg_assert_(renderPass != VK_NULL_HANDLE); std::lock_guard lock(mutex_); renderPasses_.push_back(renderPass); renderPass = VK_NULL_HANDLE; } + void QueueDeleteFramebuffer(VkFramebuffer &framebuffer) { _dbg_assert_(framebuffer != VK_NULL_HANDLE); std::lock_guard lock(mutex_); framebuffers_.push_back(framebuffer); framebuffer = VK_NULL_HANDLE; } + void QueueDeletePipelineLayout(VkPipelineLayout &pipelineLayout) { _dbg_assert_(pipelineLayout != VK_NULL_HANDLE); std::lock_guard lock(mutex_); pipelineLayouts_.push_back(pipelineLayout); pipelineLayout = VK_NULL_HANDLE; } + void QueueDeleteDescriptorSetLayout(VkDescriptorSetLayout &descSetLayout) { _dbg_assert_(descSetLayout != VK_NULL_HANDLE); std::lock_guard lock(mutex_); descSetLayouts_.push_back(descSetLayout); descSetLayout = VK_NULL_HANDLE; } + void QueueDeleteQueryPool(VkQueryPool &queryPool) { _dbg_assert_(queryPool != VK_NULL_HANDLE); std::lock_guard lock(mutex_); queryPools_.push_back(queryPool); queryPool = VK_NULL_HANDLE; } + void QueueCallback(DeleteCallback func) { std::lock_guard lock(mutex_); callbacks_.push_back(func); } - void QueueDeleteBufferAllocation(VkBuffer &buffer, VmaAllocation &alloc) { - _dbg_assert_(buffer != VK_NULL_HANDLE); + void QueueDeleteBufferAllocation(VkBuffer &buffer, VmaAllocation &alloc) { + _dbg_assert_(buffer != VK_NULL_HANDLE); + std::lock_guard lock(mutex_); buffersWithAllocs_.push_back(BufferWithAlloc{ buffer, alloc }); buffer = VK_NULL_HANDLE; alloc = VK_NULL_HANDLE; } void QueueDeleteImageAllocation(VkImage &image, VmaAllocation &alloc) { _dbg_assert_(image != VK_NULL_HANDLE && alloc != VK_NULL_HANDLE); + std::lock_guard lock(mutex_); imagesWithAllocs_.push_back(ImageWithAlloc{ image, alloc }); image = VK_NULL_HANDLE; alloc = VK_NULL_HANDLE; } + // Moves everything from del into this list. Only the source list is locked - the destination is + // either a frame's own list or a stack local, neither of which another thread can reach. void Take(VulkanDeleteList &del); void PerformDeletes(VulkanContext *vulkan, VmaAllocator allocator); @@ -129,6 +147,10 @@ public: } private: + // Does the actual destruction, on a list that's been drained out of the shared one. Returns the count. + int PerformDeletesInternal(VulkanContext *vulkan, VmaAllocator allocator); + + std::mutex mutex_; std::vector cmdPools_; std::vector descPools_; std::vector modules_; diff --git a/Common/GPU/Vulkan/VulkanLoader.cpp b/Common/GPU/Vulkan/VulkanLoader.cpp index 9c6bde6be8..5179e85976 100644 --- a/Common/GPU/Vulkan/VulkanLoader.cpp +++ b/Common/GPU/Vulkan/VulkanLoader.cpp @@ -537,7 +537,8 @@ bool VulkanMayBeAvailable() { INFO_LOG(Log::G3D, "VulkanMayBeAvailable: Found platform surface extension '%s'", platformSurfaceExtension); instanceExtensions[ci.enabledExtensionCount++] = platformSurfaceExtension; platformSurfaceExtensionFound = true; - break; + // Note: Can't stop here - the enumeration order isn't specified anywhere, so VK_KHR_surface + // may well come after the platform one, and we need both. } else if (!strcmp(iter.extensionName, VK_KHR_SURFACE_EXTENSION_NAME)) { instanceExtensions[ci.enabledExtensionCount++] = VK_KHR_SURFACE_EXTENSION_NAME; surfaceExtensionFound = true; @@ -618,8 +619,9 @@ bool VulkanMayBeAvailable() { } } } - anyGood = !blacklisted; - if (anyGood) { + // Note: Must not overwrite the verdict from a previously seen device, one good one is enough. + anyGood = anyGood || !blacklisted; + if (!blacklisted) { INFO_LOG(Log::G3D, "VulkanMayBeAvailable: Eligible device found: '%s'", props.deviceName); } else { INFO_LOG(Log::G3D, "VulkanMayBeAvailable: Blacklisted device found and ignored: '%s'", props.deviceName); diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index d8b7574b9c..cafd714545 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -694,7 +694,7 @@ void VulkanRenderManager::PollPresentTiming() { } void VulkanRenderManager::BeginFrame(bool enableProfiling, bool enableLogProfiler) { - double frameBeginTime = time_now_d() + double frameBeginTime = time_now_d(); VLOG("BeginFrame"); VkDevice device = vulkan_->GetDevice(); @@ -1806,12 +1806,22 @@ VKRPipelineLayout *VulkanRenderManager::CreatePipelineLayout(BindingType *bindin layout->frameData[i].pool.Create(vulkan_, bindingTypes, (uint32_t)bindingTypesCount, 1024); } - pipelineLayouts_.push_back(layout); + { + std::lock_guard lock(pipelineLayoutsMutex_); + pipelineLayouts_.push_back(layout); + } return layout; } void VulkanRenderManager::DestroyPipelineLayout(VKRPipelineLayout *layout) { + // The layout has to stay in pipelineLayouts_ until the frames that were recorded with it have been + // flushed by the render thread, otherwise their descriptor sets never get written. So, we can't + // remove it here - instead we let it ride along on the delete list, which won't be run until the + // fence for the frame it was queued in has been waited on. vulkan_->Delete().QueueCallback([this, layout](VulkanContext *vulkan) { + // Runs on the main thread, while the render thread may be in FlushDescriptors - so both the + // erase and the destruction of the layout itself have to be under the lock. + std::lock_guard lock(pipelineLayoutsMutex_); for (auto iter = pipelineLayouts_.begin(); iter != pipelineLayouts_.end(); iter++) { if (*iter == layout) { pipelineLayouts_.erase(iter); @@ -1828,13 +1838,17 @@ void VulkanRenderManager::DestroyPipelineLayout(VKRPipelineLayout *layout) { }); } +// Called on the render thread. void VulkanRenderManager::FlushDescriptors(int frame) { + std::lock_guard lock(pipelineLayoutsMutex_); for (VKRPipelineLayout *iter : pipelineLayouts_) { iter->FlushDescSets(vulkan_, frame, &frameData_[frame].profile); } } +// Called on the main thread, from BeginFrame. void VulkanRenderManager::ResetDescriptorLists(int frame) { + std::lock_guard lock(pipelineLayoutsMutex_); for (VKRPipelineLayout *iter : pipelineLayouts_) { VKRPipelineLayout::FrameData &data = iter->frameData[frame]; diff --git a/Common/GPU/Vulkan/VulkanRenderManager.h b/Common/GPU/Vulkan/VulkanRenderManager.h index 024d81b094..4497f3ce2d 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.h +++ b/Common/GPU/Vulkan/VulkanRenderManager.h @@ -643,5 +643,12 @@ private: HistoryBuffer &frameTimeHistory_; VKRPipelineLayout *curPipelineLayout_ = nullptr; + + // Guards pipelineLayouts_, both the vector itself and the lifetime of the layouts in it. + // The list is added to by CreatePipelineLayout and erased from by the deferred callback queued by + // DestroyPipelineLayout, both of which run on the main thread, while the render thread walks it + // every frame in FlushDescriptors. Contention is negligible - layouts are only created and destroyed + // when a game starts or stops, and the lock is otherwise taken exactly twice per frame. + std::mutex pipelineLayoutsMutex_; std::vector pipelineLayouts_; };