Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make YSort stable #42375

Merged
merged 1 commit into from
Sep 27, 2020
Merged

Make YSort stable #42375

merged 1 commit into from
Sep 27, 2020

Conversation

Pennycook
Copy link
Contributor

Keeps track of the order in which items are collected by
_collect_ysort_children, and uses that order to break
ties between items with similar Y positions.

Closes #33230.

Keeps track of the order in which items are collected by
_collect_ysort_children, and uses that order to break
ties between items with similar Y positions.
@aaronfranke
Copy link
Member

This is extremely likely to conflict with #42282. I'm marking this as a draft for now to avoid accidental merging.

@aaronfranke aaronfranke marked this pull request as draft September 27, 2020 20:19
@akien-mga
Copy link
Member

akien-mga commented Sep 27, 2020

This is extremely likely to conflict with #42282. I'm marking this as a draft for now to avoid accidental merging.

And this is extremely likely to be merged first, so there's not reason to do that.
We don't operate with First In, First Merged :)

@akien-mga akien-mga marked this pull request as ready for review September 27, 2020 21:20
@akien-mga
Copy link
Member

As this is the solution that we discussed and approved on IRC, merging.

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 27, 2020
@akien-mga akien-mga merged commit 8016521 into godotengine:master Sep 27, 2020
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@Pennycook
Copy link
Contributor Author

Thanks! And congrats for your first merged Godot contribution 🎉

Thanks! Hopefully the first of many. 😄

@Pennycook Pennycook deleted the stable-ysort branch September 27, 2020 23:09
@andriyDev
Copy link
Contributor

andriyDev commented Sep 28, 2020

This is extremely likely to conflict with #42282. I'm marking this as a draft for now to avoid accidental merging.

Actually, the code for the current version of #42282 does not modify RenderingServer at all, which means these changes should be completely independent and will work on top of each other. Also congrats!

@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 29, 2020
@Skuriles
Copy link

Just to give some feedback:
This seem to work in 3.2.4 rc1, but the problem is it only works for tiles when they cover only one cell. If you use singel tile objects which are bigger than one cell only the first one is shown.

Example: first one with tilemaps and player in YDort, second just all as child nodes
image

image

@Pennycook
Copy link
Contributor Author

@Skuriles: Is this something that worked differently before the patch?

I've observed that using multi-cell tiles with YSort is really tricky -- especially if the tiles don't fill the cells completely -- and it's something that I want to take another look at once godotengine/godot-proposals#1769 is implemented.

@Skuriles
Copy link

Skuriles commented Feb 1, 2021

@Pennycook in Godot 3.2.3 it was completely random (some tiles were shown above the other, some not even if they were from same tilemap). No the randomness is gone, but it still does not work for tiles with more than one cell.

Im fact: I had some hope to use tilemaps to just place floor, walls and all static objects from tilemaps, but this won't work. I gave up using Y-Sort with tilemaps and only render the floor and and outer walls by tilemaps.

Everything else will be a StaticBody or similar object.
I didn't event got simple tilemap with kinematicbody with multiples child sprites to work with Y-Sort.
So I will now just handle z-index with trigger and scripts and single objects.

@Calamander
Copy link
Contributor

@Skuriles Have you tried setting Tile Origin to "Bottom Left" for tilemap with bigger tiles?
@Pennycook Checked this in 3.3 rc6. Looks like there is minor inconsistency (probably also exists in earlier revisions):
With two tilemaps with YSort, one parent, one child. When placing new tiles for parent and child, child tiles are sorted behind parent tiles (no matter which tile was placed first - parent or child) until after scene is saved and reloaded.

@Pennycook
Copy link
Contributor Author

@Skuriles: I think this has been fixed by #48918. You can now set the offset and Y-Sort origin of individual tiles to any value you'd like, and in the testing I've done so far with big tiles everything seems to work.

@Calamander: Do you still see this behavior with the new TileMap editor? I'm having trouble reproducing it there.

@Calamander
Copy link
Contributor

@Pennycook which one is new TileMap? Groud's or 3.3.2?..

@Zireael07
Copy link
Contributor

@Calamander: Groud's, I'm guessing.

@Pennycook
Copy link
Contributor Author

Yes, sorry -- I meant Groud's. My thinking: if the issue is fixed in the new editor, it might be easier to identify why it isn't working in the old editor.

@Calamander
Copy link
Contributor

@Pennycook Does Godot 4.0 use Vulkan 1.2? I can't seem to run it and my videocard doesn't support Vulkan 1.2 while it does support 1.1 so I guess I can't test it.

@Calinou
Copy link
Member

Calinou commented May 31, 2021

Does Godot 4.0 use Vulkan 1.2?

IIRC, it requires Vulkan 1.0 and optionally uses Vulkan 1.1 features.

However, Intel Haswell IGPs are not supported because they don't fully support Vulkan 1.0 in the first place. Any Intel IGP after Haswell should work (Broadwell and later).

@Calamander
Copy link
Contributor

@Calinou Hmm, I have GeForce 660M (with latest available driver installed). Vulkaninfo shows my api version as 1.1.95 so then it should probably be enough. I initially thought that problem lies with Vulkan because it crashes before opening project manager right after reporting errors about unfreed shaders and shows nothing else, even in verbose mode.
crash

I guess I'll try compiling and running editor myself sometime later to see if I can determine the cause of the crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overlapping tiles in TileMaps with Y Sort enabled under a Y Sort node randomly changes rendering order
9 participants