Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of shadergen failures, other minor things #18169

Merged
merged 5 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions Common/GPU/Vulkan/VulkanRenderManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,14 +500,20 @@ void VulkanRenderManager::CompileThreadFunc() {
}
}

void VulkanRenderManager::DrainCompileQueue() {
void VulkanRenderManager::DrainAndBlockCompileQueue() {
std::unique_lock<std::mutex> lock(compileMutex_);
compileBlocked_ = true;
compileCond_.notify_all();
while (!compileQueue_.empty()) {
queueRunner_.WaitForCompileNotification();
}
}

void VulkanRenderManager::ReleaseCompileQueue() {
std::unique_lock<std::mutex> lock(compileMutex_);
compileBlocked_ = false;
}

void VulkanRenderManager::ThreadFunc() {
SetCurrentThreadName("RenderMan");
while (true) {
Expand Down Expand Up @@ -709,14 +715,12 @@ VkCommandBuffer VulkanRenderManager::GetInitCmd() {
}

VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, bool cacheLoad, const char *tag) {
VKRGraphicsPipeline *pipeline = new VKRGraphicsPipeline(pipelineFlags, tag);

if (!desc->vertexShader || !desc->fragmentShader) {
ERROR_LOG(G3D, "Can't create graphics pipeline with missing vs/ps: %p %p", desc->vertexShader, desc->fragmentShader);
delete pipeline;
return nullptr;
}

VKRGraphicsPipeline *pipeline = new VKRGraphicsPipeline(pipelineFlags, tag);
pipeline->desc = desc;
pipeline->desc->AddRef();
if (curRenderStep_ && !cacheLoad) {
Expand All @@ -733,7 +737,11 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
VKRRenderPassStoreAction::STORE, VKRRenderPassStoreAction::DONT_CARE, VKRRenderPassStoreAction::DONT_CARE,
};
VKRRenderPass *compatibleRenderPass = queueRunner_.GetRenderPass(key);
compileMutex_.lock();
std::lock_guard<std::mutex> lock(compileMutex_);
if (compileBlocked_) {
delete pipeline;
return nullptr;
}
bool needsCompile = false;
for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) {
if (!(variantBitmask & (1 << i)))
Expand All @@ -757,18 +765,19 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
}
if (needsCompile)
compileCond_.notify_one();
compileMutex_.unlock();
}
return pipeline;
}

VKRComputePipeline *VulkanRenderManager::CreateComputePipeline(VKRComputePipelineDesc *desc) {
std::lock_guard<std::mutex> lock(compileMutex_);
if (compileBlocked_) {
return nullptr;
}
VKRComputePipeline *pipeline = new VKRComputePipeline();
pipeline->desc = desc;
compileMutex_.lock();
compileQueue_.push_back(CompileQueueEntry(pipeline));
compileCond_.notify_one();
compileMutex_.unlock();
return pipeline;
}

Expand Down Expand Up @@ -814,7 +823,7 @@ void VulkanRenderManager::EndCurRenderStep() {
compileMutex_.lock();
bool needsCompile = false;
for (VKRGraphicsPipeline *pipeline : pipelinesToCheck_) {
if (!pipeline) {
if (!pipeline || compileBlocked_) {
// Not good, but let's try not to crash.
continue;
}
Expand Down
4 changes: 3 additions & 1 deletion Common/GPU/Vulkan/VulkanRenderManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ class VulkanRenderManager {
}

void ResetStats();
void DrainCompileQueue();
void DrainAndBlockCompileQueue();
void ReleaseCompileQueue();

private:
void EndCurRenderStep();
Expand Down Expand Up @@ -527,6 +528,7 @@ class VulkanRenderManager {
std::condition_variable compileCond_;
std::mutex compileMutex_;
std::vector<CompileQueueEntry> compileQueue_;
bool compileBlocked_ = false;

// Thread for measuring presentation delay.
std::thread presentWaitThread_;
Expand Down
1 change: 1 addition & 0 deletions Common/GPU/Vulkan/thin3d_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,7 @@ void VKTexture::UpdateInternal(VkCommandBuffer cmd, VulkanPushPool *pushBuffer,
int bpp = GetBpp(vulkanFormat);
int bytesPerPixel = bpp / 8;
TextureCopyBatch batch;
batch.reserve(numLevels);
for (i = 0; i < numLevels; i++) {
uint32_t offset;
VkBuffer buf;
Expand Down
19 changes: 17 additions & 2 deletions GPU/Common/ShaderId.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,21 @@ bool FragmentIdNeedsFramebufferRead(const FShaderID &id) {
(ReplaceBlendType)id.Bits(FS_BIT_REPLACE_BLEND, 3) == REPLACE_BLEND_READ_FRAMEBUFFER;
}

static GEBlendMode SanitizeBlendEq(GEBlendMode beq) {
switch (beq) {
case GE_BLENDMODE_MUL_AND_ADD:
case GE_BLENDMODE_MUL_AND_SUBTRACT:
case GE_BLENDMODE_MUL_AND_SUBTRACT_REVERSE:
case GE_BLENDMODE_MIN:
case GE_BLENDMODE_MAX:
case GE_BLENDMODE_ABSDIFF:
return beq;
default:
// Just return something that won't cause a shader gen failure.
return GE_BLENDMODE_MUL_AND_ADD;
}
}

// Here we must take all the bits of the gstate that determine what the fragment shader will
// look like, and concatenate them together into an ID.
void ComputeFragmentShaderID(FShaderID *id_out, const ComputedPipelineState &pipelineState, const Draw::Bugs &bugs) {
Expand Down Expand Up @@ -369,15 +384,15 @@ void ComputeFragmentShaderID(FShaderID *id_out, const ComputedPipelineState &pip
// 3 bits.
id.SetBits(FS_BIT_REPLACE_BLEND, 3, replaceBlend);
// 11 bits total.
id.SetBits(FS_BIT_BLENDEQ, 3, gstate.getBlendEq());
id.SetBits(FS_BIT_BLENDEQ, 3, SanitizeBlendEq(gstate.getBlendEq()));
id.SetBits(FS_BIT_BLENDFUNC_A, 4, gstate.getBlendFuncA());
id.SetBits(FS_BIT_BLENDFUNC_B, 4, gstate.getBlendFuncB());
}
id.SetBit(FS_BIT_FLATSHADE, doFlatShading);
id.SetBit(FS_BIT_COLOR_WRITEMASK, colorWriteMask);

// All framebuffers are array textures in Vulkan now.
if (gstate_c.textureIsArray && g_Config.iGPUBackend == (int)GPUBackend::VULKAN) {
if (gstate_c.textureIsArray && gstate_c.Use(GPU_USE_FRAMEBUFFER_ARRAYS)) {
id.SetBit(FS_BIT_SAMPLE_ARRAY_TEXTURE);
}

Expand Down
20 changes: 16 additions & 4 deletions GPU/GLES/ShaderManagerGLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,8 @@ void ShaderManagerGLES::DirtyLastShader() {
lastVShaderSame_ = false;
}

// Can only fail by failing to generate the code (bad FSID).
// Any actual failures driver-side happens later in the render manager.
Shader *ShaderManagerGLES::CompileFragmentShader(FShaderID FSID) {
uint64_t uniformMask;
std::string errorString;
Expand All @@ -762,6 +764,8 @@ Shader *ShaderManagerGLES::CompileFragmentShader(FShaderID FSID) {
return new Shader(render_, codeBuffer_, desc, params);
}

// Can only fail by failing to generate the code (bad VSID).
// Any actual failures driver-side happens later in the render manager.
Shader *ShaderManagerGLES::CompileVertexShader(VShaderID VSID) {
bool useHWTransform = VSID.Bit(VS_BIT_USE_HW_TRANSFORM);
uint32_t attrMask;
Expand Down Expand Up @@ -802,13 +806,12 @@ Shader *ShaderManagerGLES::ApplyVertexShader(bool useHWTransform, bool useHWTess

// Vertex shader not in cache. Let's compile it.
vs = CompileVertexShader(*VSID);
if (!vs || vs->Failed()) {
if (!vs) {
auto gr = GetI18NCategory(I18NCat::GRAPHICS);
ERROR_LOG(G3D, "Vertex shader generation failed, falling back to software transform");
if (!g_Config.bHideSlowWarnings) {
g_OSD.Show(OSDType::MESSAGE_ERROR, gr->T("hardware transform error - falling back to software"), 2.5f);
}
delete vs;

// TODO: Look for existing shader with the appropriate ID, use that instead of generating a new one - however, need to make sure
// that that shader ID is not used when computing the linked shader ID below, because then IDs won't match
Expand Down Expand Up @@ -1099,7 +1102,7 @@ bool ShaderManagerGLES::ContinuePrecompile(float sliceTime) {
}

Shader *vs = CompileVertexShader(id);
if (!vs || vs->Failed()) {
if (!vs) {
// Give up on using the cache, just bail. We can't safely create the fallback shaders here
// without trying to deduce the vertType from the VSID.
ERROR_LOG(G3D, "Failed to compile a vertex shader loading from cache. Skipping rest of shader cache.");
Expand All @@ -1121,7 +1124,16 @@ bool ShaderManagerGLES::ContinuePrecompile(float sliceTime) {

const FShaderID &id = pending.frag[i];
if (!fsCache_.ContainsKey(id)) {
fsCache_.Insert(id, CompileFragmentShader(id));
Shader *fs = CompileFragmentShader(id);
if (!fs) {
// Give up on using the cache - something went wrong.
// We'll still keep the shaders we generated so far around.
ERROR_LOG(G3D, "Failed to compile a fragment shader loading from cache. Skipping rest of shader cache.");
delete fs;
pending.Clear();
return false;
}
fsCache_.Insert(id, fs);
} else {
WARN_LOG(G3D, "Duplicate fragment shader found in GL shader cache, ignoring");
}
Expand Down
2 changes: 0 additions & 2 deletions GPU/GLES/ShaderManagerGLES.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ class Shader {
~Shader();
GLRShader *shader;

bool Failed() const { return failed_; }
bool UseHWTransform() const { return useHWTransform_; } // only relevant for vtx shaders

std::string GetShaderString(DebugShaderStringType type, ShaderID id) const;
Expand All @@ -152,7 +151,6 @@ class Shader {
private:
GLRenderManager *render_;
std::string source_;
bool failed_ = false;
bool useHWTransform_;
bool isFragment_;
uint32_t attrMask_; // only used in vertex shaders
Expand Down
2 changes: 1 addition & 1 deletion GPU/GPUState.h
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ enum {
GPU_USE_DEPTH_TEXTURE = FLAG_BIT(16),
GPU_USE_ACCURATE_DEPTH = FLAG_BIT(17),
GPU_USE_GS_CULLING = FLAG_BIT(18), // Geometry shader
// Bit 19 free.
GPU_USE_FRAMEBUFFER_ARRAYS = FLAG_BIT(19),
GPU_USE_FRAMEBUFFER_FETCH = FLAG_BIT(20),
GPU_SCALE_DEPTH_FROM_24BIT_TO_16BIT = FLAG_BIT(21),
GPU_ROUND_FRAGMENT_DEPTH_TO_16BIT = FLAG_BIT(22),
Expand Down
2 changes: 1 addition & 1 deletion GPU/Vulkan/DrawEngineVulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ void DrawEngineVulkan::DoFlush() {

shaderManager_->GetShaders(prim, dec_, &vshader, &fshader, &gshader, pipelineState_, true, useHWTessellation_, decOptions_.expandAllWeightsToFloat, decOptions_.applySkinInDecode);
if (!vshader) {
// We're screwed.
// We're screwed, can't do anything.
return;
}
_dbg_assert_msg_(vshader->UseHWTransform(), "Bad vshader");
Expand Down
8 changes: 7 additions & 1 deletion GPU/Vulkan/GPU_Vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ void GPU_Vulkan::SaveCache(const Path &filename) {
GPU_Vulkan::~GPU_Vulkan() {
if (draw_) {
VulkanRenderManager *rm = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
rm->DrainCompileQueue();
rm->DrainAndBlockCompileQueue();
}

SaveCache(shaderCachePath_);
Expand All @@ -193,6 +193,11 @@ GPU_Vulkan::~GPU_Vulkan() {

delete pipelineManager_;
// other managers are deleted in ~GPUCommonHW.

if (draw_) {
VulkanRenderManager *rm = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
rm->ReleaseCompileQueue();
}
}

u32 GPU_Vulkan::CheckGPUFeatures() const {
Expand Down Expand Up @@ -290,6 +295,7 @@ u32 GPU_Vulkan::CheckGPUFeatures() const {
features &= ~GPU_USE_LIGHT_UBERSHADER;
}

features |= GPU_USE_FRAMEBUFFER_ARRAYS;
return CheckGPUFeaturesLate(features);
}

Expand Down