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

Mesh merging #79

Closed
lawnjelly opened this issue Aug 14, 2020 · 11 comments
Closed

Mesh merging #79

lawnjelly opened this issue Aug 14, 2020 · 11 comments

Comments

@lawnjelly
Copy link
Member

lawnjelly commented Aug 14, 2020

Rather than just put this on IRC, I managed to get TPS demo working the other day and did a bit of looking into why I had very low frame rates (12fps). I only have an integrated Intel GPU so not expecting too much.

Anyway I profiled it:
Screenshot from 2020-08-13 07-06-04

I got some improvements with my octree optimization PR (up from 12fps to 17fps)
godotengine/godot#41123

Then had a look at other things. Textures were all very large, it could do with a mechanism to shrink them for hardware that isn't good with 4096x4096 textures. I hard coded the Godot importer to max size 256 to look at this.

I also found when deleting the .import folder and reimporting, it crashed in xatlas (with near zero area triangles). I've had problems like this with the latest xatlas. I did a little hack in xatlas to get it to import correctly.

Next I saw the number of drawcalls and material changes was very high. So I ran it through my mesh merging addon:
https://github.com/lawnjelly/godot-splerger

Doing a split of meshes that have multiple surfaces, then merging the resulting meshes. This reduced drawcalls from over 9000 to around 900. In GLES2 frame rate went from 12fps - 50-60fps. I can't say categorically that this is like for like as I think merging the meshes may have affected the baked lightmaps. But it does suggest that (at least on GLES) merging some of the meshes can result in far better performance.

Also worth mentioning - for something of this type for the main player, if you aren't already you might get away with just creating a navmesh and using that for physics instead of real physics. The physics was pretty dodgy and tended to get stuck easily.

If anyone wants to try merged version, hopefully this will work:
https://drive.google.com/file/d/14Fs0wjxsK_fb1GE4aZi8AFShiddafBYT/view?usp=sharing

This is the merged level file. You will need to open level/level.tscn, delete the existing linked level (the dae file) and then add to this scene a link to the tscn you downloaded.

It also complains it can't find some of the nodes as the names have changed when you run it, but it seemed to run mostly ok. I didn't take a lot of care over the merging this was just a test.

@Calinou
Copy link
Member

Calinou commented Aug 14, 2020

Also worth mentioning - for something of this type for the main player, if you aren't already you might get away with just creating a navmesh and using that for physics instead of real physics. The physics was pretty dodgy and tended to get stuck easily.

The player can move and jump freely, so it's not a good idea. Dodgy physics are due to relying on root motion for jumping, which should never be used for that purpose.

@aaronfranke
Copy link
Member

aaronfranke commented Aug 14, 2020

delete the existing linked level (the dae file)

The latest version of the TPS demo uses GLTF glb files. Sounds like you've been working on an old version. I would suggest re-doing these changes on the latest version to see if it helps there too.

The physics was pretty dodgy and tended to get stuck easily.

This has also been fixed in the latest version.

@lawnjelly
Copy link
Member Author

I couldn't get LFS working to download from github so HaSa and Calinou kindly made me some zipped version. 😁 I think this one was from April. It's using 3.2.3 dev.

I don't really have time to spend on this, it was just an afternoon while fixing another issue in octree. Just pointing out some of the bottlenecks and possible ways to make it more efficient, if anyone was interested in future. Doesn't have to be acted on, but hopefully it should show that merging can be beneficial (especially with GLES, vulkan is more efficient in this regard).

@Calinou
Copy link
Member

Calinou commented Aug 14, 2020

@lawnjelly The ZIP I uploaded here was based on commit 99e4f80, which is from June 30th.

@aaronfranke
Copy link
Member

@lawnjelly The latest version does not use Git LFS.

@lawnjelly
Copy link
Member Author

@lawnjelly The ZIP I uploaded here was based on commit 99e4f80, which is from June 30th.

I only tried the HaSa version so far.

If I get some more time I can look at the June version, but I suspect conclusions would be the same unless the meshes have been merged since.

That could probably be done in e.g. blender manually in a more sensible way with the original files, I might have lost some UV2s in the processing. Alternatively someone could just tweak the gdscript in the merging addon, it is pretty simple (and it really requires a better knowledge of how the demo works than I have).

And of course it's possible that the bottlenecks affecting me might be different on a different system, others may see less of a gain by merging.

The texture thing is quite interesting, we currently pre-package the textures in the export at a certain size, but (afaik) don't allow for changing the sizes uses at runtime. It's quite easy to resize uncompressed (if a little slow), but compressed is another matter.

Maybe the compressed contains the mipmaps already? In which case it might be possible to select say the 1st mipmap as the largest size on import. Off the top of my head I don't think there's a bias setting in OpenGL to do this, but it might be possible.

@lawnjelly The latest version does not use Git LFS.

I tried downloading the master from github and also cloning the repository but some files were missing (the large level file mentioned in the readme for instance). Maybe I was doing something wrong, I have never used LFS or tested it, or my git version or something.

@Calinou
Copy link
Member

Calinou commented Aug 14, 2020

but (afaik) don't allow for changing the sizes uses at runtime.

You can call VisualServer.texture_set_shrink_all_x2_on_set_data(true) in a script's _ready() method to decrease the size of all textures by a 2× factor when loading: https://docs.godotengine.org/en/latest/tutorials/viewports/multiple_resolutions.html#reducing-aliasing-on-downsampling

Beware as it will also impact 2D textures. We should probably have an import flag to exclude specific textures from being downsampled by that method.

@lawnjelly
Copy link
Member Author

You can call VisualServer.texture_set_shrink_all_x2_on_set_data(true) in a script's _ready() method to decrease the size of all textures by a 2× factor when loading:

Ah that looks handy. 😀 Maybe we can mention that in the optimization section of the docs.

Beware as it will also impact 2D textures. We should probably have an import flag to exclude specific textures from being downsampled by that method.

Agree, already noticed this on fonts when I hard coded it in the importing source. 👍

@aaronfranke
Copy link
Member

aaronfranke commented Aug 14, 2020

I tried downloading the master from github and also cloning the repository but some files were missing (the large level file mentioned in the readme for instance)

The README no longer mentions a large level file because the current master does not have it. This is what it says:

Git LFS

Git LFS is no longer required for the current master branch. You only need Git LFS if you are checking out the 3.1 or 3.2.1 branches. Those branches have instructions for Git LFS in their README files.

You don't need Git LFS as of mid-June. There is not really a point to discussing improvements to old versions, if you want to improve the TPS demo you need to first download the latest version of it, and again you don't need Git LFS.

@lawnjelly
Copy link
Member Author

I just tried it again (downloading from github) and running, and this time it worked! So you are right. 🥳

I have no idea why it didn't work last time, when I tried to run it it paused in the debugger on loading a menu, because I think some resource was missing. I tried both downloading from github and git clone repository name. Maybe it had been giving me the 3.2.2 branch (or maybe I had googled a forked repository? that is possible). Put it down to user error. 😊

@aaronfranke
Copy link
Member

@lawnjelly If you would like to re-do your improvements on top of the latest master, that would be welcome. Also, I noticed you currently have everything in a .tscn. If we update the TPS demo, the mesh data must be stored in an efficient format, ideally GLTF .glb files for maximum compatibility, and if you can keep the files under 25 MB (the hard limit is 100 MB, but 25 MB is the limit where it causes the GitHub website to misbehave). If you do this, you can just submit a PR.

Otherwise, this issue isn't really actionable, the file you've posted isn't useful to me.

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

No branches or pull requests

3 participants