Browse Source

Fix a crash in Image:replacePixels when a different ImageData is given than the one that created the Image.

Alex Szpakowski 7 years ago
parent
commit
22f72c7da6

+ 2 - 2
src/modules/graphics/Font.cpp

@@ -157,7 +157,7 @@ void Font::createTexture()
 	std::vector<uint8> emptydata(size.width * size.height * bpp, 0);
 	std::vector<uint8> emptydata(size.width * size.height * bpp, 0);
 
 
 	Rect rect = {0, 0, size.width, size.height};
 	Rect rect = {0, 0, size.width, size.height};
-	image->replacePixels(emptydata.data(), emptydata.size(), rect, 0, 0, false);
+	image->replacePixels(emptydata.data(), emptydata.size(), 0, 0, rect, false);
 
 
 	images.emplace_back(image, Acquire::NORETAIN);
 	images.emplace_back(image, Acquire::NORETAIN);
 
 
@@ -258,7 +258,7 @@ const Font::Glyph &Font::addGlyph(uint32 glyph)
 		g.texture = image;
 		g.texture = image;
 
 
 		Rect rect = {textureX, textureY, gd->getWidth(), gd->getHeight()};
 		Rect rect = {textureX, textureY, gd->getWidth(), gd->getHeight()};
-		image->replacePixels(gd->getData(), gd->getSize(), rect, 0, 0, false);
+		image->replacePixels(gd->getData(), gd->getSize(), 0, 0, rect, false);
 
 
 		double tX     = (double) textureX,     tY      = (double) textureY;
 		double tX     = (double) textureX,     tY      = (double) textureY;
 		double tWidth = (double) textureWidth, tHeight = (double) textureHeight;
 		double tWidth = (double) textureWidth, tHeight = (double) textureHeight;

+ 25 - 20
src/modules/graphics/Image.cpp

@@ -91,7 +91,7 @@ void Image::init(PixelFormat fmt, int w, int h, const Settings &settings)
 	if (isCompressed() && mipmapsType == MIPMAPS_GENERATED)
 	if (isCompressed() && mipmapsType == MIPMAPS_GENERATED)
 		mipmapsType = MIPMAPS_NONE;
 		mipmapsType = MIPMAPS_NONE;
 
 
-	mipmapCount = mipmapsType == MIPMAPS_NONE ? 1 : getTotalMipmapCount(w, h);
+	mipmapCount = mipmapsType == MIPMAPS_NONE ? 1 : getTotalMipmapCount(w, h, depth);
 
 
 	if (mipmapCount > 1)
 	if (mipmapCount > 1)
 		filter.mipmap = defaultMipmapFilter;
 		filter.mipmap = defaultMipmapFilter;
@@ -99,7 +99,7 @@ void Image::init(PixelFormat fmt, int w, int h, const Settings &settings)
 	initQuad();
 	initQuad();
 }
 }
 
 
-void Image::uploadImageData(love::image::ImageDataBase *d, int level, int slice)
+void Image::uploadImageData(love::image::ImageDataBase *d, int level, int slice, const Rect &rect)
 {
 {
 	love::image::ImageData *id = dynamic_cast<love::image::ImageData *>(d);
 	love::image::ImageData *id = dynamic_cast<love::image::ImageData *>(d);
 
 
@@ -107,11 +107,10 @@ void Image::uploadImageData(love::image::ImageDataBase *d, int level, int slice)
 	if (id != nullptr)
 	if (id != nullptr)
 		lock.setLock(id->getMutex());
 		lock.setLock(id->getMutex());
 
 
-	Rect rect = {0, 0, d->getWidth(), d->getHeight()};
-	uploadByteData(d->getFormat(), d->getData(), d->getSize(), rect, level, slice);
+	uploadByteData(d->getFormat(), d->getData(), d->getSize(), level, slice, rect);
 }
 }
 
 
-void Image::replacePixels(love::image::ImageDataBase *d, int slice, int mipmap, bool reloadmipmaps)
+void Image::replacePixels(love::image::ImageDataBase *d, int slice, int mipmap, const Rect &rect, bool reloadmipmaps)
 {
 {
 	// No effect if the texture hasn't been created yet.
 	// No effect if the texture hasn't been created yet.
 	if (getHandle() == 0 || usingDefaultTexture)
 	if (getHandle() == 0 || usingDefaultTexture)
@@ -121,13 +120,22 @@ void Image::replacePixels(love::image::ImageDataBase *d, int slice, int mipmap,
 		throw love::Exception("Pixel formats must match.");
 		throw love::Exception("Pixel formats must match.");
 
 
 	if (mipmap < 0 || (mipmapsType != MIPMAPS_DATA && mipmap > 0) || mipmap >= getMipmapCount())
 	if (mipmap < 0 || (mipmapsType != MIPMAPS_DATA && mipmap > 0) || mipmap >= getMipmapCount())
-		throw love::Exception("Invalid image mipmap index.");
+		throw love::Exception("Invalid image mipmap index %d.", mipmap + 1);
 
 
 	if (slice < 0 || (texType == TEXTURE_CUBE && slice >= 6)
 	if (slice < 0 || (texType == TEXTURE_CUBE && slice >= 6)
-		|| (texType == TEXTURE_VOLUME && slice >= std::max(getDepth() >> mipmap, 1))
+		|| (texType == TEXTURE_VOLUME && slice >= getDepth(mipmap))
 		|| (texType == TEXTURE_2D_ARRAY && slice >= getLayerCount()))
 		|| (texType == TEXTURE_2D_ARRAY && slice >= getLayerCount()))
 	{
 	{
-		throw love::Exception("Invalid image slice index.");
+		throw love::Exception("Invalid image slice index %d.", slice + 1);
+	}
+
+	int mipw = getPixelWidth(mipmap);
+	int miph = getPixelHeight(mipmap);
+
+	if (rect.x < 0 || rect.y < 0 || rect.w <= 0 || rect.h <= 0
+		|| (rect.x + rect.w) > mipw || (rect.y + rect.h) > miph)
+	{
+		throw love::Exception("Invalid rectangle dimensions (x=%d, y=%d, w=%d, h=%d) for %dx%d Image.", rect.x, rect.y, rect.w, rect.h, mipw, miph);
 	}
 	}
 
 
 	love::image::ImageDataBase *oldd = data.get(slice, mipmap);
 	love::image::ImageDataBase *oldd = data.get(slice, mipmap);
@@ -135,30 +143,27 @@ void Image::replacePixels(love::image::ImageDataBase *d, int slice, int mipmap,
 	if (oldd == nullptr)
 	if (oldd == nullptr)
 		throw love::Exception("Image does not store ImageData!");
 		throw love::Exception("Image does not store ImageData!");
 
 
-	int w = d->getWidth();
-	int h = d->getHeight();
+	Rect currect = {0, 0, oldd->getWidth(), oldd->getHeight()};
+	Rect newdrect = {0, 0, d->getWidth(), d->getHeight()};
 
 
-	if (w != oldd->getWidth() || h != oldd->getHeight())
-		throw love::Exception("Dimensions must match the texture's dimensions for the specified mipmap level.");
+	// We can only replace the internal Data (used when reloading due to setMode)
+	// if the dimensions match.
+	if (rect == currect && rect == newdrect)
+		data.set(slice, mipmap, d);
 
 
 	Graphics::flushStreamDrawsGlobal();
 	Graphics::flushStreamDrawsGlobal();
 
 
-	d->retain();
-	oldd->release();
-
-	data.set(slice, mipmap, d);
-
-	uploadImageData(d, mipmap, slice);
+	uploadImageData(d, mipmap, slice, rect);
 
 
 	if (reloadmipmaps && mipmap == 0 && getMipmapCount() > 1)
 	if (reloadmipmaps && mipmap == 0 && getMipmapCount() > 1)
 		generateMipmaps();
 		generateMipmaps();
 }
 }
 
 
-void Image::replacePixels(const void *data, size_t size, const Rect &rect, int slice, int mipmap, bool reloadmipmaps)
+void Image::replacePixels(const void *data, size_t size, int slice, int mipmap, const Rect &rect, bool reloadmipmaps)
 {
 {
 	Graphics::flushStreamDrawsGlobal();
 	Graphics::flushStreamDrawsGlobal();
 
 
-	uploadByteData(format, data, size, rect, mipmap, slice);
+	uploadByteData(format, data, size, mipmap, slice, rect);
 
 
 	if (reloadmipmaps && mipmap == 0 && getMipmapCount() > 1)
 	if (reloadmipmaps && mipmap == 0 && getMipmapCount() > 1)
 		generateMipmaps();
 		generateMipmaps();

+ 4 - 4
src/modules/graphics/Image.h

@@ -93,8 +93,8 @@ public:
 
 
 	virtual ~Image();
 	virtual ~Image();
 
 
-	void replacePixels(love::image::ImageDataBase *d, int slice, int mipmap, bool reloadmipmaps);
-	void replacePixels(const void *data, size_t size, const Rect &rect, int slice, int mipmap, bool reloadmipmaps);
+	void replacePixels(love::image::ImageDataBase *d, int slice, int mipmap, const Rect &rect, bool reloadmipmaps);
+	void replacePixels(const void *data, size_t size, int slice, int mipmap, const Rect &rect, bool reloadmipmaps);
 
 
 	bool isFormatLinear() const;
 	bool isFormatLinear() const;
 	bool isCompressed() const;
 	bool isCompressed() const;
@@ -112,8 +112,8 @@ protected:
 	Image(const Slices &data, const Settings &settings);
 	Image(const Slices &data, const Settings &settings);
 	Image(TextureType textype, PixelFormat format, int width, int height, int slices, const Settings &settings);
 	Image(TextureType textype, PixelFormat format, int width, int height, int slices, const Settings &settings);
 
 
-	void uploadImageData(love::image::ImageDataBase *d, int level, int slice);
-	virtual void uploadByteData(PixelFormat pixelformat, const void *data, size_t size, const Rect &rect, int level, int slice) = 0;
+	void uploadImageData(love::image::ImageDataBase *d, int level, int slice, const Rect &rect);
+	virtual void uploadByteData(PixelFormat pixelformat, const void *data, size_t size, int level, int slice, const Rect &r) = 0;
 
 
 	virtual void generateMipmaps() = 0;
 	virtual void generateMipmaps() = 0;
 
 

+ 2 - 2
src/modules/graphics/Video.cpp

@@ -88,7 +88,7 @@ Video::Video(Graphics *gfx, love::video::VideoStream *stream, float dpiscale)
 		size_t size = bpp * widths[i] * heights[i];
 		size_t size = bpp * widths[i] * heights[i];
 
 
 		Rect rect = {0, 0, widths[i], heights[i]};
 		Rect rect = {0, 0, widths[i], heights[i]};
-		img->replacePixels(data[i], size, rect, 0, 0, false);
+		img->replacePixels(data[i], size, 0, 0, rect, false);
 
 
 		images[i].set(img, Acquire::NORETAIN);
 		images[i].set(img, Acquire::NORETAIN);
 	}
 	}
@@ -163,7 +163,7 @@ void Video::update()
 			size_t size = bpp * widths[i] * heights[i];
 			size_t size = bpp * widths[i] * heights[i];
 
 
 			Rect rect = {0, 0, widths[i], heights[i]};
 			Rect rect = {0, 0, widths[i], heights[i]};
-			images[i]->replacePixels(data[i], size, rect, 0, 0, false);
+			images[i]->replacePixels(data[i], size, 0, 0, rect, false);
 		}
 		}
 	}
 	}
 }
 }

+ 6 - 3
src/modules/graphics/opengl/Image.cpp

@@ -85,7 +85,7 @@ void Image::loadDefaultTexture()
 	int slices = texType == TEXTURE_CUBE ? 6 : 1;
 	int slices = texType == TEXTURE_CUBE ? 6 : 1;
 	Rect rect = {0, 0, 2, 2};
 	Rect rect = {0, 0, 2, 2};
 	for (int slice = 0; slice < slices; slice++)
 	for (int slice = 0; slice < slices; slice++)
-		uploadByteData(PIXELFORMAT_RGBA8, px, sizeof(px), rect, 0, slice);
+		uploadByteData(PIXELFORMAT_RGBA8, px, sizeof(px), 0, slice, rect);
 }
 }
 
 
 void Image::loadData()
 void Image::loadData()
@@ -133,7 +133,10 @@ void Image::loadData()
 			love::image::ImageDataBase *id = data.get(slice, mip);
 			love::image::ImageDataBase *id = data.get(slice, mip);
 
 
 			if (id != nullptr)
 			if (id != nullptr)
-				uploadImageData(id, mip, slice);
+			{
+				Rect r = {0, 0, id->getWidth(), id->getHeight()};
+				uploadImageData(id, mip, slice, r);
+			}
 		}
 		}
 
 
 		w = std::max(w / 2, 1);
 		w = std::max(w / 2, 1);
@@ -147,7 +150,7 @@ void Image::loadData()
 		generateMipmaps();
 		generateMipmaps();
 }
 }
 
 
-void Image::uploadByteData(PixelFormat pixelformat, const void *data, size_t size, const Rect &r, int level, int slice)
+void Image::uploadByteData(PixelFormat pixelformat, const void *data, size_t size, int level, int slice, const Rect &r)
 {
 {
 	OpenGL::TempDebugGroup debuggroup("Image data upload");
 	OpenGL::TempDebugGroup debuggroup("Image data upload");
 
 

+ 1 - 1
src/modules/graphics/opengl/Image.h

@@ -59,7 +59,7 @@ public:
 
 
 private:
 private:
 
 
-	void uploadByteData(PixelFormat pixelformat, const void *data, size_t size, const Rect &rect, int level, int slice) override;
+	void uploadByteData(PixelFormat pixelformat, const void *data, size_t size, int level, int slice, const Rect &r) override;
 	void generateMipmaps() override;
 	void generateMipmaps() override;
 
 
 	void loadDefaultTexture();
 	void loadDefaultTexture();

+ 1 - 6
src/modules/graphics/wrap_Canvas.cpp

@@ -92,12 +92,7 @@ int w_Canvas_newImageData(lua_State *L)
 
 
 	int slice = 0;
 	int slice = 0;
 	int mipmap = 0;
 	int mipmap = 0;
-
-	Rect rect;
-	rect.x = 0;
-	rect.y = 0;
-	rect.w = canvas->getPixelWidth();
-	rect.h = canvas->getPixelHeight();
+	Rect rect = {0, 0, canvas->getPixelWidth(), canvas->getPixelHeight()};
 
 
 	if (!lua_isnoneornil(L, 2))
 	if (!lua_isnoneornil(L, 2))
 	{
 	{

+ 2 - 1
src/modules/graphics/wrap_Image.cpp

@@ -52,6 +52,7 @@ int w_Image_replacePixels(lua_State *L)
 
 
 	int slice = 0;
 	int slice = 0;
 	int mipmap = 0;
 	int mipmap = 0;
+	Rect r = {0, 0, id->getWidth(), id->getHeight()};
 	bool reloadmipmaps = i->getMipmapsType() == Image::MIPMAPS_GENERATED;
 	bool reloadmipmaps = i->getMipmapsType() == Image::MIPMAPS_GENERATED;
 
 
 	if (i->getTextureType() != TEXTURE_2D)
 	if (i->getTextureType() != TEXTURE_2D)
@@ -63,7 +64,7 @@ int w_Image_replacePixels(lua_State *L)
 	else if (!reloadmipmaps)
 	else if (!reloadmipmaps)
 		mipmap = (int) luaL_optinteger(L, 3, 1) - 1;
 		mipmap = (int) luaL_optinteger(L, 3, 1) - 1;
 
 
-	luax_catchexcept(L, [&](){ i->replacePixels(id, slice, mipmap, reloadmipmaps); });
+	luax_catchexcept(L, [&](){ i->replacePixels(id, slice, mipmap, r, reloadmipmaps); });
 	return 0;
 	return 0;
 }
 }