Clean up love.image because we own the memory, not DevIL

src/modules/image/Image.h

@@ -43,7 +43,7 @@ namespace image
 		/**
 		* Destructor.
 		**/
-		virtual ~Image(){};
+		virtual ~Image() {};
 
 		/**
 		* Creates new ImageData from a file.

src/modules/image/ImageData.cpp

@@ -20,12 +20,64 @@
 
 #include "ImageData.h"
 
+using love::thread::Lock;
+
 namespace love
 {
 namespace image
 {
+	void * ImageData::getData() const
+	{
+		return data;
+	}
+
+	int ImageData::getSize() const
+	{
+		return getWidth()*getHeight()*sizeof(pixel);
+	}
+
+	int ImageData::getWidth() const
+	{
+		return width;
+	}
+
+	int ImageData::getHeight() const
+	{
+		return height;
+	}
+
+	bool ImageData::inside(int x, int y) const
+	{
+		return x >= 0 && x < getWidth() && y >= 0 && y < getHeight();
+	}
+
+	void ImageData::setPixel(int x, int y, pixel c)
+	{
+		if (!inside(x, y))
+			throw love::Exception("Attempt to set out-of-range pixel!");
+
+		Lock lock(mutex);
+
+		pixel * pixels = (pixel *)getData();
+		pixels[y*getWidth()+x] = c;
+	}
+
+	pixel ImageData::getPixel(int x, int y)
+	{
+		if (!inside(x, y))
+			throw love::Exception("Attempt to get out-of-range pixel!");
+
+		Lock lock(mutex);
+
+		pixel * pixels = (pixel *)getData();
+		return pixels[y*getWidth()+x];
+	}
+
 	void ImageData::paste(ImageData * src, int dx, int dy, int sx, int sy, int sw, int sh)
 	{
+		Lock lock2(src->mutex);
+		Lock lock1(mutex);
+
 		pixel * s = (pixel *)src->getData();
 		pixel * d = (pixel *)getData();
 
@@ -57,11 +109,6 @@ namespace image
 		}
 	}
 
-	bool ImageData::inside(int x, int y) const
-	{
-		return (x >= 0 && x < getWidth() && y >= 0 && y < getHeight());
-	}
-
 	bool ImageData::getConstant(const char * in, ImageData::Format & out)
 	{
 		return formats.find(in, out);

src/modules/image/ImageData.h

@@ -24,6 +24,9 @@
 // LOVE
 #include <common/Data.h>
 #include <filesystem/File.h>
+#include <thread/threads.h>
+
+using love::thread::Mutex;
 
 namespace love
 {
@@ -41,6 +44,22 @@ namespace image
 	**/
 	class ImageData : public Data
 	{
+	protected:
+
+		// The width of the image data.
+		int width;
+
+		// The height of the image data.
+		int height;
+
+		// The actual data.
+		unsigned char * data;
+
+		// We need to be thread-safe
+		// so we lock when we're accessing our
+		// data
+		Mutex mutex;
+
 	public:
 
 		enum Format
@@ -56,7 +75,7 @@ namespace image
 		/**
 		* Destructor.
 		**/
-		virtual ~ImageData(){};
+		virtual ~ImageData() {};
 
 		static bool getConstant(const char * in, Format & out);
 		static bool getConstant(Format in, const char *& out);
@@ -84,38 +103,41 @@ namespace image
 		* Gets the width of this ImageData.
 		* @return The width of this ImageData.
 		**/
-		virtual int getWidth() const = 0;
+		int getWidth() const;
 
 		/**
 		* Gets the height of this ImageData.
 		* @return The height of this ImageData.
 		**/
-		virtual int getHeight() const  = 0;
+		int getHeight() const;
 
 		/**
-		* Sets the pixel at location (x,y). No effect if out of bounds.
+		* Sets the pixel at location (x,y).
 		* @param x The location along the x-axis.
 		* @param y The location along the y-axis.
 		* @param p The color to use for the given location.
 		**/
-		virtual void setPixel(int x, int y, pixel p) = 0;
+		void setPixel(int x, int y, pixel p);
 
 		/**
-		* Gets the pixel at location (x,y). Returns black (0,0,0,0) if out
-		* out of bounds.
+		* Gets the pixel at location (x,y).
 		* @param x The location along the x-axis.
 		* @param y The location along the y-axis.
 		* @return The color for the given location.
 		**/
-		virtual pixel getPixel(int x, int y) = 0;
+		pixel getPixel(int x, int y);
 
 		/**
 		 * Encodes raw pixel data into a given format.
-		 * @param f The format to convert to.
-		 * @return A pointer to the encoded image data.
+		 * @param f The file to save the encoded image data to.
+		 * @param format The format of the encoded data.
 		 **/
 		virtual void encode(love::filesystem::File * f, Format format) = 0;
 
+		// Implements Data.
+		void * getData() const;
+		int getSize() const;
+
 	private:
 		static StringMap<Format, FORMAT_MAX_ENUM>::Entry formatEntries[];
 		static StringMap<Format, FORMAT_MAX_ENUM> formats;

src/modules/image/devil/Image.cpp

@@ -55,15 +55,7 @@ namespace devil
 
 	love::image::ImageData * Image::newImageData(Data * data)
 	{
-		try
-		{
-			return new ImageData(data);
-		}
-		catch (love::Exception & e)
-		{
-			throw love::Exception(e.what());
-		}
-
+		return new ImageData(data);
 	}
 
 	love::image::ImageData * Image::newImageData(int width, int height)

src/modules/image/devil/ImageData.cpp

@@ -39,112 +39,10 @@ namespace image
 {
 namespace devil
 {
-	void ImageData::create(int width, int height, void * data)
-	{
-		Lock lock(devilMutex); //automatically lock and unlock
-		ILuint image;
-
-		//create the image
-		ilGenImages(1, &image);
-
-		//bind it
-		ilBindImage(image);
-
-		while (ilGetError() != IL_NO_ERROR);
-
-		//create and populate the image
-		bool success = (ilTexImage(width, height, 1, bpp, IL_RGBA, IL_UNSIGNED_BYTE, data) == IL_TRUE);
-
-		ILenum err = ilGetError();
-		while (ilGetError() != IL_NO_ERROR);
-
-		if (!success)
-		{
-			ilDeleteImages(1, &image);
-
-			if (err != IL_NO_ERROR)
-			{
-				switch (err)
-				{
-					case IL_ILLEGAL_OPERATION:
-						throw love::Exception("Illegal operation");
-					case IL_INVALID_PARAM:
-						throw love::Exception("Invalid parameters");
-					case IL_OUT_OF_MEMORY:
-						throw love::Exception("Out of memory");
-					default:
-						throw love::Exception("Unknown error (%d)", (int) err);
-				}
-			}
-
-			throw love::Exception("Could not decode image data.");
-		}
 
-		try
-		{
-			this->data = new unsigned char[width*height*bpp];
-		}
-		catch (std::bad_alloc)
-		{
-			ilDeleteImages(1, &image);
-			throw love::Exception("Out of memory");
-		}
-
-		memcpy(this->data, ilGetData(), width*height*bpp);
-
-		ilDeleteImages(1, &image);
-	}
-
-	void ImageData::load(Data * data)
+	static inline void ilxClearErrors()
 	{
-		Lock lock(devilMutex);
-		ILuint image;
-
-		// Generate DevIL image.
-		ilGenImages(1, &image);
-
-		// Bind the image.
-		ilBindImage(image);
-
-		// Try to load the image.
-		ILboolean success = ilLoadL(IL_TYPE_UNKNOWN, (void*)data->getData(), data->getSize());
-
-		// Check for errors
-		if (!success)
-		{
-			throw love::Exception("Could not decode image!");
-		}
-
-		width = ilGetInteger(IL_IMAGE_WIDTH);
-		height = ilGetInteger(IL_IMAGE_HEIGHT);
-		origin = ilGetInteger(IL_IMAGE_ORIGIN);
-
-		// Make sure the image is in RGBA format.
-		ilConvertImage(IL_RGBA, IL_UNSIGNED_BYTE);
-
-		// This should always be four.
-		bpp = ilGetInteger(IL_IMAGE_BPP);
-
-		if (bpp != 4)
-		{
-			ilDeleteImages(1, &image);
-			std::cerr << "Bits per pixel != 4" << std::endl;
-			return;
-		}
-
-		try
-		{
-			this->data = new unsigned char[width*height*bpp];
-		}
-		catch (std::bad_alloc)
-		{
-			ilDeleteImages(1, &image);
-			throw love::Exception("Out of memory");
-		}
-
-		memcpy(this->data, ilGetData(), width*height*bpp);
-
-		ilDeleteImages(1, &image);
+		while (ilGetError() != IL_NO_ERROR);
 	}
 
 	ImageData::ImageData(Data * data)
@@ -160,17 +58,19 @@ namespace devil
 	}
 
 	ImageData::ImageData(int width, int height)
-		: width(width), height(height), origin(IL_ORIGIN_UPPER_LEFT), bpp(4)
 	{
+		this->width = width;
+		this->height = height;
 		create(width, height);
 
 		// Set to black.
 		memset(data, 0, width*height*4);
 	}
 
-	ImageData::ImageData(int width, int height, void *data)
-	: width(width), height(height), origin(IL_ORIGIN_UPPER_LEFT), bpp(4)
+	ImageData::ImageData(int width, int height, void * data)
 	{
+		this->width = width;
+		this->height = height;
 		create(width, height, data);
 	}
 
@@ -179,73 +79,82 @@ namespace devil
 		delete[] data;
 	}
 
-	int ImageData::getWidth() const
+	void ImageData::create(int width, int height, void * data)
 	{
-		return width;
-	}
+		try
+		{
+			this->data = new unsigned char[width*height*sizeof(pixel)];
+		}
+		catch (std::bad_alloc &)
+		{
+			throw love::Exception("Out of memory");
+		}
 
-	int ImageData::getHeight() const
-	{
-		return height;
+		if (data)
+			memcpy(this->data, data, width*height*sizeof(pixel));
 	}
 
-	void * ImageData::getData() const
+	void ImageData::load(Data * data)
 	{
-		return data;
-	}
+		Lock lock(devilMutex);
+		ILuint image;
+		ilGenImages(1, &image);
+		ilBindImage(image);
 
-	int ImageData::getSize() const
-	{
-		return width*height*bpp;
-	}
+		try
+		{
+			bool success = IL_TRUE == ilLoadL(IL_TYPE_UNKNOWN, (void*)data->getData(), data->getSize());
 
-	void ImageData::setPixel(int x, int y, pixel c)
-	{
-		Lock lock(mutex);
-		//int tx = x > width-1 ? width-1 : x;
-		//int ty = y > height-1 ? height-1 : y; // not using these seems to not break anything
-		if (x > width-1 || y > height-1 || x < 0 || y < 0)
-			throw love::Exception("Attempt to set out-of-range pixel!");
-
-		pixel * pixels = (pixel *)getData();
-		pixels[y*width+x] = c;
-	}
+			if (!success)
+				throw love::Exception("Could not decode image!");
 
-	pixel ImageData::getPixel(int x, int y)
-	{
-		Lock lock(mutex);
-		//int tx = x > width-1 ? width-1 : x;
-		//int ty = y > height-1 ? height-1 : y; // not using these seems to not break anything
-		if (x > width-1 || y > height-1 || x < 0 || y < 0)
-			throw love::Exception("Attempt to get out-of-range pixel!");
-
-		pixel * pixels = (pixel *)getData();
-		return pixels[y*width+x];
+			width = ilGetInteger(IL_IMAGE_WIDTH);
+			height = ilGetInteger(IL_IMAGE_HEIGHT);
+
+			// Make sure the image is in RGBA format.
+			ilConvertImage(IL_RGBA, IL_UNSIGNED_BYTE);
+
+			// This should always be four.
+			int bpp = ilGetInteger(IL_IMAGE_BPP);
+			if (bpp != sizeof(pixel))
+				throw love::Exception("Could not convert image!");
+
+			create(width, height, ilGetData());
+		}
+		catch (std::exception &)
+		{
+			ilDeleteImages(1, &image);
+			throw;
+		}
+
+		ilDeleteImages(1, &image);
 	}
 
-	void ImageData::encode(love::filesystem::File * f, ImageData::Format format) {
-		Lock lock(devilMutex);
+	void ImageData::encode(love::filesystem::File * f, ImageData::Format format)
+	{
+		Lock lock1(devilMutex);
 		Lock lock2(mutex);
 
 		ILuint tempimage;
 		ilGenImages(1, &tempimage);
 		ilBindImage(tempimage);
+		ilxClearErrors();
 
-		while (ilGetError() != IL_NO_ERROR);
-
-		bool success = ilTexImage(width, height, 1, bpp, IL_RGBA, IL_UNSIGNED_BYTE, this->data) == IL_TRUE;
-
-		ILenum err = ilGetError();
-		while (ilGetError() != IL_NO_ERROR);
+		ILubyte * encoded_data = NULL;
 
-		if (!success)
+		try
 		{
-			ilDeleteImages(1, &tempimage);
+			bool success = IL_TRUE == ilTexImage(width, height, 1, sizeof(pixel), IL_RGBA, IL_UNSIGNED_BYTE, this->data);
 
-			if (err != IL_NO_ERROR)
+			ILenum err = ilGetError();
+			ilxClearErrors();
+
+			if (!success)
 			{
-				switch (err)
+				if (err != IL_NO_ERROR)
 				{
+					switch (err)
+					{
 					case IL_ILLEGAL_OPERATION:
 						throw love::Exception("Illegal operation");
 					case IL_INVALID_PARAM:
@@ -254,17 +163,17 @@ namespace devil
 						throw love::Exception("Out of memory");
 					default:
 						throw love::Exception("Unknown error (%d)", (int) err);
+					}
 				}
-			}
 
-			throw love::Exception("Could not create image for the encoding!");
-		}
+				throw love::Exception("Could not create image for the encoding!");
+			}
 
-		ilRegisterOrigin(IL_ORIGIN_UPPER_LEFT);
+			ilRegisterOrigin(IL_ORIGIN_UPPER_LEFT);
 
-		ILuint ilFormat;
-		switch (format)
-		{
+			ILuint ilFormat;
+			switch (format)
+			{
 			case ImageData::FORMAT_BMP:
 				ilFormat = IL_BMP;
 				break;
@@ -281,33 +190,35 @@ namespace devil
 			default: // PNG is the default format
 				ilFormat = IL_PNG;
 				break;
-		}
+			}
 
-		ILuint size = ilSaveL(ilFormat, NULL, 0);
-		if (!size)
-		{
-			ilDeleteImages(1, &tempimage);
-			throw love::Exception("Could not encode image!");
-		}
+			ILuint size = ilSaveL(ilFormat, NULL, 0);
+			if (!size)
+				throw love::Exception("Could not encode image!");
 
-		ILubyte * encoded_data;
-		try
-		{
-			encoded_data = new ILubyte[size];
+			try
+			{
+				encoded_data = new ILubyte[size];
+			}
+			catch (std::bad_alloc &)
+			{
+				throw love::Exception("Out of memory");
+			}
+
+			ilSaveL(ilFormat, encoded_data, size);
+
+			f->open(love::filesystem::File::WRITE);
+			f->write(encoded_data, size);
+			f->close();
 		}
-		catch (std::bad_alloc)
+		catch (std::exception &)
 		{
 			ilDeleteImages(1, &tempimage);
-			throw love::Exception("Out of memory");
+			delete[] encoded_data;
+			throw;
 		}
 
-		ilSaveL(ilFormat, encoded_data, size);
 		ilDeleteImages(1, &tempimage);
-
-		f->open(love::filesystem::File::WRITE);
-		f->write(encoded_data, size);
-		f->close();
-
 		delete[] encoded_data;
 	}
 

src/modules/image/devil/ImageData.h

@@ -24,12 +24,10 @@
 // LOVE
 #include <filesystem/File.h>
 #include <image/ImageData.h>
-#include <thread/threads.h>
 
 // DevIL
 #include <IL/il.h>
 
-using love::thread::Mutex;
 
 namespace love
 {
@@ -37,35 +35,17 @@ namespace image
 {
 namespace devil
 {
+
 	class ImageData : public love::image::ImageData
 	{
 	private:
 
-		// The width of the image data.
-		int width;
-
-		// The height of the image data.
-		int height;
-
-		// The origin of the image.
-		int origin;
-
-		// The bits per pixel.
-		int bpp;
-
-		// The actual data
-		unsigned char *data;
-
-		// Create imagedata.
+		// Create imagedata. Initialize with data if not null.
 		void create(int width, int height, void * data = 0);
 
+		// Load an encoded format.
 		void load(Data * data);
 
-		// We need to be thread-safe
-		// so we lock when we're accessing our
-		// data
-		Mutex mutex;
-
 	public:
 
 		ImageData(Data * data);
@@ -74,15 +54,7 @@ namespace devil
 		ImageData(int width, int height, void *data);
 		virtual ~ImageData();
 
-		// Implements Data.
-		void * getData() const;
-		int getSize() const;
-
 		// Implements ImageData.
-		int getWidth() const ;
-		int getHeight() const ;
-		void setPixel(int x, int y, pixel c);
-		pixel getPixel(int x, int y);
 		void encode(love::filesystem::File * f, Format format);
 
 	}; // ImageData

src/modules/image/wrap_Image.cpp

@@ -33,12 +33,14 @@ namespace image
 
 	int w_newImageData(lua_State * L)
 	{
-
 		// Case 1: Integers.
 		if (lua_isnumber(L, 1))
 		{
 			int w = luaL_checkint(L, 1);
 			int h = luaL_checkint(L, 2);
+			if (w <= 0 || h <= 0)
+				return luaL_error(L, "Invalid image size.");
+
 			ImageData * t = 0;
 			try
 			{
@@ -46,7 +48,7 @@ namespace image
 			}
 			catch (love::Exception & e)
 			{
-				return luaL_error(L, e.what());
+				return luaL_error(L, "%s", e.what());
 			}
 			luax_newtype(L, "ImageData", IMAGE_IMAGE_DATA_T, (void*)t);
 			return 1;
@@ -63,7 +65,7 @@ namespace image
 			}
 			catch (love::Exception & e)
 			{
-				return luaL_error(L, e.what());
+				return luaL_error(L, "%s", e.what());
 			}
 			luax_newtype(L, "ImageData", IMAGE_IMAGE_DATA_T, (void*)t);
 			return 1;
@@ -84,7 +86,7 @@ namespace image
 		}
 		catch (love::Exception & e)
 		{
-			return luaL_error(L, e.what());
+			return luaL_error(L, "%s", e.what());
 		}
 		luax_newtype(L, "ImageData", IMAGE_IMAGE_DATA_T, (void*)t);
 		return 1;
@@ -111,7 +113,7 @@ namespace image
 			}
 			catch (Exception & e)
 			{
-				return luaL_error(L, e.what());
+				return luaL_error(L, "%s", e.what());
 			}
 		}
 		else

src/modules/image/wrap_ImageData.cpp

@@ -145,23 +145,28 @@ namespace image
 
 	int w_ImageData_encode(lua_State * L)
 	{
+		std::string ext;
+		const char * fmt;
+		ImageData::Format format = ImageData::FORMAT_MAX_ENUM;
 		ImageData * t = luax_checkimagedata(L, 1);
+
 		if (lua_isstring(L, 2))
 			luax_convobj(L, 2, "filesystem", "newFile");
 		love::filesystem::File * file = luax_checktype<love::filesystem::File>(L, 2, "File", FILESYSTEM_FILE_T);
-		std::string ext;
-		const char * fmt;
+
 		if (lua_isnoneornil(L, 3))
 		{
 			ext = file->getExtension();
 			fmt = ext.c_str();
+			ImageData::getConstant(fmt, format);
 		}
 		else
 		{
 			fmt = luaL_checkstring(L, 3);
+			if (!ImageData::getConstant(fmt, format))
+				luaL_error(L, "Invalid image format.");
 		}
-		ImageData::Format format = ImageData::FORMAT_PNG;
-		ImageData::getConstant(fmt, format);
+
 		try
 		{
 			t->encode(file, format);