craftworkgames / MonoGame.Extended

Extensions to make MonoGame more awesome
http://www.monogameextended.net/
Other
1.42k stars 323 forks source link

[Tiled] Better handling of Tilesets in the Content Pipeline #481

Closed stefanrbk closed 6 years ago

stefanrbk commented 6 years ago

I have been doing some research on the Content Pipeline to figure out how I wanted to link resources together in the project I'm working on, which got me thinking about how the Tiled resources are linked together. I see how the tilesets are imported directly into the maps which are loading them, as well as textures for the tilesets are being loaded in at the end of the pipeline in the "Reader" step. Then I started looking at the Tiled system, and I started wondering why we haven't been using ExternalReference<> and BuildAsset<> to include references to the tilesets that load them only when needed, and the Texture2D tileset images directly into the tileset when building them. I know, @craftworkgames, you are working on overhauling the Tiled system, so I figured I would throw that idea out here!

craftworkgames commented 6 years ago

Thanks mate. I haven't really used ExternalReference or BuildAsset before but I'll keep it in mind while I'm working on it.

stefanrbk commented 6 years ago

Yeah, I was running into an issue where I would change the size of a tileset, and it would then ignore the tileset change unless a full rebuild was performed since the external tileset is loaded into the map file on import. I also thought it would be nice to load the Texture2D directly into the tileset if it could be built as a separate asset. Tileset could then persist across maps it they share the tileset.

stefanrbk commented 6 years ago

I'm currently working on getting TiledMapTileset .tsx content to build separately from TiledMap .tmx so each map file can reference the tileset directly instead of having it baked into each map content file. I'm looking ahead at how I'm going to add the remaining functionality of the .tmx file and I have no idea how we are going to be able to introduce group layers with how the TiledMapRenderer is currently set up with TiledMapModel.

I see 3 ways we could implement layer groups:

(A layer layout for illustration)

├ Group Layer 1
│├ Sub Layer 1
│└ Sub Layer 2
└ Layer 3
  1. Flatten the layers within the layer group into one layer. This fits the TiledMapModel system we have now, but it doesn't allow drawing of the sub layers independently or allow changes to the visibility of the sub layers. The sub layers above would merge together into 1 layer and you couldn't change any sub layers after.
    ├ Group Layer 1
    └ Layer 3
  2. Remove the group layers and bubble the sub layers up to the root. This also fits the TiledMapModel system, but removes the concept of groups all together. You could reference each layer and render them independently, but you couldn't hide the whole group or render the group as a whole.
    ├ Sub Layer 1
    ├ Sub Layer 2
    └ Layer 3
  3. (What I think we should do) Move the TiledMapModel system from TiledMapRenderer into TiledMapLayer. TiledMapRenderer could build the TiledMapModel into each TiledMapLayer. This would allow rendering and manipulating each layer (group layer or otherwise) and changes done to a group layer would affect its sub layers.

Moving the TiledMapModel system wouldnt change anything from an API standpoint, from what I can tell. It looks primarily background. @craftworkgames, I figured I would bring it up before I attempt any changes like that.

craftworkgames commented 6 years ago

Hey man, finally got around to reading this properly.

First of all, thanks for looking into this. Tiled Map rendering is a pretty complicated bit of the library but it's also one of the most used, so it's worth getting this right.

Clearly option 1 & 2 are not ideal. Implementing either of these will probably be a waste of time. They might kind of, sort of solve the problem in the interim but I don't think it would be long before we'd want to take the next step.

So that leaves option 3...

allow rendering and manipulating each layer (group layer or otherwise) and changes done to a group layer would affect its sub layers.

I think the discussion we have here should focus more around what we want rather than how it's going to be implemented. What you've said here starts to capture the requirements but maybe we need to explore the requirements more first.

(What I think we should do) Move the TiledMapModel system from TiledMapRenderer into TiledMapLayer. TiledMapRenderer could build the TiledMapModel into each TiledMapLayer.

Over the years a number of issues have been raised about the different ways people want to use the Tiled Maps.

Balancing all these concerns in a single map renderer is quite a challenge. I've always imagined that we might want to eventually support a few different types of renderers for these different purposes.

So that's why I think it's a good idea to keep the rendering code separated from the model. What this actually means technically is difficult to say. The way we've approached it so far could be entirely wrong. There's really no way to know for sure until we've proved that it works for each use case we want to support.

One point of reference we do have to "prove" we're on the right track is to look at other game frameworks to see how they do it. For example, LibGDX has multiple tiled renderers that cover a variety of different scenarios. I might be worth taking a closer look at how they've done things before we proceed.

Moving the TiledMapModel system wouldnt change anything from an API standpoint

Let's not panic too much about changing the API. One of the benefits of building 2.0 is that it's an opportunity to change things. If changing the API means we can support more of these scenarios then now is the time to do it.

stefanrbk commented 6 years ago

I never thought about multiple renderers. I do like that idea. When I can, i'll look at LibGDX. I know Tiled's website has a list of libraries with some other renderers I'll take a look at as well.

stefanrbk commented 6 years ago

I just looked at LibGDX. It looks like they do something similar to what I was suggesting above. They pass a TiledMapLayer object to their render function. It iterates the tiles and renders them with their equivalent of SpriteBatch. They must not have the ability to draw primitived directly to the GPU. The different renderers are just for the different map/tile shapes and have overloads for how to render each kind. I don't think we need to go that OO and have a different renderer for each map type. We could have different renderers for different situations (i.e. strictly immutable for highest performance, SpriteBatch based for customization, etc.)

What I'm thinking we could do with that example is to use TiledMapLayer objects as part of the render call, but just use the object as a Dictionary<TiledMapLayer, TiledMapLayerModel> key to reference the models to draw. This way, we can pass the layer to the renderer, it looks up the precomputed models and renders them. We would need to change the signature for all render calls to pass the TiledMapLayer instead of an index.

lithiumtoast commented 6 years ago

@stefanrbk The rendering we use for Tiled maps is fairly complicated but it comes at a great benefit for performance. We use to use SpriteBatch but the cost of iterating through the map data and calculating and submitting the triangles for rendering to the GPU every game frame was just too much for large maps. Even libGDX is using batchers for their Tiled map renderers so the same problem is quite likely. Instead what we (or I did) was wrote some code that creates a "3D model" for each layer of the Tiled map. We then upload the vertices and indices for that model to the GPU once and then tell the GPU to render the triangles in those buffers.

I'm pretty sure it was possible to pass the TiledMapLayer to the draw function and it would look up which model to use for drawing. I'm not sure what happened there.

Also using SpriteBatch for maps that need to change tiles is still going to suck. What would be better is just "patching" the vertex buffer with the required changes in tiles.

stefanrbk commented 6 years ago

I'm pretty sure it was possible to pass the TiledMapLayer to the draw function and it would look up which model to use for drawing. I'm not sure what happened there.

It was, and the code I have in my game using 1.0 referenced the layers for the rendering and that is what my branch for groups referenced as well.

Also using SpriteBatch for maps that need to change tiles is still going to suck. What would be better is just "patching" the vertex buffer with the required changes in tiles.

That is a very good idea. I'll have to look at that option when I start looking deeper into the model system!

lithiumtoast commented 6 years ago

All you need to do is basically have some sort of begin and end barrier for applying changes to the data:

Note that the CPU still wouldn't have to touch the data for layers / map if it was write only. This however wouldn't allow the layer to change in size, but it would be fairly sufficient method for small changes in tiles. The worst case is every tile in the layer gets changed, but how often is that? Doubt it would be the case for all the layers. Maybe in the case for some procedurally generated world or something. But in that case it's best to just upload the all the data in the layer to CPU as one go. This is what SpriteBatch does essentially, but still having to upload the data infrequently (less than every frame) would still be a massive boost in comparison.

stefanrbk commented 6 years ago

I think just having some mutability would be better than nothing for that situation!

craftworkgames commented 6 years ago

I've tried to rename this issue to better reflect what it's about but it seems it has turned into a bit of a mix with #435.

stefanrbk commented 6 years ago

I believe this is done now as the Tiled overhaul is complete and the tilesets are built independently now, so Imma close this now 😄