(#6480) tiled parallax support

[Quillraven]

I finally had now a little time to look into this topic (fixes #6480) and had a discussion with the guy from Tiled to understand better how Tiled is implementing parallax scrolling. You can find the discussion here.

However, I seek for your help guys because I am a little bit overwhelmed by the big amount of different renderers and where exactly I should put the code.

---

For testing, I have adjusted the existing ortho.tmx file and added some group layers with different parallax factors to also test the correct behavior of "structured" layers where the final scrolling factor is the layer's factor multiplied by its parents.

My approach is done in the map loader directly. Whenever a parent is set, which seems to happen after its children are loaded then the parallax value is simply multiplied by the parent's parallax value. That should support that use case correctly but please note that the Tiled author mentioned that this is currently bugged and will be fixed in an upcoming version. But that should not stop us from implementing it correctly ;)

---

Next I tried to update the render logic. I see there is a TiledMapRenderer interface which I assume is used by all renderers related to the Tiled Editor?

There are two implementations of this interface: BatchTiledMapRenderer and OrthoCachedTiledMapRenderer.

I saw that the second one does not correctly support group layers and also the render code seems to be copy pasted which is also not ideal. render() and render(int[] layers) has 99% the same code from what I can tell and should be merged.
But anyway, it seems like this renderer is not up to date and therefore I ignored it for now.

The BatchTiledMapRenderer is then subclassed by a few other renderers like HexagonalTiledMapRenderer or OrthogonalTiledMapRenderer.

Since parallax scrolling should be a generic solution for any renderer in my opinion I put my code in the BatchTiledMapRenderer hoping that this will then apply to all subclasses but I did not verify that yet, sorry.

My idea was to put it into the renderMapLayer method to also apply the logic to any type of layer because in my opinion it should always behave the same. It simply renders the layer with a specific offset no matter if it is a tiled, object or image layer.

I currently use the transform matrix of the batch and apply a specific translation but this has the disadvantage that I have to break the rendering process with end and begin which I guess has a negative impact on the render performance?

---

I used the existing TiledMapLayerOffsetTest to test the changes. It does already a parallax render effect but it does not 100% match the result I get in Tiled. The problem is I am missing some information in Tiled and therefore it is hard for me to understand where the difference is coming from.

---

Anyway, please have a look and let me know how to do it properly and efficiently.

Edit: I noticed that there is also some weird clipping happening and the parallax layer sometimes get cut off although it is within the view. Will need to have a look at that as well. I thought with using the translate functionality on the batch I should be safe but doesn't seem so 😅

[Quillraven]

I went now a different route and updated the already existing renderOffset functionality by adding the parallax offset here as well. This has the advantage that I don't have to break the batching and also it seems to solve the issue with the clipping I had before.

I also tested it now with all map options (iso, ortho, hex) and it looks good to me. I used the TiledMapLayerOffsetTest for that.

[lyze237]

That's a great PR!

Though from previous discussions about tile/tiledmap stuff ( #6534 (comment) ) libGDX members don't really want to add Tiled Editor (TMX) specific elements to the base classes (For example MapLayer) so I guess an alternative to that is to add that feature to TiledMapTileLayer instead.

[Quillraven]

@lyze237 I can do that but I have a question for that change. In Tiled you can have something like this:

This is a grouping of layers and the final parallax scrolling factor is then the multiplication of the factor of the layer itself and all of its parents.

I see there is a MapGroupLayer but it is not part of the tiled package. It is outside on the same level as MapLayer. However, it seems like a new instance is only created by the BaseTmxMapLoader. So I assume it is Tiled specific?

Since there is no common parent for Tiled specific layers I need to duplicate the parallax properties in TiledMapTileLayer, MapGroupLayer and TiledMapImageLayer.

I have no problem with that and could make those changes but should I then move the MapGroupLayer inside the tiled package and rename it to TiledMapGroupLayer? Or should I create a new TiledMapGroupLayer class?

If you ask me I argue that a parallax scrolling factor should be part of the general MapLayer since it is a generic property which can be implemented by any map editor. I only know Tiled and cannot judge on Tide but if Tide ever supports that as well then we anyway need to get it to the generic MapLayer somehow, correct?

But don't get this the wrong way. I am not an expert with the LibGDX repository and the reasoning behind certain decisions. If it is better for you to keep everything separate then as mentioned above, I can do the adjustments but please provide me with some answers to my questions :)

Thank you!

[lyze237]

Hey! @MobiDevelop! Sorry for the ping.

Since you helped with previous tile d related PRs, could you check this one out quickly and see what's the best way to add that?

[MobiDevelop]

I think adding parallax properties/functionality on the base MapLayer is fine. It's a pretty common/generic need whether you use Tiled (editor) for making maps. If I recall, Tide does/did have parallax support.

[Quillraven]

Thanks for the feedback! Let me then do some final adjustments and testing please. I try to finish it today or tomorrow and keep it in the MapLayer.

edit: I am finished from my side. I had to fix the hierarchical scrolling factor loading because doing it within setParent of the MapLayer doesn't work because of the order how this method gets called. That's why I am doing it now at the end when the map is loaded.

Also, added correct support for image layer rendering as well and reverted the changes to getRenderOffsetX/Y methods and put that code now directly into the renderers. The reason is that I am not sure if anyone out there is overriding the renderers/MapLayer classes and if they do then this will introduce a breaking API change. To avoid that I reverted it and put it into the renderers directly.

Please note that when you use hierarchical scrolling factors in Tiled then at the moment you get a different result in LibGDX. The reason is (as mentioned above) that there is a bug currently in Tiled which gets fixed in an upcoming version. The author mentioned that it gets fixed and is then correctly working as mentioned in the documentation. If I am not mistaken then I implemented the already fixed version according to the documentation.

[Quillraven]

cool stuff! Can you also verify the y-axis once more? Just to be safe. And most likely it is the same mistake in the other renderers as mentioned by @lyze237 above.

Thanks for helping out :)

[BoBIsHere86]

cool stuff! Can you also verify the y-axis once more? Just to be safe. And most likely it is the same mistake in the other renderers as mentioned by @lyze237 above.

Thanks for helping out :)

Totally, when I have some time either tonight or tomorrow I'll try and test out everything that's been mentioned. Also, I'll try out the Y axis as well and report back. Luckily, I had just finished my tiled color tint PR. So I was in a good spot to help out here.

[BoBIsHere86]

Alrighty, So I'm back with some results. And it's looking pretty good.

For all my tests I altered all of the parallax factors on the maps to be 1.25 on both the X and Y coordinates and used the test maps provided and then my 1 map I made. I will show both what Tiled Renders, then libGDX.

For each map I will simulate: Camera goes to the Right, then to the Left, Then UP, then Down. So we should keep an eye out to make sure Camera goes left, parallax layers go right. Camera Goes up, parallax layers go down, etc.

Also, for certain maps which are hard to follow, I explicitly mention which layer is static.

First Test: Custom ortho map:
Tiled:

libGDX:

Second: Isometric map (Note: The blue water layer is the parallax layer, Green is the static layer.)
Tiled:

libGDX:

Third: Iso Staggered ( The Blue Layer is the STATIC Layer here, The Green Stripes of land are Parallax ,it gets confusing to see sometimes.)
Tiled:

libGDX:

Fourth: Hex (The Green and Orange Hexs are STATIC, the Blue Layer is the Parallax)
Tiled:

libGDX:

Anyways, that's the extent of my testing for now. If anyone wants to try there hand at it. Or maybe I missed something. Let me know.

[BoBIsHere86]

One more follow up to this Thread.
The OrthoCachedTiledMapRenderer just does not work with the Parallax we have it now. I looked into it and it's because over here:
https://github.com/Quillraven/libgdx/blob/a59b49c5ab081c06e660b5b0cce5f742180e24fd/gdx/src/com/badlogic/gdx/maps/tiled/renderers/OrthoCachedTiledMapRenderer.java#L117
The cached flag is only false on the first run through. So renderTileLayer() only gets called once and never again. So the map parallax effect never updates.

I have never used OrthoCachedTiledMapRenderer, but I do see that the cache will become false either through forced call of invalidateCache() or in the setView() method when the bounds are exceeded.

That being said based on how I "think" this caching system works. I actually think that we should change this:
https://github.com/Quillraven/libgdx/blob/a59b49c5ab081c06e660b5b0cce5f742180e24fd/gdx/src/com/badlogic/gdx/maps/tiled/renderers/OrthoCachedTiledMapRenderer.java#L220
To base the parallax off of the viewbounds instead of the the cachebounds anyway, since it's rendering doesnt match up with tiled or any of the others when using it.
So I'd suggest it become: 'final float layerOffsetX = layer.getRenderOffsetX() * unitScale - viewBounds.x * (layer.getParallaxX() - 1);' etc...

In my repo when testing these changes and Modifying the test to FORCE a call to invalidateCache every frame, we can actually get it to work the same as all of the other renders.

That being said, Orthcache seems out of date anyway and as you've stated doesnt even support layers. So I don't know if we would want to just skip it. :-/

[Quillraven]

I think it doesn't harm to add the functionality to the Orthocache renderer as well and from what I understood you are correct that you need to call invalidateCache before rendering to get the correct result. As mentioned at the beginning of this topic (somewhere ;) ), I'd anyway suggest that this rendering logic gets refactored because it seems to be copy&pasted to the different renderers which makes it then a little bit tidious to bring in features like this one.

But I assume that this is not going to happen in the near future as this also can be a risky move to introduce bugs. So I think having it like we do it now is good enough ... just my personal point of view.

Anyway, thanks for helping out @BoBIsHere86 ! Looks good to me. I requested a re-review of the two reviewers from before. Let's hope they give it a look soon so that doesn't have to lay around another year or so :)

edit: btw do I need to do something with my fork? I am not very familiar with the github merging workflow and the forks. If I need to do anything, please let me know.

[BoBIsHere86]

I think it doesn't harm to add the functionality to the Orthocache renderer as well and from what I understood you are correct that you need to call invalidateCache before rendering to get the correct result. As mentioned at the beginning of this topic (somewhere ;) ), I'd anyway suggest that this rendering logic gets refactored because it seems to be copy&pasted to the different renderers which makes it then a little bit tidious to bring in features like this one.

But I assume that this is not going to happen in the near future as this also can be a risky move to introduce bugs. So I think having it like we do it now is good enough ... just my personal point of view.

Anyway, thanks for helping out @BoBIsHere86 ! Looks good to me. I requested a re-review of the two reviewers from before. Let's hope they give it a look soon so that doesn't have to lay around another year or so :)

edit: btw do I need to do something with my fork? I am not very familiar with the github merging workflow and the forks. If I need to do anything, please let me know.

I'm no git expert, I was hoping I could just propose changes in the PR for you to accept but I guess not.
But I think you just need to make the changes I mentioned in your featured dev branch and then recommit them.
The changes being:
Going to all of the renders where you have these lines :https://github.com/Quillraven/libgdx/blob/a59b49c5ab081c06e660b5b0cce5f742180e24fd/gdx/src/com/badlogic/gdx/maps/tiled/renderers/OrthogonalTiledMapRenderer.java#L77

You need to change the ' unitScale + viewBounds.x * (layer.getParallaxX() - 1);' portion to use minus like this:
'unitScale - viewBounds.x * (layer.getParallaxX() - 1);'

That was the big oversight I noticed which caused these issues.

Then if we wanted to work on the Orthocache I think we need to change :
https://github.com/Quillraven/libgdx/blob/a59b49c5ab081c06e660b5b0cce5f742180e24fd/gdx/src/com/badlogic/gdx/maps/tiled/renderers/OrthoCachedTiledMapRenderer.java#L220

To used viewBounds instead of cacheBounds:
final float layerOffsetX = layer.getRenderOffsetX() * unitScale + viewBounds.x * (layer.getParallaxX() - 1); // offset in tiled is y down, so we flip it final float layerOffsetY = -layer.getRenderOffsetY() * unitScale - viewBounds.y * (layer.getParallaxY() - 1);

But that being said, in order for this to show correctly in the test, you would need to add a call to invalidate the cache when using that renderer.

Maybe I can just fork your fork, and see if I can just send you a PR with the changes to make this less painful.
I'll take a look.

Either way this PR is a great addition it will be good to try and have libGDX fully up to date with Tiled's newest features.

[SimonIT]

Just cherry-picked the commits from Quillraven#1. I hope I didn't forget something

[BoBIsHere86]

Awesome, Looks like you got everything. Great to see this moving along.

[Quillraven]

So I don't need to do anything here? Thank you again for the support guys!

If I still need to do something then let me know. I should have some time on the weekend 🙂