Embedded images & tilesets with individual tiles #261

encukou · 2012-08-14T16:33:43Z

This adds basic support for images embedded in the XML file, and tilesets where each tile is backed by an individual image.

No UI is added for creating such tilesets, and the way they're displayed in the dock leaves much to be desired, especially if tiles are of different sizes.

To be honest, the patch is more about adding this to the TMX format rather than the GUI :) Here are the format changes:

Instead of <image source="..." />, you can say <image format="png"><data encoding="base64">...</data></image>.
Individual tiles may contain image tags. These override any image the tile would get from a tileset-wide ("tilesheet") image.
The tileset may leave "tilewidth" and "tileheight" unspecified if it is not using an external image.
Tags for individual tiles that aren't loaded from the "tilesheet" image must be ordered by their IDs. There may be no gaps. If there's no tileset timage, the first tile must have ID 0.

This means there's a couple of options for the TMX layout:

Tileset with external image (what Tiled does now)
Tileset with embedded "tilesheet" image
Tiles with individual external images
Tiles with individual embedded images (these patches add save support for this)
Tileset with both an "tilesheet" image and individual tile images (individual images override the tilesheet)

With these patches, Tiled should be able to read all of these, but it will only save the first or fourth. It will only save base64-encoded PNGs.
The editor will always set a tileset's "tilewidth" and "tileheight" to the largest of the individual tiles.

Helps with #103

bjorn · 2012-08-14T16:41:57Z

Just to make sure, since you're speaking of TMX format changes: are these changes compatible with Tiled Java, or did you devise a new way of storing embedded images and per-tile images?

Actually I would like Tiled Qt to support individual tile images, but I wanted to leave out support for embedded tile images, because this approach didn't make much sense to me. Do you have a use-case for it?

Btw: Thanks for putting your work into a pull request! :-)

encukou · 2012-08-14T16:55:06Z

Just to make sure, since you're speaking of TMX format changes: are these changes compatible with Tiled Java, or did you devise a new way of storing embedded images and per-tile images?

As compatible as practical. Tiled Java should be able to read this format. There is a restriction that Tiled Java doesn't have: the tiles have to be ordered by ID, starting at 0, with no gaps. (This is because tiles are loaded into a container that doesn't allow gaps.) The restriction can be lifted in the future, of course.

Actually I would like Tiled Qt to support individual tile images, but I wanted to leave out support for embedded tile images, because this approach didn't make much sense to me. Do you have a use-case for it?

Yes, I do. I'm making a library for programmatic manipulation of maps, such as "baking" several partially transparent tiles into one, if they overlap each other in the map. It makes sense to embed autogenerated images such as this, rather than having to mess with inventing filenames and managing lots of files (e.g. adding them to Git whenever they change).

encukou · 2012-08-14T23:21:14Z

To implement functionality similar to image objects (#235, #177), this would need:

a menu option to "Add tile" to a tileset (would just prompt for an image, make a tile, and call addTile: https://github.com/encukou/tiled/blob/eec7a31a/src/libtiled/tileset.cpp#L222)
each tile would remember if it has its own image, and the path to this image (if it's from a file), and that data would be used for saving instead of always embedding

Unfortunately my vacation is almost over so I probably won't get around to it soon :(

bjorn · 2012-08-25T10:08:23Z

src/tiled/tilesetmodel.cpp

+                return image.toImage().copy(
+                    -(mTileset->tileWidth() - image.width()) / 2,
+                    -(mTileset->tileHeight() - image.height()),
+                    mTileset->tileWidth(), mTileset->tileHeight());


Hmm, this is totally unacceptable for performance reasons. Converting the QPixmap to a QImage and then making another copy just to tweak the alignment. Instead, this should be implemented by the TileDelegate in tilesetview.cpp.

bjorn · 2012-08-25T10:09:46Z

src/libtiled/tileset.cpp

@@ -219,3 +219,60 @@ void Tileset::calculateTerrainDistances()
        // Repeat while we are still making new connections (could take a number of iterations for distant terrain types to connect)
    } while (bNewConnections);
 }
+
+void Tileset::addTile(Tile *newTile) {


Please keep with the coding style and place those { on a new line (same for the other functions added to Tileset).

bjorn · 2012-08-25T10:10:46Z

src/libtiled/tileset.cpp

+                }
+                if (mTileWidth < image.width()) {
+                    mTileWidth = image.width();
+                }


Please leave out { and } when you got only a single line body.

bjorn · 2012-08-25T10:15:48Z

src/libtiled/mapreader.cpp

+                readUnknownElement();
+            }
+        }
+    }else{


Please include spaces here, like so: } else {

bjorn · 2012-08-25T10:16:12Z

src/libtiled/mapreader.cpp

    QString trans = atts.value(QLatin1String("trans")).toString();
+    QString source = atts.value(QLatin1String("source")).toString();


Why change the order of these lines?

bjorn · 2012-08-25T10:27:20Z

src/libtiled/mapreader.cpp

+            if (haveImage && (tileWidth == 0 || tileHeight == 0)) {
+                xml.raiseError(tr("Invalid tileset parameters for tileset"
+                                " '%1'").arg(name));
+            }


This check needs to happen before readTilesetImage, or it needs to be moved inside that function. Otherwise you might hit the assertion at https://github.com/bjorn/tiled/blob/da6ece274e2cf18d8e4971e1f3b7f4708d1f622d/src/libtiled/tileset.cpp#L50

bjorn · 2012-08-25T10:43:42Z

src/libtiled/mapreader.cpp

        xml.raiseError(tr("Invalid tile ID: %1").arg(id));
        return;
+    } else if (id == tileset->tileCount()) {
+        tileset->addTile(new Tile(QPixmap(), id, tileset));


Rather than relying on the caller to specify the right tileset for the tile, I would prefer to have Tileset::addTile take care of creating the tile. So that this becomes:

tileset->addTile(id, QPixmap())

encukou · 2012-08-27T19:39:17Z

Rebased to current master, and addressed most of the comments. The hardest one still remains.

bjorn · 2012-08-27T20:51:02Z

src/libtiled/mapreader.cpp

+                const QXmlStreamAttributes atts = xml.attributes();
+                QString encoding = atts.value(QLatin1String("encoding"))
+                    .toString();
+                QByteArray data = xml.readElementText().toAscii();


The function toAscii was deprecated in Qt 5 because it doesn't always do what you'd think it does, since it depends on which codec is set for C strings. Please use QString::toLatin1 instead (also below).

bjorn · 2012-08-27T20:58:30Z

src/libtiled/mapwriter.cpp

+
+                QBuffer buffer;
+                tile->image().save(&buffer, "png");
+                w.writeCharacters(QString::fromAscii(buffer.data().toBase64()));


Similarly, this should be QString::fromLatin1.

bjorn · 2012-08-27T21:02:14Z

src/libtiled/tileset.cpp

+    }
+    if (mTileWidth < newTile->image().width()) {
+        mTileWidth = newTile->image().width();
+    }


Too many braces here. Also, you can replace newTile->image().height() etc. by just image.height() here.

bjorn · 2012-08-27T21:02:58Z

src/libtiled/tileset.cpp

+        }
+        if (maxHeight < tile->image().height()) {
+            maxHeight = tile->image().height();
+        }


Too many braces here. Also, the Tile class has width/height methods that forward to its image, please use those.

encukou · 2012-08-29T19:51:11Z

Thanks for your time! Here are some updated patches again.

bjorn · 2012-08-29T21:26:06Z

Looks very good now, nothing left that I want to complain about! :)

Just one question before I merge this. I noticed your first commit carries a different author email (@redhat.com). Was this intentional?

encukou · 2012-08-30T06:30:02Z

Wow, you have a sharp eye! Not intentional, fixed. (I took my work computer to vacation, and didn't set up Tiled's repo properly)

I also changed a comment and commit message in the last commit: the tiles are bottom-left aligned in their cells, instead of bottom-middle. This mimics how they appear on the map. The code already did this.

bjorn · 2012-08-30T21:10:17Z

src/libtiled/mapreader.cpp

@@ -378,17 +388,48 @@ void MapReaderPrivate::readTilesetImage(Tileset *tileset)
        tileset->setTransparentColor(QColor(trans));
    }

-    source = p->resolveReference(source, mPath);
-


Sorry I noticed one remaining problem with this part of the patch. This source path is "resolved", which means if source was relative it will now have the map path prepended to it. It is important that the resolved version is passed to the tileset because the map path will be forgotten later, which can cause problems on saving (where the relative reference would stay the same, regardless of where the map is saved - which pretty much only works when saving the map to the same location).

This patch currently causes the unresolved version to be passed into Tileset::loadFromImage.

I guess that was a rebase artifact. Thanks for noticing.

…PNG)

…age.

bjorn · 2012-08-31T20:31:08Z

Now the path gets resolved twice, but performance-wise that's rather insignificant.

Thanks for the patches!

encukou · 2012-08-31T20:57:50Z

Thank you for the thorough review!

bjorn reviewed Aug 25, 2012
View changes

src/libtiled/tileset.cpp

}

if (mTileWidth < image.width()) {

mTileWidth = image.width();

}

This comment has been minimized.

Sign in to view

bjorn Aug 25, 2012
Member

Please leave out { and } when you got only a single line body.

bjorn reviewed Aug 25, 2012
View changes

src/libtiled/mapreader.cpp

readUnknownElement();

}

}

}else{

This comment has been minimized.

Sign in to view

bjorn Aug 25, 2012
Member

Please include spaces here, like so: } else {

bjorn reviewed Aug 25, 2012
View changes

src/libtiled/mapreader.cpp

QString trans = atts.value(QLatin1String("trans")).toString();

QString source = atts.value(QLatin1String("source")).toString();

This comment has been minimized.

Sign in to view

bjorn Aug 25, 2012
Member

Why change the order of these lines?

bjorn reviewed Aug 25, 2012
View changes

bjorn mentioned this pull request Aug 25, 2012

Tiled image object (fix issue 177) #235

Closed

bjorn reviewed Aug 27, 2012
View changes

src/libtiled/mapwriter.cpp

QBuffer buffer;

tile->image().save(&buffer, "png");

w.writeCharacters(QString::fromAscii(buffer.data().toBase64()));

This comment has been minimized.

Sign in to view

bjorn Aug 27, 2012
Member

Similarly, this should be QString::fromLatin1.

bjorn reviewed Aug 27, 2012
View changes

bjorn reviewed Aug 30, 2012
View changes

encukou added 4 commits Aug 30, 2012

Read tilesets with individual tile images (external or inline base64-…

7305fe6

…PNG)

Write inline images for individual tiles for tilesets w/o external im…

078a1ef

…age.

Keep the max tile width/height as the tileset's tile dimensions

203fae2

Display small tiles bottom-left aligned

f833583

bjorn merged commit f833583 into mapeditor:master Aug 31, 2012

dxu mentioned this pull request Jun 26, 2017

Embed tileset images as base64 in Javascript export? #1625

Closed

parasyte mentioned this pull request Jul 16, 2017

Support "Collection of Images" tilesets (Tiled 0.10) melonjs/melonJS#895

Closed

mapeditor / tiled

Embedded images & tilesets with individual tiles #261

Embedded images & tilesets with individual tiles #261

encukou commented Aug 14, 2012

bjorn commented Aug 14, 2012

encukou commented Aug 14, 2012

encukou commented Aug 14, 2012

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

encukou commented Aug 27, 2012

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

encukou commented Aug 29, 2012

bjorn commented Aug 29, 2012

encukou commented Aug 30, 2012

This comment has been minimized.

This comment has been minimized.

bjorn commented Aug 31, 2012

encukou commented Aug 31, 2012

		QString trans = atts.value(QLatin1String("trans")).toString();
		QString source = atts.value(QLatin1String("source")).toString();

mapeditor / tiled

Sponsor mapeditor/tiled

Embedded images & tilesets with individual tiles #261

Embedded images & tilesets with individual tiles #261

Conversation

encukou commented Aug 14, 2012

bjorn commented Aug 14, 2012

encukou commented Aug 14, 2012

encukou commented Aug 14, 2012

This comment has been minimized.

bjorn Aug 25, 2012 Member

This comment has been minimized.

bjorn Aug 25, 2012 Member

This comment has been minimized.

bjorn Aug 25, 2012 Member

This comment has been minimized.

bjorn Aug 25, 2012 Member

This comment has been minimized.

bjorn Aug 25, 2012 Member

This comment has been minimized.

bjorn Aug 25, 2012 Member

This comment has been minimized.

bjorn Aug 25, 2012 Member

encukou commented Aug 27, 2012

This comment has been minimized.

bjorn Aug 27, 2012 Member

This comment has been minimized.

bjorn Aug 27, 2012 Member

This comment has been minimized.

bjorn Aug 27, 2012 Member

This comment has been minimized.

bjorn Aug 27, 2012 Member

encukou commented Aug 29, 2012

bjorn commented Aug 29, 2012

encukou commented Aug 30, 2012

This comment has been minimized.

bjorn Aug 30, 2012 Member

This comment has been minimized.

encukou Aug 31, 2012 Author Contributor

bjorn commented Aug 31, 2012

encukou commented Aug 31, 2012

bjorn Aug 25, 2012
Member

bjorn Aug 25, 2012
Member

bjorn Aug 25, 2012
Member

bjorn Aug 25, 2012
Member

bjorn Aug 25, 2012
Member

bjorn Aug 25, 2012
Member

bjorn Aug 25, 2012
Member

bjorn Aug 27, 2012
Member

bjorn Aug 27, 2012
Member

bjorn Aug 27, 2012
Member

bjorn Aug 27, 2012
Member

bjorn Aug 30, 2012
Member

encukou Aug 31, 2012
Author Contributor