[Scummvm-git-logs] scummvm master -> c13961362f8f32f3e75cb03df7cf1b9845c39c9e

ccawley2011 noreply at scummvm.org
Wed Jun 29 21:37:42 UTC 2022


This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
863988fee4 OPENGL: Add proper error checking to the Shader class
c13961362f OPENGL: Ensure that glGetError() is cleared before calling a checked function


Commit: 863988fee4c0c9d70a2d0ac7919ab6d8ab0fdbd3
    https://github.com/scummvm/scummvm/commit/863988fee4c0c9d70a2d0ac7919ab6d8ab0fdbd3
Author: Cameron Cawley (ccawley2011 at gmail.com)
Date: 2022-06-29T22:37:38+01:00

Commit Message:
OPENGL: Add proper error checking to the Shader class

Changed paths:
  A graphics/opengl/debug.cpp
  A graphics/opengl/debug.h
  R backends/graphics/opengl/debug.cpp
  R backends/graphics/opengl/debug.h
    backends/graphics/opengl/framebuffer.cpp
    backends/graphics/opengl/opengl-graphics.cpp
    backends/graphics/opengl/pipelines/clut8.cpp
    backends/graphics/opengl/pipelines/fixed.cpp
    backends/graphics/opengl/pipelines/shader.cpp
    backends/graphics/opengl/shader.cpp
    backends/graphics/opengl/texture.cpp
    backends/module.mk
    graphics/module.mk
    graphics/opengl/shader.cpp
    graphics/opengl/shader.h


diff --git a/backends/graphics/opengl/framebuffer.cpp b/backends/graphics/opengl/framebuffer.cpp
index a8e4f758ddb..1daa2d02eab 100644
--- a/backends/graphics/opengl/framebuffer.cpp
+++ b/backends/graphics/opengl/framebuffer.cpp
@@ -19,10 +19,10 @@
  *
  */
 
-#include "backends/graphics/opengl/debug.h"
 #include "backends/graphics/opengl/framebuffer.h"
 #include "backends/graphics/opengl/texture.h"
 #include "backends/graphics/opengl/pipelines/pipeline.h"
+#include "graphics/opengl/debug.h"
 
 namespace OpenGL {
 
diff --git a/backends/graphics/opengl/opengl-graphics.cpp b/backends/graphics/opengl/opengl-graphics.cpp
index 7552f605ad0..6c7aaf2fa1a 100644
--- a/backends/graphics/opengl/opengl-graphics.cpp
+++ b/backends/graphics/opengl/opengl-graphics.cpp
@@ -21,12 +21,12 @@
 
 
 #include "backends/graphics/opengl/opengl-graphics.h"
-#include "backends/graphics/opengl/debug.h"
 #include "backends/graphics/opengl/texture.h"
 #include "backends/graphics/opengl/pipelines/pipeline.h"
 #include "backends/graphics/opengl/pipelines/fixed.h"
 #include "backends/graphics/opengl/pipelines/shader.h"
 #include "backends/graphics/opengl/shader.h"
+#include "graphics/opengl/debug.h"
 
 #include "common/array.h"
 #include "common/textconsole.h"
diff --git a/backends/graphics/opengl/pipelines/clut8.cpp b/backends/graphics/opengl/pipelines/clut8.cpp
index dc4732e4f27..0d8a587847b 100644
--- a/backends/graphics/opengl/pipelines/clut8.cpp
+++ b/backends/graphics/opengl/pipelines/clut8.cpp
@@ -20,9 +20,9 @@
  */
 
 #include "backends/graphics/opengl/pipelines/clut8.h"
-#include "backends/graphics/opengl/debug.h"
 #include "backends/graphics/opengl/shader.h"
 #include "backends/graphics/opengl/framebuffer.h"
+#include "graphics/opengl/debug.h"
 
 namespace OpenGL {
 
diff --git a/backends/graphics/opengl/pipelines/fixed.cpp b/backends/graphics/opengl/pipelines/fixed.cpp
index edf4ce6ddc7..ab30d05d927 100644
--- a/backends/graphics/opengl/pipelines/fixed.cpp
+++ b/backends/graphics/opengl/pipelines/fixed.cpp
@@ -20,7 +20,7 @@
  */
 
 #include "backends/graphics/opengl/pipelines/fixed.h"
-#include "backends/graphics/opengl/debug.h"
+#include "graphics/opengl/debug.h"
 
 namespace OpenGL {
 
diff --git a/backends/graphics/opengl/pipelines/shader.cpp b/backends/graphics/opengl/pipelines/shader.cpp
index e2385bffcaf..6d0c3f14eb0 100644
--- a/backends/graphics/opengl/pipelines/shader.cpp
+++ b/backends/graphics/opengl/pipelines/shader.cpp
@@ -20,9 +20,9 @@
  */
 
 #include "backends/graphics/opengl/pipelines/shader.h"
-#include "backends/graphics/opengl/debug.h"
 #include "backends/graphics/opengl/shader.h"
 #include "backends/graphics/opengl/framebuffer.h"
+#include "graphics/opengl/debug.h"
 
 namespace OpenGL {
 
diff --git a/backends/graphics/opengl/shader.cpp b/backends/graphics/opengl/shader.cpp
index 2d789f8ed1e..71cdf544a88 100644
--- a/backends/graphics/opengl/shader.cpp
+++ b/backends/graphics/opengl/shader.cpp
@@ -20,7 +20,7 @@
  */
 
 #include "backends/graphics/opengl/shader.h"
-#include "backends/graphics/opengl/debug.h"
+#include "graphics/opengl/debug.h"
 
 #if !USE_FORCED_GLES
 
diff --git a/backends/graphics/opengl/texture.cpp b/backends/graphics/opengl/texture.cpp
index 01244be5401..e525b6c323b 100644
--- a/backends/graphics/opengl/texture.cpp
+++ b/backends/graphics/opengl/texture.cpp
@@ -20,11 +20,11 @@
  */
 
 #include "backends/graphics/opengl/texture.h"
-#include "backends/graphics/opengl/debug.h"
 #include "backends/graphics/opengl/shader.h"
 #include "backends/graphics/opengl/pipelines/pipeline.h"
 #include "backends/graphics/opengl/pipelines/clut8.h"
 #include "backends/graphics/opengl/framebuffer.h"
+#include "graphics/opengl/debug.h"
 
 #include "common/algorithm.h"
 #include "common/endian.h"
diff --git a/backends/module.mk b/backends/module.mk
index e53150eff34..91e500e01f6 100644
--- a/backends/module.mk
+++ b/backends/module.mk
@@ -123,7 +123,6 @@ endif
 # OpenGL specific source files.
 ifdef USE_OPENGL
 MODULE_OBJS += \
-	graphics/opengl/debug.o \
 	graphics/opengl/framebuffer.o \
 	graphics/opengl/opengl-graphics.o \
 	graphics/opengl/shader.o \
diff --git a/graphics/module.mk b/graphics/module.mk
index 40369cad9f2..732239c8a2c 100644
--- a/graphics/module.mk
+++ b/graphics/module.mk
@@ -31,6 +31,7 @@ MODULE_OBJS := \
 	managed_surface.o \
 	nine_patch.o \
 	opengl/context.o \
+	opengl/debug.o \
 	opengl/shader.o \
 	pixelformat.o \
 	primitives.o \
diff --git a/backends/graphics/opengl/debug.cpp b/graphics/opengl/debug.cpp
similarity index 96%
rename from backends/graphics/opengl/debug.cpp
rename to graphics/opengl/debug.cpp
index 91f11862596..8ea8afcdadc 100644
--- a/backends/graphics/opengl/debug.cpp
+++ b/graphics/opengl/debug.cpp
@@ -19,13 +19,15 @@
  *
  */
 
-#include "backends/graphics/opengl/debug.h"
+#include "graphics/opengl/debug.h"
 
 #include "common/str.h"
 #include "common/textconsole.h"
 
 #include "graphics/opengl/system_headers.h"
 
+#if defined(USE_OPENGL)
+
 #ifdef OPENGL_DEBUG
 
 namespace OpenGL {
@@ -65,3 +67,5 @@ void checkGLError(const char *expr, const char *file, int line) {
 } // End of namespace OpenGL
 
 #endif
+
+#endif
diff --git a/backends/graphics/opengl/debug.h b/graphics/opengl/debug.h
similarity index 100%
rename from backends/graphics/opengl/debug.h
rename to graphics/opengl/debug.h
diff --git a/graphics/opengl/shader.cpp b/graphics/opengl/shader.cpp
index cadd2bdf683..2ef2f2a8fe2 100644
--- a/graphics/opengl/shader.cpp
+++ b/graphics/opengl/shader.cpp
@@ -117,17 +117,18 @@ static const GLchar *readFile(const Common::String &filename) {
 }
 
 static GLuint createDirectShader(const char *shaderSource, GLenum shaderType, const Common::String &name) {
-	GLuint shader = glCreateShader(shaderType);
-	glShaderSource(shader, 1, &shaderSource, NULL);
-	glCompileShader(shader);
+	GLuint shader;
+	GL_ASSIGN(shader, glCreateShader(shaderType));
+	GL_CALL(glShaderSource(shader, 1, &shaderSource, NULL));
+	GL_CALL(glCompileShader(shader));
 
 	GLint status;
-	glGetShaderiv(shader, GL_COMPILE_STATUS, &status);
+	GL_CALL(glGetShaderiv(shader, GL_COMPILE_STATUS, &status));
 	if (status != GL_TRUE) {
 		GLint logSize;
-		glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &logSize);
+		GL_CALL(glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &logSize));
 		GLchar *log = new GLchar[logSize];
-		glGetShaderInfoLog(shader, logSize, nullptr, log);
+		GL_CALL(glGetShaderInfoLog(shader, logSize, nullptr, log));
 		error("Could not compile shader %s: %s", name.c_str(), log);
 	}
 
@@ -173,17 +174,18 @@ static GLuint createCompatShader(const char *shaderSource, GLenum shaderType, co
 		shaderSource
 	};
 
-	GLuint shader = glCreateShader(shaderType);
-	glShaderSource(shader, 4, shaderSources, NULL);
-	glCompileShader(shader);
+	GLuint shader;
+	GL_ASSIGN(shader, glCreateShader(shaderType));
+	GL_CALL(glShaderSource(shader, 4, shaderSources, NULL));
+	GL_CALL(glCompileShader(shader));
 
 	GLint status;
-	glGetShaderiv(shader, GL_COMPILE_STATUS, &status);
+	GL_CALL(glGetShaderiv(shader, GL_COMPILE_STATUS, &status));
 	if (status != GL_TRUE) {
 		GLint logSize;
-		glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &logSize);
+		GL_CALL(glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &logSize));
 		GLchar *log = new GLchar[logSize];
-		glGetShaderInfoLog(shader, logSize, nullptr, log);
+		GL_CALL(glGetShaderInfoLog(shader, logSize, nullptr, log));
 		error("Could not compile shader %s: %s", name.c_str(), log);
 	}
 
@@ -212,7 +214,7 @@ static GLuint loadShaderFromFile(const char *base, const char *extension, GLenum
 struct SharedPtrProgramDeleter {
 	void operator()(GLuint *ptr) {
 		if (ptr) {
-			glDeleteProgram(*ptr);
+			GL_CALL(glDeleteProgram(*ptr));
 		}
 		delete ptr;
 	}
@@ -225,31 +227,32 @@ uint32 Shader::previousNumAttributes = 0;
 Shader::Shader(const Common::String &name, GLuint vertexShader, GLuint fragmentShader, const char *const *attributes)
 	: _name(name) {
 	assert(attributes);
-	GLuint shaderProgram = glCreateProgram();
-	glAttachShader(shaderProgram, vertexShader);
-	glAttachShader(shaderProgram, fragmentShader);
+	GLuint shaderProgram;
+	GL_ASSIGN(shaderProgram, glCreateProgram());
+	GL_CALL(glAttachShader(shaderProgram, vertexShader));
+	GL_CALL(glAttachShader(shaderProgram, fragmentShader));
 
 	for (int idx = 0; attributes[idx]; ++idx) {
-		glBindAttribLocation(shaderProgram, idx, attributes[idx]);
+		GL_CALL(glBindAttribLocation(shaderProgram, idx, attributes[idx]));
 		_attributes.push_back(VertexAttrib(idx, attributes[idx]));
 	}
-	glLinkProgram(shaderProgram);
+	GL_CALL(glLinkProgram(shaderProgram));
 
 	GLint status;
-	glGetProgramiv(shaderProgram, GL_LINK_STATUS, &status);
+	GL_CALL(glGetProgramiv(shaderProgram, GL_LINK_STATUS, &status));
 	if (status != GL_TRUE) {
 		GLint logSize;
-		glGetProgramiv(shaderProgram, GL_INFO_LOG_LENGTH, &logSize);
+		GL_CALL(glGetProgramiv(shaderProgram, GL_INFO_LOG_LENGTH, &logSize));
 		GLchar *log = new GLchar[logSize];
-		glGetProgramInfoLog(shaderProgram, logSize, nullptr, log);
+		GL_CALL(glGetProgramInfoLog(shaderProgram, logSize, nullptr, log));
 		error("Could not link shader %s: %s", name.c_str(), log);
 	}
 
-	glDetachShader(shaderProgram, vertexShader);
-	glDetachShader(shaderProgram, fragmentShader);
+	GL_CALL(glDetachShader(shaderProgram, vertexShader));
+	GL_CALL(glDetachShader(shaderProgram, fragmentShader));
 
-	glDeleteShader(vertexShader);
-	glDeleteShader(fragmentShader);
+	GL_CALL(glDeleteShader(vertexShader));
+	GL_CALL(glDeleteShader(fragmentShader));
 
 	_shaderNo = Common::SharedPtr<GLuint>(new GLuint(shaderProgram), SharedPtrProgramDeleter());
 	_uniforms = Common::SharedPtr<UniformsMap>(new UniformsMap());
@@ -283,49 +286,49 @@ void Shader::use(bool forceReload) {
 	// The previous shader might have had more attributes. Disable any extra ones.
 	if (_attributes.size() < previousNumAttributes) {
 		for (uint32 i = _attributes.size(); i < previousNumAttributes; ++i) {
-			glDisableVertexAttribArray(i);
+			GL_CALL(glDisableVertexAttribArray(i));
 		}
 	}
 
 	_previousShader = this;
 	previousNumAttributes = _attributes.size();
 
-	glUseProgram(*_shaderNo);
+	GL_CALL(glUseProgram(*_shaderNo));
 	for (uint32 i = 0; i < _attributes.size(); ++i) {
 		VertexAttrib &attrib = _attributes[i];
 		if (attrib._enabled) {
-			glEnableVertexAttribArray(i);
-			glBindBuffer(GL_ARRAY_BUFFER, attrib._vbo);
-			glVertexAttribPointer(i, attrib._size, attrib._type, attrib._normalized, attrib._stride, attrib._pointer);
+			GL_CALL(glEnableVertexAttribArray(i));
+			GL_CALL(glBindBuffer(GL_ARRAY_BUFFER, attrib._vbo));
+			GL_CALL(glVertexAttribPointer(i, attrib._size, attrib._type, attrib._normalized, attrib._stride, attrib._pointer));
 		} else {
-			glDisableVertexAttribArray(i);
+			GL_CALL(glDisableVertexAttribArray(i));
 			switch (attrib._size) {
 			case 2:
-				glVertexAttrib2fv(i, attrib._const);
+				GL_CALL(glVertexAttrib2fv(i, attrib._const));
 				break;
 			case 3:
-				glVertexAttrib3fv(i, attrib._const);
+				GL_CALL(glVertexAttrib3fv(i, attrib._const));
 				break;
 			case 4:
-				glVertexAttrib4fv(i, attrib._const);
+				GL_CALL(glVertexAttrib4fv(i, attrib._const));
 				break;
 			}
 		}
 	}
-	glBindBuffer(GL_ARRAY_BUFFER, 0);
+	GL_CALL(glBindBuffer(GL_ARRAY_BUFFER, 0));
 }
 
 GLuint Shader::createBuffer(GLenum target, GLsizeiptr size, const GLvoid *data, GLenum usage) {
 	GLuint vbo;
-	glGenBuffers(1, &vbo);
-	glBindBuffer(target, vbo);
-	glBufferData(target, size, data, usage);
-	glBindBuffer(GL_ARRAY_BUFFER, 0);
+	GL_CALL(glGenBuffers(1, &vbo));
+	GL_CALL(glBindBuffer(target, vbo));
+	GL_CALL(glBufferData(target, size, data, usage));
+	GL_CALL(glBindBuffer(GL_ARRAY_BUFFER, 0));
 	return vbo;
 }
 
 void Shader::freeBuffer(GLuint vbo) {
-	glDeleteBuffers(1, &vbo);
+	GL_CALL(glDeleteBuffers(1, &vbo));
 }
 
 VertexAttrib &Shader::getAttributeAt(uint32 idx) {
@@ -372,12 +375,12 @@ void Shader::disableVertexAttribute(const char *attrib, int size, const float *d
 }
 
 void Shader::unbind() {
-	glUseProgram(0);
+	GL_CALL(glUseProgram(0));
 	_previousShader = nullptr;
 
 	// Disable all vertex attributes as well
 	for (uint32 i = 0; i < previousNumAttributes; ++i) {
-		glDisableVertexAttribArray(i);
+		GL_CALL(glDisableVertexAttribArray(i));
 	}
 	previousNumAttributes = 0;
 }
diff --git a/graphics/opengl/shader.h b/graphics/opengl/shader.h
index 4e1d2fe8d20..c580c95c997 100644
--- a/graphics/opengl/shader.h
+++ b/graphics/opengl/shader.h
@@ -32,6 +32,7 @@
 #include "math/vector3d.h"
 #include "math/vector4d.h"
 
+#include "graphics/opengl/debug.h"
 #include "graphics/opengl/system_headers.h"
 
 namespace OpenGL {
@@ -67,7 +68,7 @@ public:
 		GLint pos = getUniformLocation(uniform);
 		if (pos != -1) {
 			use();
-			glUniformMatrix4fv(pos, 1, GL_FALSE, m.getData());
+			GL_CALL(glUniformMatrix4fv(pos, 1, GL_FALSE, m.getData()));
 			return true;
 		} else {
 			return false;
@@ -78,7 +79,7 @@ public:
 		GLint pos = getUniformLocation(uniform);
 		if (pos != -1) {
 			use();
-			glUniformMatrix3fv(pos, 1, GL_FALSE, m.getData());
+			GL_CALL(glUniformMatrix3fv(pos, 1, GL_FALSE, m.getData()));
 			return true;
 		} else {
 			return false;
@@ -89,7 +90,7 @@ public:
 		GLint pos = getUniformLocation(uniform);
 		if (pos != -1) {
 			use();
-			glUniform4fv(pos, 1, v.getData());
+			GL_CALL(glUniform4fv(pos, 1, v.getData()));
 			return true;
 		} else {
 			return false;
@@ -100,7 +101,7 @@ public:
 		GLint pos = getUniformLocation(uniform);
 		if (pos != -1) {
 			use();
-			glUniform3fv(pos, 1, v.getData());
+			GL_CALL(glUniform3fv(pos, 1, v.getData()));
 			return true;
 		} else {
 			return false;
@@ -111,7 +112,7 @@ public:
 		GLint pos = getUniformLocation(uniform);
 		if (pos != -1) {
 			use();
-			glUniform2fv(pos, 1, v.getData());
+			GL_CALL(glUniform2fv(pos, 1, v.getData()));
 			return true;
 		} else {
 			return false;
@@ -122,7 +123,7 @@ public:
 		GLint pos = getUniformLocation(uniform);
 		if (pos != -1) {
 			use();
-			glUniform1i(pos, x);
+			GL_CALL(glUniform1i(pos, x));
 			return true;
 		} else {
 			return false;
@@ -134,7 +135,7 @@ public:
 		GLint pos = getUniformLocation(uniform);
 		if (pos != -1) {
 			use();
-			glUniform1f(pos, f);
+			GL_CALL(glUniform1f(pos, f));
 			return true;
 		} else {
 			return false;
@@ -144,7 +145,8 @@ public:
 	GLint getUniformLocation(const Common::String &uniform) const {
 		UniformsMap::iterator kv = _uniforms->find(uniform);
 		if (kv == _uniforms->end()) {
-			GLint ret = glGetUniformLocation(*_shaderNo, uniform.c_str());
+			GLint ret;
+			GL_ASSIGN(ret, glGetUniformLocation(*_shaderNo, uniform.c_str()));
 			_uniforms->setVal(uniform, ret);
 			return ret;
 		} else {


Commit: c13961362f8f32f3e75cb03df7cf1b9845c39c9e
    https://github.com/scummvm/scummvm/commit/c13961362f8f32f3e75cb03df7cf1b9845c39c9e
Author: Cameron Cawley (ccawley2011 at gmail.com)
Date: 2022-06-29T22:37:38+01:00

Commit Message:
OPENGL: Ensure that glGetError() is cleared before calling a checked function

Changed paths:
    graphics/opengl/debug.cpp
    graphics/opengl/debug.h


diff --git a/graphics/opengl/debug.cpp b/graphics/opengl/debug.cpp
index 8ea8afcdadc..c32c4e720dc 100644
--- a/graphics/opengl/debug.cpp
+++ b/graphics/opengl/debug.cpp
@@ -55,6 +55,13 @@ Common::String getGLErrStr(GLenum error) {
 }
 } // End of anonymous namespace
 
+void clearGLError() {
+	GLenum error;
+
+	while ((error = glGetError()) != GL_NO_ERROR)
+		;
+}
+
 void checkGLError(const char *expr, const char *file, int line) {
 	GLenum error;
 
diff --git a/graphics/opengl/debug.h b/graphics/opengl/debug.h
index e2d3cc2d2a4..0a567bd9c1f 100644
--- a/graphics/opengl/debug.h
+++ b/graphics/opengl/debug.h
@@ -29,10 +29,11 @@
 #ifdef OPENGL_DEBUG
 
 namespace OpenGL {
+void clearGLError();
 void checkGLError(const char *expr, const char *file, int line);
 } // End of namespace OpenGL
 
-#define GL_WRAP_DEBUG(call, name) do { (call); OpenGL::checkGLError(#name, __FILE__, __LINE__); } while (false)
+#define GL_WRAP_DEBUG(call, name) do { OpenGL::clearGLError(); (call); OpenGL::checkGLError(#name, __FILE__, __LINE__); } while (false)
 #else
 #define GL_WRAP_DEBUG(call, name) do { (call); } while (false)
 #endif




More information about the Scummvm-git-logs mailing list