Scripting: Unexpected results when using Image for tiles

[eishiya]

These are really several issues, but they're all about roughly the same thing: embedding image data directly into tilesets via scripting.

1. Tileset.loadFromImage(image) creates an Image Collection in which every tile has the same source image, but uses different subrects of it. This is quite unexpected, I'd have thought this would set the Tileset's source image instead. Saving a Tileset created this way to a file duplicates the entire image (base64-encoded) for each tile. I'm guessing this is because there is no source image path and Tiled treats every tileset with no image path as an Image Collection, but this is one case where it should not.
   • @djedditt Reported a related issue where large embedded tilesets created this way would freeze Tiled when copy+pasting tiles from them. I suspect this is due to the large image being duplicated for each copy of the tile, and then potentially duplicated to the preview brush.
2. Tile.setImage() requires that the tile's imageRect be set first. If it's left at the default (presumably 0,0,0,0), setImage() seems to fail, and the tile is not written out when the Tileset is saved (in TSX; see (5) below).
3. The documentation for Tile.setImage() warns that it "does not affect the saved tileset!". It does though, and the image data is embedded correctly (in TSX, anyway). What it doesn't affect is Tiles that have a source image path, as far as I can tell. So, setImage() cannot be used to modify tiles that were created normally. I have not looked into what happens if one tries setImage() on a tile that was created with setImage() :]
4. Because Tile.setImage() does not affect normal tiles, it means it cannot be used to replace the image of a tile. This means that if I want to create a script that embeds all the images of a tileset, I have to either create a whole new tileset and put the tiles there, or create new tiles and delete the original ones. Either way, tile IDs are likely to change, breaking existing maps. Some way to override an existing tile's image with setImage() would be nice.
5. TSX files will store the embedded images as base64 data, but TSJ files ditch them entirely. This means TSJ files lack a feature that TSX files have, when they should have exact feature parity. In addition, there seems to be no way for scripted formats to access images set via Tileset.loadFromImage and Tile.setImage, so they'll have the same problem as TSJ.

Edit: I also experimented with ImageLayer.setImage and ran into several issues:

• The map view is not updated on setImage, you have to change document tabs and back to see the result.
• If no source path is set, the image is not embedded into the file, even in TMX.
• If a source path is set, it's saved as an absolute path instead of being converted to a relative path where possible. I noticed that for me on Windows, the volume letter was lower-case rather than upper-case, maybe this got in the way of resolving the path somehow?
• The docs claim ImageLayer.imageSource is a string, but it's some sort of object, probably a QUrl. It being a QUrl also means it needs to be processed before it can be used as a path.

[eishiya]

To aid in testing this stuff, here is the script I discovered most of these issues with. It adds an "Embed Images" action to the Tileset menu, which attempts to embed any external images into the tileset, using loadFromImage for tilesheets, and setImage for Image Collections:
Edit: Included ImageLayer "support" for testing that as well.

/* 	Embed Tileset Image by eishiya, last updated 21 Mar 2023

	Adds an action to the Tileset and Map menus that lets you embed an image
	into the tileset or map directly. This makes the tileset easier
	to transfer to other devices, but harder to modify.
	
	This script should NOT be used in production! Image Layer embedding does not
	actually work, and Tileset image embedding does not work as expected.
	In addition, most of these changes are not reversible except through further
	scripting! Most custom formats can't access the embedded images, and most
	Tiled map/tileset parsers can't read them.
	
	Requires Tiled 1.10+ to work correctly with Image Collections, but should
	work for other document types otherwise.
	
	WARNING: For Image Collections, *new* tiles have to be created in order
	to embed the image. This will break the tile IDs! If you use this script
	on Image Collections, make sure you do it before you use the tiles anywhere.
*/

var embedImages = tiled.registerAction("EmbedImages", function(action) {
	let asset = tiled.activeAsset;
	if(!asset)
		return;
	if(asset.isTileset) {
		if(asset.isCollection) {
			let tiles = asset.tiles;
			let tilesToRemove = [];
			for(tile of tiles) {
				let image = new Image(tile.imageFileName);
				if(image && image.width * image.height > 0) {
					//tile.setImage(image); //does nothing to existing tiles.
					let newTile = asset.addTile();
					newTile.imageRect = tile.imageRect;
					newTile.setImage(image);
					tilesToRemove.push(tile);
				} else {
					tiled.log("Warning: Tile "+tile.id+" in tileset '"+asset.name+"' has an invalid image file name. It may have already been embedded.");
				}
			}
			asset.removeTiles(tilesToRemove);
		} else { //image-based tileset;
			if(!asset.image || asset.image.length < 1) {
				tiled.error("Embed failed: This tileset has no image set.");
				return;
			}
			let image = new Image(asset.image);
			if(!image || image.width * image.height < 1) {
				tiled.error("Embed failed: The image could not be loaded from file.");
				return;
			}
			asset.loadFromImage(image);
		}
		
	} else if(asset.isTileMap) {
		//tiled.warn("Embedding Image Layer images is not implemented yet.");
		function embedImageLayers(layer) {
			if(layer.isImageLayer) {
				let path = layer.imageSource.toString().replace(/^(file:\/{3})/,"");
				let image = new Image(path);
				if(image && image.width * image.height > 0) {
					//layer.setImage(image, path);
					layer.setImage(image);
				} else {
					tiled.log("Warning: Image Layer "+layer.name+" has an invalid image file name. It may have already been embedded.");
				}
			} else if(layer.isTileMap || layer.isGroupLayer) {
				for(let gi = 0; gi < layer.layerCount; ++gi) {
					embedImageLayers(layer.layerAt(gi));
				}
			}
		}
		embedImageLayers(asset);
	}

});
embedImages.text = "Embed Images";

tiled.extendMenu("Tileset", [
	{ action: "EmbedImages", before: "TilesetProperties" },
	{separator: true}
]);
tiled.extendMenu("Map", [
	{ action: "EmbedImages", before: "MapProperties" },
	{separator: true}
]);

[bjorn]

#3841 addressed part of the reported issues, but reading the list again, there are still some things that could be easily addressed, like the documentation for Tile.setImage() and possibly the updating of the map view when setting images directly.

[bjorn]

Alright, going over the original list of issues:

1. Tileset.loadFromImage(image) no longer creates an image collection, since an image collection is now considered a tileset without image, rather than just without image file name. I think the freeze should also be resolved, since the image now gets embedded only once, at the tileset.
2. I haven't tested this, but it shouldn't be necessary to first set imageRect. At least, Tile.setImage should initialize or adjust this property automatically.
3. Documentation of Tile.setImage was fixed in Scripting: Added Tile.image for accessing a tile's image data #3882.
4. Does Tile.setImage still not affect normal tiles? If so I wonder why.
5. JSON format still does not support embedded images for now.

Then regarding image layers:

• The map view is still not immediately updated when setting the image directly... should be fixed now in 95a5cb3.
• The image is now embedded, when no source is set.
• I haven't checked the relative path issue.
• Docs for ImageLayer.imageSource were adjusted to make it clear it's a Qt.Url.

[bjorn]

@eishiya Btw, your script complains about my image layer, since its regexp doesn't work in my case:

> tiled.activeAsset.currentLayer.imageSource
$1 = file:///home/bjorn/tiled-tests/tmw_desert_spacing.png
> $1.toString().replace(/^(file:\/{3})/,"");
$2 = home/bjorn/tiled-tests/tmw_desert_spacing.png

It strips away one too many /. Was your URL looking differently? It seems like on Linux it should be /^(file:\/{2})/ instead. Maybe I should just also add an ImageLayer.imageFileName that just returns a string.

[eishiya]

On Windows, you get e.g. file:///D:/myImage.png. So, I guess the regex "trick" is Windows-only, and there are probably multiple scripts out there (not just mine) that won't work on Linux. An OS-agnostic way to get a usable path would be nice.

Are QUrls ever actually useful in the Tiled API? Every path parameter seems to take a string path, so if possible, I think everything that returns a path should return a string path, not a QUrl.
Edit: I guess QUrls would be useful if Tiled ever gains the ability to load URLs that aren't file://

Do you want me to test the changes so far? I won't be able to today, but I can take a look another day.

[bjorn]

Are QUrls ever actually useful in the Tiled API? Every path parameter seems to take a string path, so if possible, I think everything that returns a path should return a string path, not a QUrl.

I guess not... it's just that internally QUrl is used, because somebody at some point requested the ability to refer to online resources directly. I even had a halfway working patch for loading the images from the network. But they stopped sponsoring the feature before it was done and it never came up anywhere else so it wasn't a priority. I think I'm glad I don't need to maintain that functionality now, anyway.

Do you want me to test the changes so far? I won't be able to today, but I can take a look another day.

It would be a great help, but if not I'll look into testing things a little tomorrow. For these changes, testing is more work than the implementation. :-)

[bjorn]

4. Does Tile.setImage still not affect normal tiles? If so I wonder why.

I've tested this with the help of your script. The difference with the other setImage functions is that this one just changes the image, not any image file name that might be set on the tile. Hence, when saving the tileset, the map writer still considers the images external.

The embedding does work if I add tile.imageFileName = ""; before the setImage call. But for consistency, the Tile.setImage function should probably also take a source path.

[bjorn]

• If a source path is set, it's saved as an absolute path instead of being converted to a relative path where possible. I noticed that for me on Windows, the volume letter was lower-case rather than upper-case, maybe this got in the way of resolving the path somehow?

I think this is the only remaining issue I haven't looked into. If you can reproduce this, feel free to open a new issue about it.