Merge pull request #16919 from hrydgard/gl-delayed-readback-prep

Move GLFrameData out of GLRenderManager.

CMakeLists.txt

@@ -632,6 +632,8 @@ add_library(Common STATIC
 	Common/GPU/OpenGL/gl3stub.h
 	Common/GPU/OpenGL/GLFeatures.cpp
 	Common/GPU/OpenGL/GLFeatures.h
+	Common/GPU/OpenGL/GLFrameData.cpp
+	Common/GPU/OpenGL/GLFrameData.h
 	Common/GPU/OpenGL/thin3d_gl.cpp
 	Common/GPU/OpenGL/GLRenderManager.cpp
 	Common/GPU/OpenGL/GLRenderManager.h

Common/Common.vcxproj

@@ -436,6 +436,7 @@
     <ClInclude Include="GPU\OpenGL\DataFormatGL.h" />
     <ClInclude Include="GPU\OpenGL\gl3stub.h" />
     <ClInclude Include="GPU\OpenGL\GLFeatures.h" />
+    <ClInclude Include="GPU\OpenGL\GLFrameData.h" />
     <ClInclude Include="GPU\OpenGL\GLQueueRunner.h" />
     <ClInclude Include="GPU\OpenGL\GLRenderManager.h" />
     <ClInclude Include="GPU\OpenGL\GLSLProgram.h" />
@@ -871,6 +872,7 @@
     <ClCompile Include="GPU\OpenGL\DataFormatGL.cpp" />
     <ClCompile Include="GPU\OpenGL\gl3stub.c" />
     <ClCompile Include="GPU\OpenGL\GLFeatures.cpp" />
+    <ClCompile Include="GPU\OpenGL\GLFrameData.cpp" />
     <ClCompile Include="GPU\OpenGL\GLQueueRunner.cpp" />
     <ClCompile Include="GPU\OpenGL\GLRenderManager.cpp" />
     <ClCompile Include="GPU\OpenGL\GLSLProgram.cpp" />

Common/Common.vcxproj.filters

@@ -464,6 +464,9 @@
     <ClInclude Include="UI\PopupScreens.h">
       <Filter>UI</Filter>
     </ClInclude>
+    <ClInclude Include="GPU\OpenGL\GLFrameData.h">
+      <Filter>GPU\OpenGL</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="ABI.cpp" />
@@ -878,6 +881,9 @@
     <ClCompile Include="UI\PopupScreens.cpp">
       <Filter>UI</Filter>
     </ClCompile>
+    <ClCompile Include="GPU\OpenGL\GLFrameData.cpp">
+      <Filter>GPU\OpenGL</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <Filter Include="Crypto">

Common/GPU/OpenGL/GLFrameData.cpp

@@ -0,0 +1,75 @@
+#include "Common/GPU/OpenGL/GLCommon.h"
+#include "Common/GPU/OpenGL/GLFrameData.h"
+#include "Common/GPU/OpenGL/GLRenderManager.h"
+#include "Common/Log.h"
+
+void GLDeleter::Take(GLDeleter &other) {
+	_assert_msg_(IsEmpty(), "Deleter already has stuff");
+	shaders = std::move(other.shaders);
+	programs = std::move(other.programs);
+	buffers = std::move(other.buffers);
+	textures = std::move(other.textures);
+	inputLayouts = std::move(other.inputLayouts);
+	framebuffers = std::move(other.framebuffers);
+	pushBuffers = std::move(other.pushBuffers);
+	other.shaders.clear();
+	other.programs.clear();
+	other.buffers.clear();
+	other.textures.clear();
+	other.inputLayouts.clear();
+	other.framebuffers.clear();
+	other.pushBuffers.clear();
+}
+
+// Runs on the GPU thread.
+void GLDeleter::Perform(GLRenderManager *renderManager, bool skipGLCalls) {
+	for (auto pushBuffer : pushBuffers) {
+		renderManager->UnregisterPushBuffer(pushBuffer);
+		if (skipGLCalls) {
+			pushBuffer->Destroy(false);
+		}
+		delete pushBuffer;
+	}
+	pushBuffers.clear();
+	for (auto shader : shaders) {
+		if (skipGLCalls)
+			shader->shader = 0;  // prevent the glDeleteShader
+		delete shader;
+	}
+	shaders.clear();
+	for (auto program : programs) {
+		if (skipGLCalls)
+			program->program = 0;  // prevent the glDeleteProgram
+		delete program;
+	}
+	programs.clear();
+	for (auto buffer : buffers) {
+		if (skipGLCalls)
+			buffer->buffer_ = 0;
+		delete buffer;
+	}
+	buffers.clear();
+	for (auto texture : textures) {
+		if (skipGLCalls)
+			texture->texture = 0;
+		delete texture;
+	}
+	textures.clear();
+	for (auto inputLayout : inputLayouts) {
+		// No GL objects in an inputLayout yet
+		delete inputLayout;
+	}
+	inputLayouts.clear();
+	for (auto framebuffer : framebuffers) {
+		if (skipGLCalls) {
+			framebuffer->handle = 0;
+			framebuffer->color_texture.texture = 0;
+			framebuffer->z_stencil_buffer = 0;
+			framebuffer->z_stencil_texture.texture = 0;
+			framebuffer->z_buffer = 0;
+			framebuffer->stencil_buffer = 0;
+		}
+		delete framebuffer;
+	}
+	framebuffers.clear();
+}

Common/GPU/OpenGL/GLFrameData.h

@@ -0,0 +1,52 @@
+#pragma once
+
+#include <mutex>
+#include <condition_variable>
+#include <vector>
+#include <set>
+
+#include "Common/GPU/OpenGL/GLCommon.h"
+
+class GLRShader;
+class GLRBuffer;
+class GLRTexture;
+class GLRInputLayout;
+class GLRFramebuffer;
+class GLPushBuffer;
+class GLRProgram;
+class GLRenderManager;
+
+class GLDeleter {
+public:
+	void Perform(GLRenderManager *renderManager, bool skipGLCalls);
+
+	bool IsEmpty() const {
+		return shaders.empty() && programs.empty() && buffers.empty() && textures.empty() && inputLayouts.empty() && framebuffers.empty() && pushBuffers.empty();
+	}
+
+	void Take(GLDeleter &other);
+
+	std::vector<GLRShader *> shaders;
+	std::vector<GLRProgram *> programs;
+	std::vector<GLRBuffer *> buffers;
+	std::vector<GLRTexture *> textures;
+	std::vector<GLRInputLayout *> inputLayouts;
+	std::vector<GLRFramebuffer *> framebuffers;
+	std::vector<GLPushBuffer *> pushBuffers;
+};
+
+// Per-frame data, round-robin so we can overlap submission with execution of the previous frame.
+struct GLFrameData {
+	bool skipSwap = false;
+
+	std::mutex fenceMutex;
+	std::condition_variable fenceCondVar;
+	bool readyForFence = true;
+
+	// Swapchain.
+	bool hasBegun = false;
+
+	GLDeleter deleter;
+	GLDeleter deleter_prev;
+	std::set<GLPushBuffer *> activePushBuffers;
+};

Common/GPU/OpenGL/GLQueueRunner.cpp

@@ -88,9 +88,6 @@ void GLQueueRunner::DestroyDeviceObjects() {
 	delete[] readbackBuffer_;
 	readbackBuffer_ = nullptr;
 	readbackBufferSize_ = 0;
-	delete[] tempBuffer_;
-	tempBuffer_ = nullptr;
-	tempBufferSize_ = 0;
 	CHECK_GL_ERROR_IF_DEBUG();
 }
 
@@ -1481,26 +1478,24 @@ void GLQueueRunner::PerformReadback(const GLRStep &pass) {
 	CHECK_GL_ERROR_IF_DEBUG();
 
 	// Always read back in 8888 format for the color aspect.
-	GLuint internalFormat = GL_RGBA;
 	GLuint format = GL_RGBA;
 	GLuint type = GL_UNSIGNED_BYTE;
 	int srcAlignment = 4;
-	int dstAlignment = (int)DataFormatSizeInBytes(pass.readback.dstFormat);
 
 #ifndef USING_GLES2
 	if (pass.readback.aspectMask & GL_DEPTH_BUFFER_BIT) {
-		internalFormat = GL_DEPTH_COMPONENT;
 		format = GL_DEPTH_COMPONENT;
 		type = GL_FLOAT;
 		srcAlignment = 4;
 	} else if (pass.readback.aspectMask & GL_STENCIL_BUFFER_BIT) {
-		internalFormat = GL_STENCIL_INDEX;
 		format = GL_STENCIL_INDEX;
 		type = GL_UNSIGNED_BYTE;
 		srcAlignment = 1;
 	}
 #endif
 
+	readbackAspectMask_ = pass.readback.aspectMask;
+
 	int pixelStride = pass.readback.srcRect.w;
 	// Apply the correct alignment.
 	glPixelStorei(GL_PACK_ALIGNMENT, srcAlignment);
@@ -1511,31 +1506,20 @@ void GLQueueRunner::PerformReadback(const GLRStep &pass) {
 
 	GLRect2D rect = pass.readback.srcRect;
 
-	bool convert = internalFormat == GL_RGBA && pass.readback.dstFormat != DataFormat::R8G8B8A8_UNORM;
-
-	int tempSize = srcAlignment * rect.w * rect.h;
-	int readbackSize = dstAlignment * rect.w * rect.h;
-	if (convert && tempSize > tempBufferSize_) {
-		delete[] tempBuffer_;
-		tempBuffer_ = new uint8_t[tempSize];
-		tempBufferSize_ = tempSize;
-	}
+	int readbackSize = srcAlignment * rect.w * rect.h;
 	if (readbackSize > readbackBufferSize_) {
 		delete[] readbackBuffer_;
 		readbackBuffer_ = new uint8_t[readbackSize];
 		readbackBufferSize_ = readbackSize;
 	}
 
-	glReadPixels(rect.x, rect.y, rect.w, rect.h, format, type, convert ? tempBuffer_ : readbackBuffer_);
+	glReadPixels(rect.x, rect.y, rect.w, rect.h, format, type, readbackBuffer_);
 	#ifdef DEBUG_READ_PIXELS
 	LogReadPixelsError(glGetError());
 	#endif
 	if (!gl_extensions.IsGLES || gl_extensions.GLES3) {
 		glPixelStorei(GL_PACK_ROW_LENGTH, 0);
 	}
-	if (convert && tempBuffer_ && readbackBuffer_) {
-		ConvertFromRGBA8888(readbackBuffer_, tempBuffer_, pixelStride, pixelStride, rect.w, rect.h, pass.readback.dstFormat);
-	}
 	CHECK_GL_ERROR_IF_DEBUG();
 }
 
@@ -1624,8 +1608,25 @@ void GLQueueRunner::CopyFromReadbackBuffer(GLRFramebuffer *framebuffer, int widt
 		// Something went wrong during the read and no readback buffer was allocated, probably.
 		return;
 	}
-	for (int y = 0; y < height; y++) {
-		memcpy(pixels + y * pixelStride * bpp, readbackBuffer_ + y * width * bpp, width * bpp);
+
+	// Always read back in 8888 format for the color aspect.
+	GLuint internalFormat = GL_RGBA;
+#ifndef USING_GLES2
+	if (readbackAspectMask_ & GL_DEPTH_BUFFER_BIT) {
+		internalFormat = GL_DEPTH_COMPONENT;
+	} else if (readbackAspectMask_ & GL_STENCIL_BUFFER_BIT) {
+		internalFormat = GL_STENCIL_INDEX;
+	}
+#endif
+
+	bool convert = internalFormat == GL_RGBA && destFormat != Draw::DataFormat::R8G8B8A8_UNORM;
+	if (convert) {
+		// srcStride is width because we read back "packed" (with no gaps) from GL.
+		ConvertFromRGBA8888(pixels, readbackBuffer_, pixelStride, width, width, height, destFormat);
+	} else {
+		for (int y = 0; y < height; y++) {
+			memcpy(pixels + y * pixelStride * bpp, readbackBuffer_ + y * width * bpp, width * bpp);
+		}
 	}
 }
 

Common/GPU/OpenGL/GLQueueRunner.h

@@ -6,6 +6,7 @@
 #include <unordered_map>
 
 #include "Common/GPU/OpenGL/GLCommon.h"
+#include "Common/GPU/OpenGL/GLFrameData.h"
 #include "Common/GPU/DataFormat.h"
 #include "Common/GPU/Shader.h"
 #include "Common/GPU/thin3d.h"
@@ -419,9 +420,7 @@ class GLQueueRunner {
 	// We size it generously.
 	uint8_t *readbackBuffer_ = nullptr;
 	int readbackBufferSize_ = 0;
-	// Temp buffer for color conversion
-	uint8_t *tempBuffer_ = nullptr;
-	int tempBufferSize_ = 0;
+	uint32_t readbackAspectMask_ = 0;
 
 	float maxAnisotropyLevel_ = 0.0f;
 

Common/GPU/OpenGL/GLRenderManager.cpp

@@ -41,77 +41,6 @@ GLRTexture::~GLRTexture() {
 	}
 }
 
-void GLDeleter::Take(GLDeleter &other) {
-	_assert_msg_(IsEmpty(), "Deleter already has stuff");
-	shaders = std::move(other.shaders);
-	programs = std::move(other.programs);
-	buffers = std::move(other.buffers);
-	textures = std::move(other.textures);
-	inputLayouts = std::move(other.inputLayouts);
-	framebuffers = std::move(other.framebuffers);
-	pushBuffers = std::move(other.pushBuffers);
-	other.shaders.clear();
-	other.programs.clear();
-	other.buffers.clear();
-	other.textures.clear();
-	other.inputLayouts.clear();
-	other.framebuffers.clear();
-	other.pushBuffers.clear();
-}
-
-// Runs on the GPU thread.
-void GLDeleter::Perform(GLRenderManager *renderManager, bool skipGLCalls) {
-	for (auto pushBuffer : pushBuffers) {
-		renderManager->UnregisterPushBuffer(pushBuffer);
-		if (skipGLCalls) {
-			pushBuffer->Destroy(false);
-		}
-		delete pushBuffer;
-	}
-	pushBuffers.clear();
-	for (auto shader : shaders) {
-		if (skipGLCalls)
-			shader->shader = 0;  // prevent the glDeleteShader
-		delete shader;
-	}
-	shaders.clear();
-	for (auto program : programs) {
-		if (skipGLCalls)
-			program->program = 0;  // prevent the glDeleteProgram
-		delete program;
-	}
-	programs.clear();
-	for (auto buffer : buffers) {
-		if (skipGLCalls)
-			buffer->buffer_ = 0;
-		delete buffer;
-	}
-	buffers.clear();
-	for (auto texture : textures) {
-		if (skipGLCalls)
-			texture->texture = 0;
-		delete texture;
-	}
-	textures.clear();
-	for (auto inputLayout : inputLayouts) {
-		// No GL objects in an inputLayout yet
-		delete inputLayout;
-	}
-	inputLayouts.clear();
-	for (auto framebuffer : framebuffers) {
-		if (skipGLCalls) {
-			framebuffer->handle = 0;
-			framebuffer->color_texture.texture = 0;
-			framebuffer->z_stencil_buffer = 0;
-			framebuffer->z_stencil_texture.texture = 0;
-			framebuffer->z_buffer = 0;
-			framebuffer->stencil_buffer = 0;
-		}
-		delete framebuffer;
-	}
-	framebuffers.clear();
-}
-
 GLRenderManager::~GLRenderManager() {
 	_dbg_assert_(!run_);
 
@@ -414,7 +343,7 @@ void GLRenderManager::BeginFrame() {
 
 	int curFrame = GetCurFrame();
 
-	FrameData &frameData = frameData_[curFrame];
+	GLFrameData &frameData = frameData_[curFrame];
 	{
 		VLOG("PUSH: BeginFrame (curFrame = %d, readyForFence = %d, time=%0.3f)", curFrame, (int)frameData.readyForFence, time_now_d());
 		std::unique_lock<std::mutex> lock(frameData.fenceMutex);
@@ -435,7 +364,7 @@ void GLRenderManager::Finish() {
 	curRenderStep_ = nullptr;  // EndCurRenderStep is this simple here.
 
 	int curFrame = GetCurFrame();
-	FrameData &frameData = frameData_[curFrame];
+	GLFrameData &frameData = frameData_[curFrame];
 
 	frameData_[curFrame].deleter.Take(deleter_);
 
@@ -463,7 +392,7 @@ void GLRenderManager::Finish() {
 
 // Render thread. Returns true if the caller should handle a swap.
 bool GLRenderManager::Run(GLRRenderThreadTask &task) {
-	FrameData &frameData = frameData_[task.frame];
+	GLFrameData &frameData = frameData_[task.frame];
 
 	if (!frameData.hasBegun) {
 		frameData.hasBegun = true;