BGE: Save screenshots in a different thread

This patch allows the game engine to keep running while performing things like PNG compression and disk I/O.

As an example, my crowd simulation rasterizer saves a screenshot for every frame. This now takes up 13 msec per frame, which was 31 msec before this patch. Effectively, it allows the simulation to save every frame and still run at 60 FPS.

Reviewers: lordloki, moguri, panzergame

Reviewed By: moguri, panzergame

Projects: #game_engine

Differential Revision: https://developer.blender.org/D1507

8 files changed, 178 insertions, 85 deletions

diff --git a/source/gameengine/BlenderRoutines/KX_BlenderCanvas.cpp b/source/gameengine/BlenderRoutines/KX_BlenderCanvas.cpp
index e37818678d6..927b26faf8a 100644
--- a/source/gameengine/BlenderRoutines/KX_BlenderCanvas.cpp
+++ b/source/gameengine/BlenderRoutines/KX_BlenderCanvas.cpp
@@ -35,23 +35,16 @@
 
 #include "KX_BlenderCanvas.h"
 
-#include "DNA_image_types.h"
-#include "DNA_scene_types.h"
 #include "DNA_screen_types.h"
+#include "DNA_scene_types.h"
 #include "DNA_windowmanager_types.h"
 
 #include "BKE_image.h"
-#include "BKE_global.h"
-#include "BKE_main.h"
-
-#include "BLI_path_util.h"
-#include "BLI_string.h"
 
 #include <assert.h>
+#include <iostream>
 
 extern "C" {
-#include "IMB_imbuf.h"
-#include "IMB_imbuf_types.h"
 #include "WM_api.h"
 #include "wm_cursors.h"
 #include "wm_window.h"
@@ -341,33 +334,20 @@ void KX_BlenderCanvas::MakeScreenShot(const char *filename)
 	area_dummy.totrct.ymax = m_frame_rect.GetTop();
 
 	dumprect = screenshot(&area_dummy, &dumpsx, &dumpsy);
-
-	if (dumprect) {
-		/* initialize image file format data */
-		Scene *scene = (screen)? screen->scene: NULL;
-		ImageFormatData im_format;
-
-		if (scene)
-			im_format = scene->r.im_format;
-		else
-			BKE_imformat_defaults(&im_format);
-
-		/* create file path */
-		char path[FILE_MAX];
-		BLI_strncpy(path, filename, sizeof(path));
-		BLI_path_abs(path, G.main->name);
-		BLI_path_frame(path, m_frame, 0);
-		m_frame++;
-		BKE_image_path_ensure_ext_from_imtype(path, im_format.imtype);
-
-		/* create and save imbuf */
-		ImBuf *ibuf = IMB_allocImBuf(dumpsx, dumpsy, 24, 0);
-		ibuf->rect = dumprect;
-
-		BKE_imbuf_write_as(ibuf, path, &im_format, false);
-
-		ibuf->rect = NULL;
-		IMB_freeImBuf(ibuf);
-		MEM_freeN(dumprect);
+	if (!dumprect) {
+		std::cerr << "KX_BlenderCanvas: Unable to take screenshot!" << std::endl;
+		return;
 	}
+
+	/* initialize image file format data */
+	Scene *scene = (screen)? screen->scene: NULL;
+	ImageFormatData *im_format = (ImageFormatData *)MEM_mallocN(sizeof(ImageFormatData), "im_format");
+
+	if (scene)
+		*im_format = scene->r.im_format;
+	else
+		BKE_imformat_defaults(im_format);
+
+	/* save_screenshot() frees dumprect and im_format */
+	save_screenshot(filename, dumpsx, dumpsy, dumprect, im_format);
 }
diff --git a/source/gameengine/BlenderRoutines/KX_BlenderCanvas.h b/source/gameengine/BlenderRoutines/KX_BlenderCanvas.h
index 817a667d783..6f408f86551 100644
--- a/source/gameengine/BlenderRoutines/KX_BlenderCanvas.h
+++ b/source/gameengine/BlenderRoutines/KX_BlenderCanvas.h
@@ -212,7 +212,6 @@ private:
 	RAS_Rect 	m_area_rect;
 	int			m_area_left;
 	int			m_area_top;
-	int			m_frame;
 
 
 #ifdef WITH_CXX_GUARDEDALLOC
diff --git a/source/gameengine/GamePlayer/common/GPC_Canvas.cpp b/source/gameengine/GamePlayer/common/GPC_Canvas.cpp
index 52c4d13c638..2b355407d46 100644
--- a/source/gameengine/GamePlayer/common/GPC_Canvas.cpp
+++ b/source/gameengine/GamePlayer/common/GPC_Canvas.cpp
@@ -30,31 +30,14 @@
  */
 
 
-#ifndef NOPNG
-#ifdef WIN32
-#include "png.h"
-#else
-#include <png.h>
-#endif
-#endif // NOPNG
-
 #include "RAS_IPolygonMaterial.h"
 #include "GPC_Canvas.h"
 
-#include "BLI_path_util.h"
-#include "BLI_string.h"
-
 #include "DNA_scene_types.h"
 #include "DNA_space_types.h"
 
-#include "BKE_global.h"
-#include "BKE_main.h"
 #include "BKE_image.h"
-
-extern "C" {
-#include "IMB_imbuf.h"
-#include "IMB_imbuf_types.h"
-}
+#include "MEM_guardedalloc.h"
 
 
 GPC_Canvas::GPC_Canvas(
@@ -164,37 +147,22 @@ MakeScreenShot(
 	const char* filename
 ) {
 	// copy image data
-	unsigned char *pixels = new unsigned char[GetWidth() * GetHeight() * 4];
+	unsigned int dumpsx = GetWidth();
+	unsigned int dumpsy = GetHeight();
+	unsigned int *pixels = (unsigned int *)MEM_mallocN(sizeof(int) * dumpsx * dumpsy, "pixels");
 
 	if (!pixels) {
 		std::cout << "Cannot allocate pixels array" << std::endl;
 		return;
 	}
 
-	glReadPixels(0, 0, GetWidth(), GetHeight(), GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+	glReadPixels(0, 0, dumpsx, dumpsy, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
 
 	// initialize image file format data
-	ImageFormatData im_format;
-	BKE_imformat_defaults(&im_format);
-
-	// create file path 
-	char path[FILE_MAX];
-	BLI_strncpy(path, filename, sizeof(path));
-	BLI_path_abs(path, G.main->name);
-	BLI_path_frame(path, m_frame, 0);
-	m_frame++;
-	BKE_image_path_ensure_ext_from_imtype(path, im_format.imtype);
-
-	// create and save imbuf 
-	ImBuf *ibuf = IMB_allocImBuf(GetWidth(), GetHeight(), 24, 0);
-	ibuf->rect = (unsigned int*)pixels;
-
-	BKE_imbuf_write_as(ibuf, path, &im_format, false);
-
-	ibuf->rect = NULL;
-	IMB_freeImBuf(ibuf);
+	ImageFormatData *im_format = (ImageFormatData *)MEM_mallocN(sizeof(ImageFormatData), "im_format");
+	BKE_imformat_defaults(im_format);
 
-	// clean up
-	delete [] (pixels);
+	/* save_screenshot() frees dumprect and im_format */
+	save_screenshot(filename, dumpsx, dumpsy, pixels, im_format);
 }
 
diff --git a/source/gameengine/GamePlayer/common/GPC_Canvas.h b/source/gameengine/GamePlayer/common/GPC_Canvas.h
index 34cc9759a08..9a108203ee8 100644
--- a/source/gameengine/GamePlayer/common/GPC_Canvas.h
+++ b/source/gameengine/GamePlayer/common/GPC_Canvas.h
@@ -56,8 +56,6 @@ protected:
 	/** Rect that defines the area used for rendering,
 	 * relative to the context */
 	RAS_Rect m_displayarea;
-	/** Frame counter for screenshots */
-	int m_frame;
 
 	int m_viewport[4];
 
diff --git a/source/gameengine/Rasterizer/CMakeLists.txt b/source/gameengine/Rasterizer/CMakeLists.txt
index 5bc3f22e327..496a864244b 100644
--- a/source/gameengine/Rasterizer/CMakeLists.txt
+++ b/source/gameengine/Rasterizer/CMakeLists.txt
@@ -31,6 +31,7 @@ set(INC
 	../../blender/makesdna
 	../../blender/blenlib
 	../../blender/blenkernel
+	../../blender/imbuf
 	../../../intern/container
 	../../../intern/glew-mx
 	../../../intern/guardedalloc
@@ -53,6 +54,7 @@ set(SRC
 	RAS_Polygon.cpp
 	RAS_TexVert.cpp
 	RAS_texmatrix.cpp
+	RAS_ICanvas.cpp
 
 	RAS_2DFilterManager.h
 	RAS_BucketManager.h
diff --git a/source/gameengine/Rasterizer/RAS_ICanvas.cpp b/source/gameengine/Rasterizer/RAS_ICanvas.cpp
new file mode 100644
index 00000000000..808d257f8f0
--- /dev/null
+++ b/source/gameengine/Rasterizer/RAS_ICanvas.cpp
@@ -0,0 +1,128 @@
+/*
+ * ***** BEGIN GPL LICENSE BLOCK *****
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * ***** END GPL LICENSE BLOCK *****
+ */
+
+/** \file gameengine/Rasterizer/RAS_ICanvas.cpp
+ *  \ingroup bgerast
+ */
+
+#include "RAS_ICanvas.h"
+#include "DNA_scene_types.h"
+
+#include "BKE_image.h"
+#include "BKE_global.h"
+#include "BKE_main.h"
+
+#include "BLI_task.h"
+#include "BLI_path_util.h"
+#include "BLI_string.h"
+
+#include "MEM_guardedalloc.h"
+
+extern "C" {
+#include "IMB_imbuf.h"
+#include "IMB_imbuf_types.h"
+}
+
+
+// Task data for saving screenshots in a different thread.
+struct ScreenshotTaskData
+{
+	unsigned int *dumprect;
+	int dumpsx;
+	int dumpsy;
+	char *path;
+	ImageFormatData *im_format;
+};
+
+/**
+ * Function that actually performs the image compression and saving to disk of a screenshot.
+ * Run in a separate thread by RAS_ICanvas::save_screenshot().
+ *
+ * @param taskdata Must point to a ScreenshotTaskData object. This function takes ownership
+ *                 of all pointers in the ScreenshotTaskData, and frees them.
+ */
+void save_screenshot_thread_func(TaskPool *__restrict pool, void *taskdata, int threadid);
+
+
+RAS_ICanvas::RAS_ICanvas()
+{
+	m_taskscheduler = BLI_task_scheduler_create(TASK_SCHEDULER_AUTO_THREADS);
+	m_taskpool = BLI_task_pool_create(m_taskscheduler, NULL);
+}
+
+RAS_ICanvas::~RAS_ICanvas()
+{
+	if (m_taskpool) {
+		BLI_task_pool_work_and_wait(m_taskpool);
+		BLI_task_pool_free(m_taskpool);
+		m_taskpool = NULL;
+	}
+
+	if (m_taskscheduler) {
+		BLI_task_scheduler_free(m_taskscheduler);
+		m_taskscheduler = NULL;
+	}
+}
+
+
+void save_screenshot_thread_func(TaskPool *__restrict UNUSED(pool), void *taskdata, int UNUSED(threadid))
+{
+	ScreenshotTaskData *task = static_cast<ScreenshotTaskData *>(taskdata);
+
+	/* create and save imbuf */
+	ImBuf *ibuf = IMB_allocImBuf(task->dumpsx, task->dumpsy, 24, 0);
+	ibuf->rect = task->dumprect;
+
+	BKE_imbuf_write_as(ibuf, task->path, task->im_format, false);
+
+	ibuf->rect = NULL;
+	IMB_freeImBuf(ibuf);
+	MEM_freeN(task->dumprect);
+	MEM_freeN(task->path);
+	MEM_freeN(task->im_format);
+}
+
+
+void RAS_ICanvas::save_screenshot(const char *filename, int dumpsx, int dumpsy, unsigned int *dumprect,
+                                  ImageFormatData * im_format)
+{
+	/* create file path */
+	char *path = (char *)MEM_mallocN(FILE_MAX, "screenshot-path");
+	BLI_strncpy(path, filename, FILE_MAX);
+	BLI_path_abs(path, G.main->name);
+	BLI_path_frame(path, m_frame, 0);
+	m_frame++;
+	BKE_image_path_ensure_ext_from_imtype(path, im_format->imtype);
+
+	/* Save the actual file in a different thread, so that the
+	 * game engine can keep running at full speed. */
+	ScreenshotTaskData *task = (ScreenshotTaskData *)MEM_mallocN(sizeof(ScreenshotTaskData), "screenshot-data");
+	task->dumprect = dumprect;
+	task->dumpsx = dumpsx;
+	task->dumpsy = dumpsy;
+	task->path = path;
+	task->im_format = im_format;
+
+	BLI_task_pool_push(m_taskpool,
+	                   save_screenshot_thread_func,
+	                   task,
+	                   true, // free task data
+	                   TASK_PRIORITY_LOW);
+}
diff --git a/source/gameengine/Rasterizer/RAS_ICanvas.h b/source/gameengine/Rasterizer/RAS_ICanvas.h
index 471c2c97fa1..91cc13c8f85 100644
--- a/source/gameengine/Rasterizer/RAS_ICanvas.h
+++ b/source/gameengine/Rasterizer/RAS_ICanvas.h
@@ -37,6 +37,9 @@
 #endif
 
 class RAS_Rect;
+struct TaskScheduler;
+struct TaskPool;
+struct ImageFormatData;
 
 /**
  * 2D rendering device context. The connection from 3d rendercontext to 2d surface.
@@ -56,10 +59,8 @@ public:
 		MOUSE_NORMAL
 	};
 
-	virtual 
-	~RAS_ICanvas(
-	) {
-	}
+	RAS_ICanvas();
+	virtual ~RAS_ICanvas();
 
 	virtual 
 		void 
@@ -260,7 +261,23 @@ public:
 	
 protected:
 	RAS_MouseState m_mousestate;
+	int m_frame;  /// frame number for screenshots.
+	TaskScheduler *m_taskscheduler;
+	TaskPool *m_taskpool;
 
+	/**
+	 * Saves screenshot data to a file. The actual compression and disk I/O is performed in
+	 * a separate thread.
+	 *
+	 * @param filename name of the file, can contain "###" for sequential numbering. A copy of the string
+	 *                 is made, so the pointer can be freed by the caller.
+	 * @param dumpsx width in pixels.
+	 * @param dumpsy height in pixels.
+	 * @param dumprect pixel data; ownership is passed to this function, which also frees the data.
+	 * @param im_format image format for the file; ownership is passed to this function, which also frees the data.
+	 */
+	void save_screenshot(const char *filename, int dumpsx, int dumpsy, unsigned int *dumprect,
+	                     ImageFormatData * im_format);
 
 #ifdef WITH_CXX_GUARDEDALLOC
 	MEM_CXX_CLASS_ALLOC_FUNCS("GE:RAS_ICanvas")
diff --git a/source/gameengine/Rasterizer/SConscript b/source/gameengine/Rasterizer/SConscript
index a643f46e39a..a29acda545a 100644
--- a/source/gameengine/Rasterizer/SConscript
+++ b/source/gameengine/Rasterizer/SConscript
@@ -44,6 +44,7 @@ incs = [
     '#source/blender/blenkernel',
     '#source/blender/gpu',
     '#source/blender/makesdna',
+    '#source/blender/imbuf',
     ]
 
 defs = []