From a5af0e3f0e4690b73f7d9b6bff64743c379d43b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Nov 2020 00:48:35 +0100 Subject: [PATCH] Show an error on screen if a shader fails to compile. Part of #1 investigation of #13541 --- Common/GPU/OpenGL/GLFeatures.cpp | 3 ++- Common/GPU/OpenGL/GLFeatures.h | 1 + Common/GPU/OpenGL/GLQueueRunner.cpp | 23 ++++++++++++++--------- Common/GPU/OpenGL/GLQueueRunner.h | 9 +++++++++ Common/GPU/OpenGL/GLRenderManager.h | 4 ++++ Common/GPU/OpenGL/thin3d_gl.cpp | 4 ++++ Common/GPU/Shader.h | 5 +++++ Common/GPU/thin3d.h | 2 ++ GPU/Common/FragmentShaderGenerator.cpp | 2 ++ GPU/GLES/ShaderManagerGLES.cpp | 1 + Windows/GPU/WindowsGLContext.cpp | 5 +++++ android/jni/app-android.cpp | 10 ++++++++++ ios/ViewController.mm | 5 +++++ 13 files changed, 64 insertions(+), 10 deletions(-) diff --git a/Common/GPU/OpenGL/GLFeatures.cpp b/Common/GPU/OpenGL/GLFeatures.cpp index ad4df9281923..df8632779cf9 100644 --- a/Common/GPU/OpenGL/GLFeatures.cpp +++ b/Common/GPU/OpenGL/GLFeatures.cpp @@ -356,6 +356,7 @@ void CheckGLExtensions() { gl_extensions.ARB_cull_distance = g_set_gl_extensions.count("GL_ARB_cull_distance") != 0; gl_extensions.ARB_depth_clamp = g_set_gl_extensions.count("GL_ARB_depth_clamp") != 0; gl_extensions.ARB_uniform_buffer_object = g_set_gl_extensions.count("GL_ARB_uniform_buffer_object") != 0; + gl_extensions.ARB_explicit_attrib_location = g_set_gl_extensions.count("GL_ARB_explicit_attrib_location") != 0; if (gl_extensions.IsGLES) { gl_extensions.OES_texture_npot = g_set_gl_extensions.count("GL_OES_texture_npot") != 0; @@ -507,7 +508,7 @@ void CheckGLExtensions() { } if (gl_extensions.VersionGEThan(3, 3)) { gl_extensions.ARB_blend_func_extended = true; - // ARB_explicit_attrib_location = true; + gl_extensions.ARB_explicit_attrib_location = true; } if (gl_extensions.VersionGEThan(4, 0)) { // ARB_gpu_shader5 = true; diff --git a/Common/GPU/OpenGL/GLFeatures.h b/Common/GPU/OpenGL/GLFeatures.h index 884b0a3df323..f4da0ce221e7 100644 --- a/Common/GPU/OpenGL/GLFeatures.h +++ b/Common/GPU/OpenGL/GLFeatures.h @@ -55,6 +55,7 @@ struct GLExtensions { bool ARB_pixel_buffer_object; bool ARB_blend_func_extended; // dual source blending bool EXT_blend_func_extended; // dual source blending (GLES, new 2015) + bool ARB_explicit_attrib_location; bool ARB_shader_image_load_store; bool ARB_shading_language_420pack; bool ARB_conservative_depth; diff --git a/Common/GPU/OpenGL/GLQueueRunner.cpp b/Common/GPU/OpenGL/GLQueueRunner.cpp index c01eb0ed702b..e53721bb1b52 100644 --- a/Common/GPU/OpenGL/GLQueueRunner.cpp +++ b/Common/GPU/OpenGL/GLQueueRunner.cpp @@ -277,21 +277,26 @@ void GLQueueRunner::RunInitSteps(const std::vector &steps, bool ski glCompileShader(shader); GLint success = 0; glGetShaderiv(shader, GL_COMPILE_STATUS, &success); + std::string infoLog = GetInfoLog(shader, glGetShaderiv, glGetShaderInfoLog); if (!success) { - std::string infoLog = GetInfoLog(shader, glGetShaderiv, glGetShaderInfoLog); -#if PPSSPP_PLATFORM(ANDROID) - ERROR_LOG(G3D, "Error in shader compilation! %s\n", infoLog.c_str()); - ERROR_LOG(G3D, "Shader source:\n%s\n", (const char *)code); -#endif - ERROR_LOG(G3D, "Error in shader compilation for: %s", step.create_shader.shader->desc.c_str()); - ERROR_LOG(G3D, "Info log: %s", infoLog.c_str()); - ERROR_LOG(G3D, "Shader source:\n%s\n", (const char *)code); + std::string errorString = StringFromFormat( + "Error in shader compilation for: %s\n" + "Info log: %s\n" + "Shader source:\n%s\n//END\n\n", + step.create_shader.shader->desc.c_str(), + infoLog.c_str(), + LineNumberString(code).c_str()); + if (errorCallback_) { + std::string desc = StringFromFormat("Shader compilation failed: %s", step.create_shader.stage == GL_VERTEX_SHADER ? "vertex" : "fragment"); + errorCallback_(desc.c_str(), errorString.c_str(), errorCallbackUserData_); + } Reporting::ReportMessage("Error in shader compilation: info: %s\n%s\n%s", infoLog.c_str(), step.create_shader.shader->desc.c_str(), (const char *)code); + } else { #ifdef SHADERLOG OutputDebugStringUTF8(infoLog.c_str()); #endif step.create_shader.shader->failed = true; - step.create_shader.shader->error = infoLog; + step.create_shader.shader->error = infoLog; // Hm, we never use this. } // Before we throw away the code, attach it to the shader for debugging. step.create_shader.shader->code = code; diff --git a/Common/GPU/OpenGL/GLQueueRunner.h b/Common/GPU/OpenGL/GLQueueRunner.h index 79559c4d4db6..f47bfd7c40ec 100644 --- a/Common/GPU/OpenGL/GLQueueRunner.h +++ b/Common/GPU/OpenGL/GLQueueRunner.h @@ -6,6 +6,7 @@ #include "Common/GPU/OpenGL/GLCommon.h" #include "Common/GPU/DataFormat.h" +#include "Common/GPU/Shader.h" #include "Common/Data/Collections/TinySet.h" struct GLRViewport { @@ -341,6 +342,11 @@ class GLQueueRunner { public: GLQueueRunner() {} + void SetErrorCallback(ErrorCallbackFn callback, void *userdata) { + errorCallback_ = callback; + errorCallbackUserData_ = userdata; + } + void RunInitSteps(const std::vector &steps, bool skipGLCalls); void RunSteps(const std::vector &steps, bool skipGLCalls); @@ -423,4 +429,7 @@ class GLQueueRunner { bool sawOutOfMemory_ = false; bool useDebugGroups_ = false; + + ErrorCallbackFn errorCallback_ = nullptr; + void *errorCallbackUserData_ = nullptr; }; diff --git a/Common/GPU/OpenGL/GLRenderManager.h b/Common/GPU/OpenGL/GLRenderManager.h index a2f3a7effe4f..3999332e691f 100644 --- a/Common/GPU/OpenGL/GLRenderManager.h +++ b/Common/GPU/OpenGL/GLRenderManager.h @@ -357,6 +357,10 @@ class GLRenderManager { GLRenderManager(); ~GLRenderManager(); + void SetErrorCallback(ErrorCallbackFn callback, void *userdata) { + queueRunner_.SetErrorCallback(callback, userdata); + } + void ThreadStart(Draw::DrawContext *draw); void ThreadEnd(); bool ThreadFrame(); // Returns false to request exiting the loop. diff --git a/Common/GPU/OpenGL/thin3d_gl.cpp b/Common/GPU/OpenGL/thin3d_gl.cpp index be13d312e866..76f0fa72308b 100644 --- a/Common/GPU/OpenGL/thin3d_gl.cpp +++ b/Common/GPU/OpenGL/thin3d_gl.cpp @@ -354,6 +354,10 @@ class OpenGLContext : public DrawContext { } uint32_t GetDataFormatSupport(DataFormat fmt) const override; + void SetErrorCallback(ErrorCallbackFn callback, void *userdata) override { + renderManager_.SetErrorCallback(callback, userdata); + } + DepthStencilState *CreateDepthStencilState(const DepthStencilStateDesc &desc) override; BlendState *CreateBlendState(const BlendStateDesc &desc) override; SamplerState *CreateSamplerState(const SamplerStateDesc &desc) override; diff --git a/Common/GPU/Shader.h b/Common/GPU/Shader.h index 9457a8461bb1..c19753420aed 100644 --- a/Common/GPU/Shader.h +++ b/Common/GPU/Shader.h @@ -41,3 +41,8 @@ struct ShaderLanguageDesc { bool forceMatrix4x4 = false; bool coefsFromBuffers = false; }; + +// For passing error messages from shader compilation (and other critical issues) back to the host. +// This can run on any thread - be aware! +// TODO: See if we can find a less generic name for this. +typedef void (*ErrorCallbackFn)(const char *shortDesc, const char *details, void *userdata); diff --git a/Common/GPU/thin3d.h b/Common/GPU/thin3d.h index 8aeaa819b20f..9b55995eef9d 100644 --- a/Common/GPU/thin3d.h +++ b/Common/GPU/thin3d.h @@ -567,6 +567,8 @@ class DrawContext { virtual uint32_t GetSupportedShaderLanguages() const = 0; + virtual void SetErrorCallback(ErrorCallbackFn callback, void *userdata) {} + // Partial pipeline state, used to create pipelines. (in practice, in d3d11 they'll use the native state objects directly). virtual DepthStencilState *CreateDepthStencilState(const DepthStencilStateDesc &desc) = 0; virtual BlendState *CreateBlendState(const BlendStateDesc &desc) = 0; diff --git a/GPU/Common/FragmentShaderGenerator.cpp b/GPU/Common/FragmentShaderGenerator.cpp index 10b34cf167c8..5c07c72cd335 100644 --- a/GPU/Common/FragmentShaderGenerator.cpp +++ b/GPU/Common/FragmentShaderGenerator.cpp @@ -65,6 +65,8 @@ bool GenerateFragmentShader(const FShaderID &id, char *buffer, const ShaderLangu ShaderWriter p(buffer, compat, ShaderStage::Fragment, gl_exts.data(), gl_exts.size()); + p.C("SABOTAGE"); + bool lmode = id.Bit(FS_BIT_LMODE); bool doTexture = id.Bit(FS_BIT_DO_TEXTURE); bool enableFog = id.Bit(FS_BIT_ENABLE_FOG); diff --git a/GPU/GLES/ShaderManagerGLES.cpp b/GPU/GLES/ShaderManagerGLES.cpp index e86573a1e86b..be3358c9593c 100644 --- a/GPU/GLES/ShaderManagerGLES.cpp +++ b/GPU/GLES/ShaderManagerGLES.cpp @@ -801,6 +801,7 @@ LinkedShader *ShaderManagerGLES::ApplyFragmentShader(VShaderID VSID, Shader *vs, Shader *fs = fsCache_.Get(FSID); if (!fs) { // Fragment shader not in cache. Let's compile it. + // Can't really tell if we succeeded since the compile is on the GPU thread later. fs = CompileFragmentShader(FSID); fsCache_.Insert(FSID, fs); diskCacheDirty_ = true; diff --git a/Windows/GPU/WindowsGLContext.cpp b/Windows/GPU/WindowsGLContext.cpp index 765321c903f9..e919a1042051 100644 --- a/Windows/GPU/WindowsGLContext.cpp +++ b/Windows/GPU/WindowsGLContext.cpp @@ -27,6 +27,7 @@ #include "Core/Config.h" #include "Core/ConfigValues.h" #include "Core/Core.h" +#include "Core/Host.h" #include "Common/Data/Encoding/Utf8.h" #include "Common/Data/Text/I18n.h" #include "UI/OnScreenDisplay.h" @@ -420,6 +421,10 @@ bool WindowsGLContext::InitFromRenderThread(std::string *error_message) { return false; } + draw_->SetErrorCallback([](const char *shortDesc, const char *details, void *userdata) { + host->NotifyUserMessage(details, 5.0, 0xFFFFFFFF, "error_callback"); + }, nullptr); + // These are auto-reset events. pauseEvent = CreateEvent(NULL, FALSE, FALSE, NULL); resumeEvent = CreateEvent(NULL, FALSE, FALSE, NULL); diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index e36e15ab92ed..82a69e8c2712 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -82,6 +82,7 @@ struct JNIEnv {}; #include "Core/System.h" #include "Core/HLE/sceUsbCam.h" #include "Core/HLE/sceUsbGps.h" +#include "Core/Host.h" #include "Common/CPUDetect.h" #include "Common/Log.h" #include "UI/GameInfoCache.h" @@ -777,6 +778,10 @@ extern "C" bool Java_org_ppsspp_ppsspp_NativeRenderer_displayInit(JNIEnv * env, return false; } + graphicsContext->GetDrawContext()->SetErrorCallback([](const char *shortDesc, const char *details, void *userdata) { + host->NotifyUserMessage(details, 5.0, 0xFFFFFFFF, "error_callback"); + }, nullptr); + if (useCPUThread) { EmuThreadStart(); } else { @@ -795,6 +800,11 @@ extern "C" bool Java_org_ppsspp_ppsspp_NativeRenderer_displayInit(JNIEnv * env, SystemToast("Graphics initialization failed. Quitting."); return false; } + + graphicsContext->GetDrawContext()->SetErrorCallback([](const char *shortDesc, const char *details, void *userdata) { + host->NotifyUserMessage(details, 5.0, 0xFFFFFFFF, "error_callback"); + }, nullptr); + graphicsContext->ThreadStart(); renderer_inited = true; } diff --git a/ios/ViewController.mm b/ios/ViewController.mm index 02b18a5ec55b..1f665e4b37ef 100644 --- a/ios/ViewController.mm +++ b/ios/ViewController.mm @@ -34,6 +34,7 @@ #include "Core/System.h" #include "Core/HLE/sceUsbCam.h" #include "Core/HLE/sceUsbGps.h" +#include "Core/Host.h" #include #include @@ -202,6 +203,10 @@ - (void)viewDidLoad { graphicsContext = new IOSGraphicsContext(); + graphicsContext->GetDrawContext()->SetErrorCallback([](const char *shortDesc, const char *details, void *userdata) { + host->NotifyUserMessage(details, 5.0, 0xFFFFFFFF, "error_callback"); + }, nullptr); + graphicsContext->ThreadStart(); dp_xscale = (float)dp_xres / (float)pixel_xres;