Show an error on screen if a shader fails to compile.

Part of #1 investigation of #13541
hrydgard · Nov 5, 2020 · a5af0e3 · a5af0e3
1 parent 28da995
commit a5af0e3
Show file tree

Hide file tree

Showing 13 changed files with 64 additions and 10 deletions.
diff --git 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
@@ -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
@@ -277,21 +277,26 @@ void GLQueueRunner::RunInitSteps(const std::vector<GLRInitStep> &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
@@ -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<GLRInitStep> &steps, bool skipGLCalls);
 
 	void RunSteps(const std::vector<GLRStep *> &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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -34,6 +34,7 @@
 #include "Core/System.h"
 #include "Core/HLE/sceUsbCam.h"
 #include "Core/HLE/sceUsbGps.h"
+#include "Core/Host.h"
 
 #include <sys/types.h>
 #include <sys/sysctl.h>
@@ -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;