Skip to content

Commit

Permalink
ImageData: remove automatic mutex locks.
Browse files Browse the repository at this point in the history
  • Loading branch information
slime73 committed Feb 18, 2024
1 parent d880c48 commit 007090a
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 123 deletions.
1 change: 1 addition & 0 deletions src/modules/font/BMFontRasterizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 1 addition & 4 deletions src/modules/font/ImageRasterizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions src/modules/graphics/GraphicsReadback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 0 additions & 6 deletions src/modules/graphics/Texture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 0 additions & 2 deletions src/modules/graphics/opengl/GraphicsReadback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 0 additions & 10 deletions src/modules/image/ImageData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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();

Expand Down
56 changes: 6 additions & 50 deletions src/modules/image/wrap_ImageData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -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);

Expand All @@ -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,
Expand All @@ -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 }
};

Expand Down
59 changes: 15 additions & 44 deletions src/modules/image/wrap_ImageData.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down
8 changes: 1 addition & 7 deletions src/modules/window/sdl/Window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 007090a

Please sign in to comment.