Add support for Tileset Atlas

[deathbeam]

Useful for y-sorting, multi tile objects, base tile images changing, etc.

• Add new "atlas" property to tileset that determines if tileset is atlas tileset or not
• Atlas property is set on tileset creation through type
• When atlas tileset is loaded, if it doesnt have any image rects image rects for every tile on tileset will be generated
• Atlas tileset always saves all image rects

For controls, go to rearrange tiles mode:

• Left click draging on existing tile starts moving it, releasing saves
• Left click dragging on empty space starts selection for creating new tile
• Right cick dragging anywhere starts selection for deleting all tiles overlapping the selection
• Holding shift before pressing right or left click will disable snapping to grid which allows any sized drawing

[eishiya]

This sounds like a partial implementation of the Tile Atlas concept discussed in #2863? Image rects were added as a first step towards this. You may want to see the discussion there about why this should probably be a separate tileset type rather than just extending image rects to "Based on Tileset Image" tilesets.

[deathbeam]

Looking through it, yea. Even though I think you are overcomplicating it a bit there with the off-grid tiles like the 16x19 example there as that one also fits in 16x32 so the grid display can always just remain in the regular baseline grid set on the tileset, so dont think new view is even needed.

New tileset type, I guess the benefit would be to just inherently save all the image recs instead of saving just the custom ones? which means image recs would be able to also get removed (which would be ideal for merging tiles, which is the feature im trying to replicate here from godot and I think godot mostly works like that anyway, and I think you are also talking about ending on something like that eventually).

[deathbeam]

Ok got some progress done, I for now added boolean "atlas" on the tileset and adjusted how everything is rendered when set to atlas (grid is no longer used at all and every image rect is drawn directly, and indexAt also uses image rects for resolving tiles they are clicking on). Also added ability to delete tiles, add and merge is something I want to look at next after resolving some rendering quirks this still has

[deathbeam]

ok switched back to grid view for simplicity and grid spans, now I can delete tiles without breaking grid and selection also works perfectly. Just need logic for adding the tiles back after they are deleted i guess

Also works with animated tiles out of the box:

[deathbeam]

I think as far as functionality goes i think i have most of stuff working properly now

Issues:

• atm im kinda bastardizing rearrange tiles mode for splitting existing multi tile tiles and readding removed tiles back. I think mode like this is ideal tho, but maybe it should be called differently when in tile atlas mode. And maybe it should remove tiles on right click, add on left click, not sure
• merge multiple tiles into one is only done manually atm, ideally this would be hold and drag in rearrange tiles mode I think
• after the above is done I think disabling manual input to the image rect would be best
• no way to regenerate all the image recs from scratch, currently they are generated when tileset is opened without any image rects (maybe new button, or maybe not even needed)

[deathbeam]

Ok got merging and everything else done, for now in Rearrange Tiles mode:

• Tiles can be merged by holding down left click (this creates multi tile tile)
• Tiles can be split by right clicking on multi tile edges
• Tiles can be removed from atlas by right clicking single tiles (so needs to be split first then removed) - current limitation, ideally right click hold would delete multiple, but this is probably not super common use case and the visual seection doesnt work for right click, QT limitation maybe?
• Tiles can be added back to atlas by left clicking empty tiles

[deathbeam]

Ok I think this is now properly ready for review, I was doing some testing since yesterday and I think i fixed everything I found and im happy with the implementation. I did not adjusted documentation or stuff like python bindings etc, I think that can be done as finishing touches after this is reviewed and if the implementation even makes sense (I tried my best but I was not familiar with Tiled codebase at all until I started working on this PR).

[bjorn]

First off, I really appreciate the effort you're putting into this, @deathbeam! It worked quite well in my tests and it's nice to be able to quickly merge and split the tiles. Things I noticed:

• There's an odd issue when toggling the "Rearrange Tiles" option in one tileset and then switching to another tileset, which then shows the toggled button but isn't in the rearrange mode. However, I noticed this is an existing issue which should be fixed in a separate PR.

• There's an issue with adjusting the draw margins which might be related to the maximum tile size not getting updated for atlas tilesetes? I'm not sure why Tileset::updateTileSize does an early return in this case. This information is used in Map::recomputeDrawMargins to make sure enough of the view is updated. I think there is also an existing issue with this mechanism, but not sure if it's the same.

• If I click and drag a small amount on a tile and then click another tile, it decides to merge the tiles. I think intended behavior is to only merge when I do a single drag covering multiple tiles.

• The dynamic wrapping mode doesn't work for atlas tilesets (which couldn't be combined with the "rearrange tiles" mode, at the very least). It does work for image collection tilesets and I feel it should work for atlas tilesets while not rearranging them.

• I feel like splitting/merging of tiles should be a separate mode from "Rearrange Tiles" mode. There doesn't appear to be a reason the original Rearrange Tiles mode couldn't be supported for atlas tilesets (though probably only in "dynamic wrapping" mode).

Unfortunately, apart from these small issues this approach is not what we (or I) had planned for the atlas tileset. It was meant to cover not just grid-aligned sub-rects but allow for tiles of any size (to resolve #1008). I realize technically you could do this in the current solution with a base tile size of 1x1, but that doesn't seem ideal.

Another issue is the automatic assignment of tile IDs based on the location. This way, it is not possible to move the tile rects in response to a rearranged tileset image while keeping the tile IDs the same. In addition, using a "12+12 bit spatial hash" creates huge gaps in the IDs and ends up placing a bunch of arbitrary limits (not just the 4096x4096 tile coordinates limit but since we have only 28 bits (+4 flags) available for a global tile ID, also limiting the amount of such tilesets in a map to 16). These limits are probably reasonable, but I think if such ID management is desired it's better added as a separate option.

I'm a little undecided how to move forward with this. Even if we fix the above issues and move the ID assignment to a separate option, the tile grid limitation remains and forms the basis of the entire implementation. Yet, the file format does not have this limitation, so the tileset view could be replaced later to enable per-pixel tile rects to be drawn or dragged around. Alternatively we keep this PR open for now until we've found time to support that as well. Let me know your thoughts.

[deathbeam]

Thanks for reviewing this and testing it.

1. Yea noticed this issue as well but agree with what you said, did not wanted to fix this in this PR
2. The reason why I skip update tile size for atlas is because it adjusts the base grid and breaks the view as the idea is based on not changing the grid size even with larger tiles. I think the solution would be to find maximum imageRect size instead of just atlas tileSize

3. Hmm yea I am probably not resetting the selection properly, should probably be easy fix
4. I can look into this, but I can see it causing issues when selecting tiles, but it probs wont
5. The reason why i did not addded support for rearrange is to keep stable ids when adding/deleting tiles, e.g having them based on x/y (as otherwise adding tile, deleting it and then adding it back results with different tile id). With rearrange this breaks (also its kinda hard to rearrange multi tile tiles because they are still just for example 16x16 tile and i delete everything that is nearby and set span on the grid. which doesnt rly work well when the tile can be rearranged)

For the implementation, yea I intentionally decided to diverge from allowing any tile size tiles (even thouhg I initially tried to do that) because that makes implementing UI for it kinda nightmarish and technically drawing offsets per tile + this can cover #1008 in a bit more cumbersome way or the 1x1 tile size (but I think its less cumbersome compared to the drawbacks of making UI that supports tiles of any size). Even though the 1x1 tile size idea is kinda interesting, technically the atlas could work on 1x1 tile size while pre-generating tiles with the desired tile size, but not sure how big performance hit will this have, I can try to maybe look at that.

For the auto assignment, it has drawbacks but I believe that the IDs remainign stable on add/delete is a lot more important with functionality that allows to freely resize/remove/add them. But the limits are unfortunate, but I couldnt think of a better way to store the IDs while not breaking firstgid. I guess this being toggle that could be specific for atlas tilesets is interesting idea, did not considered that. But even with that I still dont think supporting rearranging is good idea due to grid and tile merging.

Just as side node, the Godot 4 feature that I was trying to mostly replicate has also exactly same limitations I believe (maybe except ID space as that is mostly caused by firstgid) but I think the workflow is still very good and better than trying to support every possible scenario which makes the UX a lot worse.

For final thoughts, as you said the data format supports doing much more than this, which means extending the UI in future also shouldnt break stuff but I think waiting till the perfect UI for it is made is also not the best idea and I think the worklow this UI supports is the most common use case of feature like this, but I can try to implement what I can based on the comments.

So for summary:

1. Toggle bug - Can be fixed in separate PR
2. Draw margins - I can fix this somewhat easily without the dynamic tile updates
3. Selection not resetting -Bug I can fix
4. Dynamic wrapping outside of rearrange mode - I can most likely fix this
5. Allowing rearranging tiles and separate mode - I dont think allowing rearranging is good idea
6. Any tile size - 1x1 working grid, generation grid can make this possible with current solution (configurable maybe?)
7. Tile id generation - Toggle for old behaviour could work

[deathbeam]

Ok tried 1x1 grid and not even my PC could handle that (even though maybe that was related to span performance issue i just fixed, EDIT: nvm still way too slow). For now enabled editing the rectangle at least and fixed the selection bug.

[deathbeam]

So fixed 1, 2, 3, 4 and 7.

Partially fixed 6 by allowing the rectangle to be editable in the properties window and making sure the spans at least try to support it properly (works somewhat well).

For 7. Decided to just remove the new tile ID format compltely and use the old old. Even if I think that the new tile ID format is better for atlas tilesets, maybe that is something that can be done in separate PR to not add extra complexity to this PR when its not absolutely necessary

And for 5. I am still not sure how rearranging can be supported in nice way, maybe as you said only in dynamic mode but even then I need to resolve what to display based on row/col and image rect, and when repositioning this breaks, but i will try to figure something out (maybe just swapping ids is enough i guess, whats rendered would not change on the atlas itself, just when the tile is referenced elsewhere, which is the biggest benefit of swapping anyway i guess?)

[dogboydog]

I didn't get the chance to try this yet, but...

Unfortunately, apart from these small issues this approach is not what we (or I) had planned for the atlas tileset. It was meant to cover not just grid-aligned sub-rects but allow for tiles of any size (to resolve #1008). I realize technically you could do this in the current solution with a base tile size of 1x1, but that doesn't seem ideal.

This is basically my only use case for having an atlas tileset - arbitrary tile size, to allow using a sprite atlas as a tileset where the individual tiles have been packed to take up the least amount of space rather than being in a grid. So it'd be too bad if this feature wouldn't allow that setup. The current workaround I use is having an image collection tileset plus the sub rect feature. Others mention sprite atlases in the linked issue as well, so they'd be similarly prevented from using the feature

[deathbeam]

im also trying slightly different approach where the tileset is just 1 cell on the "grid" and it just draws whole tileset and tiles are just selection on the tileset and nothing else. This makes any sized rects easy, and grid somewhat too because i can just auto snap to grid. Downside is that spacing cannot really work with this I think very easily, margin still might tho.

EDIT:

got it to mostly work but making this work with everything else as well is rly annoying, unlike the grid based implementation that just works ™️

[deathbeam]

Ok pushed changes with different way of doing this in commit on top of the original changes, which supports moving tiles, drawing any size tiles and adding/deleting in the rearrange tiles mode (but no merging and splitting, needs to be done manually by deleting first then recreating etc, which is unfortunate but its not easy to support everything at once).

Currently that is all that is supported (animation and map editor view doesnt work with it yet, selection simply doesnt propagate due to how this is implemented and I did not bothered with trying to support dynamic wrap yet either), just want to get some feedback on the UX. But it should support everyones use cases I believe.

For controls, go to rearrange tiles mode:

• Left click draging on existing tile starts moving it, releasing saves
• Left click dragging on empty space starts selection for creating new tile
• Right cick dragging anywhere starts selection for deleting all tiles overlapping the selection
• Holding shift before pressing right or left click will disable snapping to grid which allows any sized drawing (some editors do this the other way around but I think its better to snap by default for better UX)

[deathbeam]

Ok I believe I resolved all issues except dynamic view now:

Im not sure if supporting dynamic view for this new approach is even worth it, maybe? But it would also be a lot simpler to just disable it for tileset view :d

Would appreciate if someone other than me also checked the new UX and if everything works fine outside of dynamic view