Browse Source

ImageData: remove automatic mutex locks.

Sasha Szpakowski 1 year ago
parent
commit
007090af14

+ 1 - 0
src/modules/font/BMFontRasterizer.cpp

@@ -343,6 +343,7 @@ GlyphData *BMFontRasterizer::getGlyphDataForIndex(int index) const
 	uint8 *pixels = (uint8 *) g->getData();
 	const uint8 *ipixels = (const uint8 *) imagedata->getData();
 
+	// Always lock the mutex since the user can't know when to do it.
 	love::thread::Lock lock(imagedata->getMutex());
 
 	// Copy the subsection of the texture from the ImageData to the GlyphData.

+ 1 - 4
src/modules/font/ImageRasterizer.cpp

@@ -90,7 +90,7 @@ GlyphData *ImageRasterizer::getGlyphDataForIndex(int index) const
 	if (gm.width == 0)
 		return g;
 
-	// We don't want another thread modifying our ImageData mid-copy.
+	// Always lock the mutex since the user can't know when to do it.
 	love::thread::Lock lock(imageData->getMutex());
 
 	Color32 *gdpixels = (Color32 *) g->getData();
@@ -118,9 +118,6 @@ void ImageRasterizer::load(const uint32 *glyphs, int glyphcount)
 	int imgw = imageData->getWidth();
 	int imgh = imageData->getHeight();
 
-	// We don't want another thread modifying our ImageData mid-parse.
-	love::thread::Lock lock(imageData->getMutex());
-
 	// Set the only metric that matters
 	metrics.height = imgh;
 

+ 1 - 0
src/modules/graphics/GraphicsReadback.cpp

@@ -192,6 +192,7 @@ GraphicsReadback::Status GraphicsReadback::readbackBuffer(Buffer *buffer, size_t
 
 		if (imageData.get())
 		{
+			// Always lock the mutex since the user can't know when to do it.
 			love::thread::Lock lock(imageData->getMutex());
 
 			if (imageData->getWidth() != rect.w)

+ 0 - 6
src/modules/graphics/Texture.cpp

@@ -451,12 +451,6 @@ void Texture::drawLayer(Graphics *gfx, int layer, Quad *q, const Matrix4 &m)
 
 void Texture::uploadImageData(love::image::ImageDataBase *d, int level, int slice, int x, int y)
 {
-	love::image::ImageData *id = dynamic_cast<love::image::ImageData *>(d);
-
-	love::thread::EmptyLock lock;
-	if (id != nullptr)
-		lock.setLock(id->getMutex());
-
 	Rect rect = {x, y, d->getWidth(), d->getHeight()};
 	uploadByteData(d->getData(), d->getSize(), level, slice, rect);
 }

+ 0 - 2
src/modules/graphics/opengl/GraphicsReadback.cpp

@@ -68,8 +68,6 @@ GraphicsReadback::GraphicsReadback(love::graphics::Graphics *gfx, ReadbackMethod
 	{
 		void *dest = prepareReadbackDest(size);
 
-		love::thread::Lock lock(imageData->getMutex());
-
 		// Direct readback without copying avoids the need for a staging buffer,
 		// and lowers the system requirements of immediate RT readback.
 		Texture *t = (Texture *) texture;

+ 0 - 10
src/modules/image/ImageData.cpp

@@ -194,10 +194,7 @@ love::filesystem::FileData *ImageData::encode(FormatHandler::EncodedFormat encod
 	}
 
 	if (encoder != nullptr)
-	{
-		thread::Lock lock(mutex);
 		encodedimage = encoder->encode(rawimage, encodedFormat);
-	}
 
 	if (encoder == nullptr || encodedimage.data == nullptr)
 		throw love::Exception("No suitable image encoder for the %s pixel format.", getPixelFormatName(format));
@@ -537,8 +534,6 @@ void ImageData::setPixel(int x, int y, const Colorf &c)
 	if (pixelSetFunction == nullptr)
 		throw love::Exception("ImageData:setPixel does not currently support the %s pixel format.", getPixelFormatName(format));
 
-	Lock lock(mutex);
-
 	pixelSetFunction(c, p);
 }
 
@@ -553,8 +548,6 @@ void ImageData::getPixel(int x, int y, Colorf &c) const
 	if (pixelGetFunction == nullptr)
 		throw love::Exception("ImageData:getPixel does not currently support the %s pixel format.", getPixelFormatName(format));
 
-	Lock lock(mutex);
-
 	pixelGetFunction(p, c);
 }
 
@@ -701,9 +694,6 @@ void ImageData::paste(ImageData *src, int dx, int dy, int sx, int sy, int sw, in
 	if (sy + sh > srcH)
 		sh = srcH - sy;
 
-	Lock lock2(src->mutex);
-	Lock lock1(mutex);
-
 	uint8 *s = (uint8 *) src->getData();
 	uint8 *d = (uint8 *) getData();
 

+ 6 - 50
src/modules/image/wrap_ImageData.cpp

@@ -158,18 +158,15 @@ int w_ImageData_setPixel(lua_State *L)
 	return 0;
 }
 
-// ImageData:mapPixel. Not thread-safe! See wrap_ImageData.lua for the thread-
-// safe wrapper function.
-int w_ImageData__mapPixelUnsafe(lua_State *L)
+int w_ImageData_mapPixel(lua_State *L)
 {
 	ImageData *t = luax_checkimagedata(L, 1);
 	luaL_checktype(L, 2, LUA_TFUNCTION);
 
-	// No optints because we assume they're done in the wrapper function.
-	int sx = (int) lua_tonumber(L, 3);
-	int sy = (int) lua_tonumber(L, 4);
-	int w  = (int) lua_tonumber(L, 5);
-	int h  = (int) lua_tonumber(L, 6);
+	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());
 
 	if (!(t->inside(sx, sy) && t->inside(sx+w-1, sy+h-1)))
 		return luaL_error(L, "Invalid rectangle dimensions.");
@@ -264,32 +261,9 @@ int w_ImageData_encode(lua_State *L)
 	return 1;
 }
 
-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);
-
 	float (*float16to32)(float16 f);
 	float16 (*float32to16)(float f);
 
@@ -302,20 +276,6 @@ struct FFI_ImageData
 
 static FFI_ImageData ffifuncs =
 {
-	[](Proxy *p) // 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) // unlockMutex
-	{
-		ImageData *i = (ImageData *) p->object;
-		i->getMutex()->unlock();
-	},
-
 	float16to32,
 	float32to16,
 	float11to32,
@@ -336,12 +296,8 @@ static const luaL_Reg w_ImageData_functions[] =
 	{ "getPixel", w_ImageData_getPixel },
 	{ "setPixel", w_ImageData_setPixel },
 	{ "paste", w_ImageData_paste },
+	{ "mapPixel", w_ImageData_mapPixel },
 	{ "encode", w_ImageData_encode },
-
-	// Used in the Lua wrapper code.
-	{ "_mapPixelUnsafe", w_ImageData__mapPixelUnsafe },
-	{ "_performAtomic", w_ImageData__performAtomic },
-
 	{ 0, 0 }
 };
 

+ 15 - 44
src/modules/image/wrap_ImageData.lua

@@ -38,29 +38,6 @@ local function clamp01(x)
 	return min(max(x, 0), 1)
 end
 
--- Implement thread-safe ImageData:mapPixel regardless of whether the FFI is
--- used or not.
-function ImageData: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
-
-	if type(ix) ~= "number" then error("bad argument #2 to ImageData:mapPixel (expected number)", 2) end
-	if type(iy) ~= "number" then error("bad argument #3 to ImageData:mapPixel (expected number)", 2) end
-	if type(iw) ~= "number" then error("bad argument #4 to ImageData:mapPixel (expected number)", 2) end
-	if type(ih) ~= "number" then error("bad argument #5 to ImageData:mapPixel (expected number)", 2) end
-
-	if type(func) ~= "function" then error("bad argument #1 to ImageData:mapPixel (expected function)", 2) end
-	if not (inside(ix, iy, idw, idh) and inside(ix+iw-1, iy+ih-1, idw, idh)) then error("Invalid rectangle dimensions", 2) end
-
-	-- 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.
 
@@ -84,9 +61,6 @@ typedef uint16_t float10;
 
 typedef struct FFI_ImageData
 {
-	void (*lockMutex)(Proxy *p);
-	void (*unlockMutex)(Proxy *p);
-
 	float (*float16to32)(float16 f);
 	float16 (*float32to16)(float f);
 
@@ -381,20 +355,23 @@ local objectcache = setmetatable({}, {
 
 -- Overwrite existing functions with new FFI versions.
 
-function ImageData:_performAtomic(...)
-	ffifuncs.lockMutex(self)
-	local success, err = pcall(...)
-	ffifuncs.unlockMutex(self)
-
-	if not success then
-		error(err, 3)
-	end
-end
-
-function ImageData:_mapPixelUnsafe(func, ix, iy, iw, ih)
+function ImageData:mapPixel(func, ix, iy, iw, ih)
 	local p = objectcache[self]
 	local idw, idh = p.width, p.height
 
+	ix = ix or 0
+	iy = iy or 0
+	iw = iw or idw
+	ih = ih or idh
+
+	if type(ix) ~= "number" then error("bad argument #2 to ImageData:mapPixel (expected number)", 2) end
+	if type(iy) ~= "number" then error("bad argument #3 to ImageData:mapPixel (expected number)", 2) end
+	if type(iw) ~= "number" then error("bad argument #4 to ImageData:mapPixel (expected number)", 2) end
+	if type(ih) ~= "number" then error("bad argument #5 to ImageData:mapPixel (expected number)", 2) end
+
+	if type(func) ~= "function" then error("bad argument #1 to ImageData:mapPixel (expected function)", 2) end
+	if not (inside(ix, iy, idw, idh) and inside(ix+iw-1, iy+ih-1, idw, idh)) then error("Invalid rectangle dimensions", 2) end
+
 	if p.pointer == nil then error("ImageData:mapPixel does not currently support the "..p.format.." pixel format.", 2) end
 
 	ix = floor(ix)
@@ -427,12 +404,8 @@ function ImageData:getPixel(x, y)
 
 	if p.pointer == nil then error("ImageData:getPixel does not currently support the "..p.format.." pixel format.", 2) end
 
-	ffifuncs.lockMutex(self)
 	local pixel = p.pointer[y * p.width + x]
-	local r, g, b, a = p.tolua(pixel)
-	ffifuncs.unlockMutex(self)
-
-	return r, g, b, a
+	return p.tolua(pixel)
 end
 
 function ImageData:setPixel(x, y, r, g, b, a)
@@ -457,9 +430,7 @@ function ImageData:setPixel(x, y, r, g, b, a)
 
 	if p.pointer == nil then error("ImageData:setPixel does not currently support the "..p.format.." pixel format.", 2) end
 
-	ffifuncs.lockMutex(self)
 	p.fromlua(p.pointer[y * p.width + x], r, g, b, a)
-	ffifuncs.unlockMutex(self)
 end
 
 function ImageData:getWidth()

+ 1 - 7
src/modules/window/sdl/Window.cpp

@@ -1065,13 +1065,7 @@ bool Window::setIcon(love::image::ImageData *imgd)
 	int bytesperpixel = (int) getPixelFormatBlockSize(imgd->getFormat());
 	int pitch = w * bytesperpixel;
 
-	SDL_Surface *sdlicon = nullptr;
-
-	{
-		// We don't want another thread modifying the ImageData mid-copy.
-		love::thread::Lock lock(imgd->getMutex());
-		sdlicon = SDL_CreateRGBSurfaceFrom(imgd->getData(), w, h, bytesperpixel * 8, pitch, rmask, gmask, bmask, amask);
-	}
+	SDL_Surface *sdlicon = SDL_CreateRGBSurfaceFrom(imgd->getData(), w, h, bytesperpixel * 8, pitch, rmask, gmask, bmask, amask);
 
 	if (!sdlicon)
 		return false;