Implemented efficient FFI versions of ImageData methods.

Note that ImageData:mapPixel (with its optional sub-rectangle parameters) is often many times faster than a loop of ImageData:setPixel and/or getPixel, for anything more than a few pixels.

platform/xcode/liblove.xcodeproj/project.pbxproj

@@ -1514,6 +1514,7 @@
 		FAB2D5A81AABDD8A008224A4 /* TrueTypeRasterizer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TrueTypeRasterizer.cpp; sourceTree = "<group>"; };
 		FAB2D5A91AABDD8A008224A4 /* TrueTypeRasterizer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TrueTypeRasterizer.h; sourceTree = "<group>"; };
 		FAC734C11B2E021A00AB460A /* wrap_SoundData.lua */ = {isa = PBXFileReference; lastKnownFileType = text; path = wrap_SoundData.lua; sourceTree = "<group>"; };
+		FAC734C21B2E628700AB460A /* wrap_ImageData.lua */ = {isa = PBXFileReference; lastKnownFileType = text; path = wrap_ImageData.lua; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -2277,6 +2278,7 @@
 				FA0B7BE51A95902C000E1D17 /* wrap_Image.h */,
 				FA0B7BE61A95902C000E1D17 /* wrap_ImageData.cpp */,
 				FA0B7BE71A95902C000E1D17 /* wrap_ImageData.h */,
+				FAC734C21B2E628700AB460A /* wrap_ImageData.lua */,
 			);
 			path = image;
 			sourceTree = "<group>";

src/modules/image/ImageData.cpp

@@ -71,16 +71,13 @@ void ImageData::setPixel(int x, int y, pixel c)
 	Lock lock(mutex);
 
 	pixel *pixels = (pixel *) getData();
-	pixels[y*getWidth()+x] = c;
+	pixels[y*width+x] = c;
 }
 
 void ImageData::setPixelUnsafe(int x, int y, pixel c)
 {
-	if (!inside(x, y))
-		throw love::Exception("Attempt to set out-of-range pixel!");
-
 	pixel *pixels = (pixel *) getData();
-	pixels[y*getWidth()+x] = c;
+	pixels[y*width+x] = c;
 }
 
 pixel ImageData::getPixel(int x, int y) const
@@ -88,8 +85,16 @@ pixel ImageData::getPixel(int x, int y) const
 	if (!inside(x, y))
 		throw love::Exception("Attempt to get out-of-range pixel!");
 
+	Lock lock(mutex);
+
+	const pixel *pixels = (const pixel *) getData();
+	return pixels[y*width+x];
+}
+
+pixel ImageData::getPixelUnsafe(int x, int y) const
+{
 	const pixel *pixels = (const pixel *) getData();
-	return pixels[y*getWidth()+x];
+	return pixels[y*width+x];
 }
 
 void ImageData::paste(ImageData *src, int dx, int dy, int sx, int sy, int sw, int sh)

src/modules/image/ImageData.h

@@ -98,7 +98,7 @@ public:
 
 	/**
 	 * Sets the pixel at location (x,y).
-	 * Not thread-safe!
+	 * Not thread-safe, and doesn't verify the coordinates!
 	 **/
 	void setPixelUnsafe(int x, int y, pixel p);
 
@@ -110,6 +110,12 @@ public:
 	 **/
 	pixel getPixel(int x, int y) const;
 
+	/**
+	 * Gets the pixel at location (x,y).
+	 * Not thread-safe, and doesn't verify the coordinates!
+	 **/
+	pixel getPixelUnsafe(int x, int y) const;
+
 	/**
 	 * Encodes raw pixel data into a given format.
 	 * @param f The file to save the encoded image data to.

src/modules/image/wrap_ImageData.cpp

@@ -23,11 +23,21 @@
 #include "common/wrap_Data.h"
 #include "filesystem/File.h"
 
+// Shove the wrap_ImageData.lua code directly into a raw string literal.
+static const char imagedata_lua[] =
+#include "wrap_ImageData.lua"
+;
+
 namespace love
 {
 namespace image
 {
 
+/**
+ * NOTE: Additional wrapper code is in wrap_ImageData.lua. Be sure to keep it
+ * in sync with any changes made to this file!
+ **/
+
 ImageData *luax_checkimagedata(lua_State *L, int idx)
 {
 	return luax_checktype<ImageData>(L, idx, IMAGE_IMAGE_DATA_ID);
@@ -130,8 +140,9 @@ static int luax_retnumbererror(lua_State *L, int level, int retnum, int ttype)
 	                     where, retnum, ttypename);
 }
 
-// ImageData:mapPixel. Not thread-safe! See the wrapper function below.
-static int w_ImageData_mapPixelUnsafe(lua_State *L)
+// ImageData:mapPixel. Not thread-safe! See wrap_ImageData.lua for the thread-
+// safe wrapper function.
+int w_ImageData__mapPixelUnsafe(lua_State *L)
 {
 	ImageData *t = luax_checkimagedata(L, 1);
 	luaL_checktype(L, 2, LUA_TFUNCTION);
@@ -186,38 +197,6 @@ static int w_ImageData_mapPixelUnsafe(lua_State *L)
 			t->setPixelUnsafe(x, y, c);
 		}
 	}
-	return 0;
-}
-
-// Thread-safe wrapper for the above function.
-int w_ImageData_mapPixel(lua_State *L)
-{
-	ImageData *t = luax_checkimagedata(L, 1);
-	luaL_checktype(L, 2, LUA_TFUNCTION);
-	int sx = luaL_optint(L, 3, 0);
-	int sy = luaL_optint(L, 4, 0);
-	int w = luaL_optint(L, 5, t->getWidth());
-	int h = luaL_optint(L, 6, t->getHeight());
-
-	lua_pushcfunction(L, w_ImageData_mapPixelUnsafe);
-	lua_pushvalue(L, 1);
-	lua_pushvalue(L, 2);
-	lua_pushinteger(L, sx);
-	lua_pushinteger(L, sy);
-	lua_pushinteger(L, w);
-	lua_pushinteger(L, h);
-
-	int ret = 0;
-
-	// Lock this ImageData's mutex during the entire mapPixel. We pcall instead
-	// of call because lua_error longjmp's without calling object destructors.
-	{
-		love::thread::Lock lock(t->getMutex());
-		ret = lua_pcall(L, 6, 0, 0);
-	}
-
-	if (ret != 0)
-		return lua_error(L);
 
 	return 0;
 }
@@ -265,6 +244,50 @@ int w_ImageData_encode(lua_State *L)
 	return 0;
 }
 
+int w_ImageData__performAtomic(lua_State *L)
+{
+	ImageData *t = luax_checkimagedata(L, 1);
+	int err = 0;
+
+	{
+		love::thread::Lock lock(t->getMutex());
+		// call the function, passing any user-specified arguments.
+		err = lua_pcall(L, lua_gettop(L) - 2, LUA_MULTRET, 0);
+	}
+
+	// Unfortunately, this eats the stack trace, too bad.
+	if (err != 0)
+		return lua_error(L);
+
+	// The function and everything after it in the stack are eaten by the pcall,
+	// leaving only the ImageData object. Everything else is a return value.
+	return lua_gettop(L) - 1;
+}
+
+// C functions in a struct, necessary for the FFI versions of ImageData methods.
+struct FFI_ImageData
+{
+	void (*lockMutex)(Proxy *p);
+	void (*unlockMutex)(Proxy *p);
+};
+
+static FFI_ImageData ffifuncs =
+{
+	[](Proxy *p) -> void // lockMutex
+	{
+		// We don't do any type-checking for the Proxy here since these functions
+		// are always called from code which has already done type checking.
+		ImageData *i = (ImageData *) p->object;
+		i->getMutex()->lock();
+	},
+
+	[](Proxy *p) -> void // unlockMutex
+	{
+		ImageData *i = (ImageData *) p->object;
+		i->getMutex()->unlock();
+	}
+};
+
 static const luaL_Reg functions[] =
 {
 	// Data
@@ -277,15 +300,36 @@ static const luaL_Reg functions[] =
 	{ "getDimensions", w_ImageData_getDimensions },
 	{ "getPixel", w_ImageData_getPixel },
 	{ "setPixel", w_ImageData_setPixel },
-	{ "mapPixel", w_ImageData_mapPixel },
 	{ "paste", w_ImageData_paste },
 	{ "encode", w_ImageData_encode },
+
+	// Used in the Lua wrapper code.
+	{ "_mapPixelUnsafe", w_ImageData__mapPixelUnsafe },
+	{ "_performAtomic", w_ImageData__performAtomic },
+
 	{ 0, 0 }
 };
 
 extern "C" int luaopen_imagedata(lua_State *L)
 {
-	return luax_register_type(L, IMAGE_IMAGE_DATA_ID, functions);
+	// The last argument pushes the type's metatable onto the stack.
+	int ret = luax_register_type(L, IMAGE_IMAGE_DATA_ID, functions, true);
+
+	// Load and execute ImageData.lua, sending the metatable and the ffi
+	// functions struct pointer as arguments.
+	if (ret > 0)
+	{
+		luaL_loadbuffer(L, imagedata_lua, sizeof(imagedata_lua), "ImageData.lua");
+		lua_pushvalue(L, -2);
+		lua_pushlightuserdata(L, &ffifuncs);
+		lua_call(L, 2, 0);
+
+		// Pop the metatable.
+		lua_pop(L, 1);
+		ret--;
+	}
+
+	return ret;
 }
 
 } // image

src/modules/image/wrap_ImageData.lua

@@ -0,0 +1,189 @@
+R"luastring"--(
+-- DO NOT REMOVE THE ABOVE LINE. It is used to load this file as a C++ string.
+-- There is a matching delimiter at the bottom of the file.
+
+--[[
+Copyright (c) 2006-2015 LOVE Development Team
+
+This software is provided 'as-is', without any express or implied
+warranty.  In no event will the authors be held liable for any damages
+arising from the use of this software.
+
+Permission is granted to anyone to use this software for any purpose,
+including commercial applications, and to alter it and redistribute it
+freely, subject to the following restrictions:
+
+1. The origin of this software must not be misrepresented; you must not
+claim that you wrote the original software. If you use this software
+in a product, an acknowledgment in the product documentation would be
+appreciated but is not required.
+2. Altered source versions must be plainly marked as such, and must not be
+misrepresented as being the original software.
+3. This notice may not be removed or altered from any source distribution.
+--]]
+
+local ImageData_mt, ffifuncspointer = ...
+
+local tonumber, assert = tonumber, assert
+local type, pcall = type, pcall
+
+local function inside(x, y, w, h)
+	return x >= 0 and x < w and y >= 0 and y < h
+end
+
+-- Implement thread-safe ImageData:mapPixel regardless of whether the FFI is
+-- used or not.
+function ImageData_mt.__index:mapPixel(func, ix, iy, iw, ih)
+	local idw, idh = self:getDimensions()
+
+	ix = ix or 0
+	iy = iy or 0
+	iw = iw or idw
+	ih = ih or idh
+
+	assert(type(ix) == "number", "Invalid argument #2 to ImageData:mapPixel (expected number)")
+	assert(type(iy) == "number", "Invalid argument #3 to ImageData:mapPixel (expected number)")
+	assert(type(iw) == "number", "Invalid argument #4 to ImageData:mapPixel (expected number)")
+	assert(type(ih) == "number", "Invalid argument #5 to ImageData:mapPixel (expected number)")
+
+	assert(type(func) == "function", "Invalid argument #1 to ImageData:mapPixel (expected function)")
+	assert(inside(ix, iy, idw, idh) and inside(ix+iw-1, iy+ih-1, idw, idh), "Invalid rectangle dimensions")
+
+	-- performAtomic and mapPixelUnsafe have Lua-C API and FFI versions.
+	self:_performAtomic(self._mapPixelUnsafe, self, func, ix, iy, iw, ih)
+end
+
+
+-- Everything below this point is efficient FFI replacements for existing
+-- ImageData functionality.
+
+if type(jit) ~= "table" or not jit.status() then
+	-- LuaJIT's FFI is *much* slower than LOVE's regular methods when the JIT
+	-- compiler is disabled.
+	return
+end
+
+local status, ffi = pcall(require, "ffi")
+if not status then return end
+
+pcall(ffi.cdef, [[
+typedef struct Proxy Proxy;
+
+typedef struct FFI_ImageData
+{
+	void (*lockMutex)(Proxy *p);
+	void (*unlockMutex)(Proxy *p);
+} FFI_ImageData;
+
+typedef struct ImageData_Pixel
+{
+	uint8_t r, g, b, a;
+} ImageData_Pixel;
+]])
+
+local ffifuncs = ffi.cast("FFI_ImageData *", ffifuncspointer)
+
+local pixelpointer = ffi.typeof("ImageData_Pixel *")
+
+local _getWidth = ImageData_mt.__index.getWidth
+local _getHeight = ImageData_mt.__index.getHeight
+local _getDimensions = ImageData_mt.__index.getDimensions
+
+-- Table which holds ImageData objects as keys, and information about the objects
+-- as values. Uses weak keys so the ImageData objects can still be GC'd properly.
+local objectcache = setmetatable({}, {
+	__mode = "k",
+	__index = function(self, imagedata)
+		local width, height = _getDimensions(imagedata)
+		local pointer = ffi.cast(pixelpointer, imagedata:getPointer())
+
+		local p = {
+			width = width,
+			height = height,
+			pointer = pointer,
+		}
+
+		self[imagedata] = p
+		return p
+	end,
+})
+
+
+-- Overwrite existing functions with new FFI versions.
+
+function ImageData_mt.__index:_performAtomic(...)
+	ffifuncs.lockMutex(self)
+	local success, err = pcall(...)
+	ffifuncs.unlockMutex(self)
+
+	if not success then
+		error(err, 3)
+	end
+end
+
+function ImageData_mt.__index:_mapPixelUnsafe(func, ix, iy, iw, ih)
+	local p = objectcache[self]
+	local idw, idh = p.width, p.height
+
+	local pixels = p.pointer
+
+	for y=iy, iy+ih-1 do
+		for x=ix, ix+iw-1 do
+			local p = pixels[y*idw+x]
+			local r, g, b, a = func(x, y, tonumber(p.r), tonumber(p.g), tonumber(p.b), tonumber(p.a))
+			pixels[y*idw+x].r = r
+			pixels[y*idw+x].g = g
+			pixels[y*idw+x].b = b
+			pixels[y*idw+x].a = a == nil and 255 or a
+		end
+	end
+end
+
+function ImageData_mt.__index:getPixel(x, y)
+	local p = objectcache[self]
+	assert(inside(x, y, p.width, p.height), "Attempt to get out-of-range pixel!")
+
+	ffifuncs.lockMutex(self)
+	local pixel = p.pointer[y * p.width + x]
+	local r, g, b, a = tonumber(pixel.r), tonumber(pixel.g), tonumber(pixel.b), tonumber(pixel.a)
+	ffifuncs.unlockMutex(self)
+
+	return r, g, b, a
+end
+
+local temppixel = ffi.new("ImageData_Pixel")
+
+function ImageData_mt.__index:setPixel(x, y, r, g, b, a)	
+	local p = objectcache[self]
+	assert(inside(x, y, p.width, p.height), "Attempt to set out-of-range pixel!")
+
+	if type(r) == "table" then
+		local t = r
+		r, g, b, a = t[1], t[2], t[3], t[4]
+	end
+
+	temppixel.r = r
+	temppixel.g = g
+	temppixel.b = b
+	temppixel.a = a == nil and 255 or a
+
+	ffifuncs.lockMutex(self)
+	p.pointer[y * p.width + x] = temppixel
+	ffifuncs.unlockMutex(self)
+end
+
+function ImageData_mt.__index:getWidth()
+	return objectcache[self].width
+end
+
+function ImageData_mt.__index:getHeight()
+	return objectcache[self].height
+end
+
+function ImageData_mt.__index:getDimensions()
+	local p = objectcache[self]
+	return p.width, p.height
+end
+
+-- DO NOT REMOVE THE NEXT LINE. It is used to load this file as a C++ string.
+--)luastring"--"