tiled
tiled copied to clipboard
[WIP] Tile Size per Tile Layer
Hey Bjorn, so this is the new PR for a redo of the Tile Size by Layer PR.
I'd like to keep that other one up for now because I use it as a refresher on what how things connect to each other in Tiled.
This is kind of an early PR to see if this is heading the right direction. I mostly just want your thoughts whenever you have time to spare from GSoC.
What I've done
I made a class called WorkSpace
which stores tile sizes and sizes (possibly more later). The MapDocument
now has a function called currentWorkSpace
which returns a WorkSpace
with TileLayer
specs if a TileLayer
is active and the Map
specs if not.
Then I pass that WorkSpace
everywhere that needs the current map's tile sizes.
All the MapRenderer
methods now take a WorkSpace
and use that instead of the Map
. Later, I think we could get rid of the Map
dependency and just use a WorkSpace
(or something similar)?
The grids are now according to the current work space's tile size and size.
There's a Resize Layer option in the Layer menu now, and it reuses the resize dialog the Map
uses. The Map
basically only works as the default tile size and size for TileLayer
. No layers change when a Map
is resized.
I'm thinking of having a "Sync with Map" toggle in the properties menu. Thoughts on that?
What I haven't done
- Updated the .py plugins
I hope this is a better design than what I had last time!
Luca
In english 'workspace' is one word, therefore it should actually be 'Workspace' instead of 'WorkSpace' ;)
I'm thinking of having a "Sync with Map" toggle in the properties menu. Thoughts on that?
With layer size being independent from the map size, I think we need a selection like the following in the Resize Map dialog (screenshot from GIMP):
Btw, please delete the following files from the patch:
.qmake.stash
run
*/moc_predefs.h
All the MapRenderer methods now take a Workspace and use that instead of the Map. Later, I think we could get rid of the Map dependency and just use a Workspace (or something similar)?
Please try to explain why you introduced the Workspace class. What problem is it solving? I just hope we can do something less intrusive or something more incremental to work towards this feature.
For example, a lot of code seems to have changed because the Workspace needs to be passed to the functions of MapRenderer. But the MapRenderer is generally owned by the MapDocument, so it may be possible for the MapDocument to apply the Workspace (or just current layer) to the MapRenderer whenever it changes. Of course it may need to send out some kind of workspaceChanged signal as well so that appropriate repaint requests can be done by the MapScene.
And some functions, like drawTileSelection
, should probably take a TileLayer as argument instead. It is only used from places where passing the relevant tile layer is generally not a problem. This depends somewhat of course, on what happens to tile selection. Do you have an idea what should happen after selecting some tiles, and then switching to a layer with a different tile size? I think we may need to associate selection with tile layers.
I'd want to try to rely on the Map fallback as little as possible, because eventually I think the tile size on the map should be just removed, or at best be kept as a "default for new tile layers". Of course, some way of changing the tile size for all layers (or some set of layers) needs to remain. But can you shed some light on where this fallback is currently really necessary? Is this just for grid rendering and grid snapping for objects?
Hey @bjorn thanks for the feedback, I've been working on other stuff, I can do a little work on this here and there if no one wants to take over.
Anyways, the reason for the workspace class was to go along with your suggestion about using a "GridParameters" or "GridSize" to pass along into the draw functions.
But yeah, I found it pretty intrusive as well since most of the time the map renderer is owned by MapDocument. The main reason I thought just passing it in a parameter would be nice is because I'm sure that the correct grid size is used every single time the map is rendered. I was also worried about the renderers that are not owned by MapDocument (which are usually TileLayers, so I guess that worry is a little bit unfounded).
For selection, I was thinking (1) just to transfer the grid coordinates selected. i.e. if (0, 0) to (1, 1) is selected, (0, 0) to (1, 1) is also selected on a layer with a different grid size. That, or (2) project the selection from one grid to another. The problem with (1) is infidelity (the selection won't look the same), and the problem with (2) is loss of information when grid sizes are not aligned exactly (i.e. switching from one layer to another and back might make it so the original selection is different).
I prefer (2), but I haven't really found myself transferring selections between layers either.
I'll try out a different solution soon! I think I just misunderstood what you meant in your previous post about this.
Thanks!