Merge pull request #17442 from hrydgard/glr-render-command-shrink

Shrink the GLRRenderCommand struct from 152 to 88 bytes
hrydgard · May 16, 2023 · c729519 · c729519
2 parents acea6de + f593d65
commit c729519
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 17 deletions.
diff --git a/Common/GPU/OpenGL/GLQueueRunner.cpp b/Common/GPU/OpenGL/GLQueueRunner.cpp
@@ -1038,28 +1038,34 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step, bool first, bool last
 		{
 			_dbg_assert_(curProgram);
 			if (IsMultiviewSupported()) {
-				int layout = GetStereoBufferIndex(c.uniformMatrix4.name);
+				int layout = GetStereoBufferIndex(c.uniformStereoMatrix4.name);
 				if (layout >= 0) {
 					int size = 2 * 16 * sizeof(float);
-					glBindBufferBase(GL_UNIFORM_BUFFER, layout, *c.uniformMatrix4.loc);
-					glBindBuffer(GL_UNIFORM_BUFFER, *c.uniformMatrix4.loc);
+					glBindBufferBase(GL_UNIFORM_BUFFER, layout, *c.uniformStereoMatrix4.loc);
+					glBindBuffer(GL_UNIFORM_BUFFER, *c.uniformStereoMatrix4.loc);
 					void *matrices = glMapBufferRange(GL_UNIFORM_BUFFER, 0, size, GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT);
-					memcpy(matrices, c.uniformMatrix4.m, size);
+					memcpy(matrices, c.uniformStereoMatrix4.mData, size);
 					glUnmapBuffer(GL_UNIFORM_BUFFER);
 					glBindBuffer(GL_UNIFORM_BUFFER, 0);
 				}
+				delete[] c.uniformStereoMatrix4.mData;  // We only playback once.
 			} else {
-				int loc = c.uniformMatrix4.loc ? *c.uniformMatrix4.loc : -1;
-				if (c.uniformMatrix4.name) {
-					loc = curProgram->GetUniformLoc(c.uniformMatrix4.name);
+				int loc = c.uniformStereoMatrix4.loc ? *c.uniformStereoMatrix4.loc : -1;
+				if (c.uniformStereoMatrix4.name) {
+					loc = curProgram->GetUniformLoc(c.uniformStereoMatrix4.name);
 				}
 				if (loc >= 0) {
 					if (GetVRFBOIndex() == 0) {
-						glUniformMatrix4fv(loc, 1, false, c.uniformMatrix4.m);
+						glUniformMatrix4fv(loc, 1, false, c.uniformStereoMatrix4.mData);
 					} else {
-						glUniformMatrix4fv(loc, 1, false, &c.uniformMatrix4.m[16]);
+						glUniformMatrix4fv(loc, 1, false, c.uniformStereoMatrix4.mData + 16);
 					}
 				}
+				if (GetVRFBOIndex() == 1 || GetVRPassesCount() == 1) {
+					// Only delete the data if we're rendering the only or the second eye.
+					// If we delete during the first eye, we get a use-after-free or double delete.
+					delete[] c.uniformStereoMatrix4.mData;
+				}
 			}
 			CHECK_GL_ERROR_IF_DEBUG();
 			break;

diff --git a/Common/GPU/OpenGL/GLQueueRunner.h b/Common/GPU/OpenGL/GLQueueRunner.h
@@ -25,7 +25,7 @@ struct GLOffset2D {
 	int x, y;
 };
 
-enum class GLRAllocType {
+enum class GLRAllocType : uint8_t {
 	NONE,
 	NEW,
 	ALIGNED,
@@ -127,8 +127,13 @@ struct GLRRenderData {
 		struct {
 			const char *name;  // if null, use loc
 			const GLint *loc;
-			float m[32];
+			float m[16];
 		} uniformMatrix4;
+		struct {
+			const char *name;  // if null, use loc
+			const GLint *loc;
+			float *mData;  // new'd, 32 entries
+		} uniformStereoMatrix4;
 		struct {
 			uint32_t clearColor;
 			float clearZ;

diff --git a/Common/GPU/OpenGL/GLRenderManager.cpp b/Common/GPU/OpenGL/GLRenderManager.cpp
@@ -41,6 +41,11 @@ GLRTexture::~GLRTexture() {
 	}
 }
 
+GLRenderManager::GLRenderManager() {
+	// size_t sz = sizeof(GLRRenderData);
+	// _dbg_assert_(sz == 88);
+}
+
 GLRenderManager::~GLRenderManager() {
 	_dbg_assert_(!run_);
 

diff --git a/Common/GPU/OpenGL/GLRenderManager.h b/Common/GPU/OpenGL/GLRenderManager.h
@@ -390,10 +390,9 @@ struct GLRRenderThreadTask {
 // directly in the destructor.
 class GLRenderManager {
 public:
-	GLRenderManager() {}
+	GLRenderManager();
 	~GLRenderManager();
 
-
 	void SetInvalidationCallback(InvalidationCallback callback) {
 		invalidationCallback_ = callback;
 	}
@@ -765,10 +764,11 @@ class GLRenderManager {
 		_dbg_assert_(curProgram_);
 #endif
 		GLRRenderData data{ GLRRenderCommand::UNIFORMSTEREOMATRIX };
-		data.uniformMatrix4.name = name;
-		data.uniformMatrix4.loc = loc;
-		memcpy(&data.uniformMatrix4.m[0], left, sizeof(float) * 16);
-		memcpy(&data.uniformMatrix4.m[16], right, sizeof(float) * 16);
+		data.uniformStereoMatrix4.name = name;
+		data.uniformStereoMatrix4.loc = loc;
+		data.uniformStereoMatrix4.mData = new float[32];
+		memcpy(&data.uniformStereoMatrix4.mData[0], left, sizeof(float) * 16);
+		memcpy(&data.uniformStereoMatrix4.mData[16], right, sizeof(float) * 16);
 		curRenderStep_->commands.push_back(data);
 	}