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

Bind mesh merging functionality and improvements #56513

Closed
wants to merge 1 commit into from

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Jan 5, 2022

Mesh merging functionality introduced in Rooms & Portals is exposed for general use. Some improvements to robustness.

Implements godotengine/godot-proposals#901 (at runtime, rather than import)
Fixes #7844

Introduction

Mesh merging is a very useful tool in the performance toolbox. Many games, especially outdoor games are bottlenecked by the number of MeshInstances, both in terms of draw calls in the renderer, but also in terms of housekeeping scene tree side.
Merging meshes, either ahead of time (pre-baking and saving) or at runtime (usually at level startup) can offer significant performance improvements, particularly for things like vegetation and procedural modular block levels (such as minecraft type maps).
This is especially relevant when hardware instancing is not available.

Note however that there is a trade off - merged meshes will be culled as a block, so depending on how you apply it you can lose culling accuracy. This will still be a net win in many situations though. And, unlike instancing, you also lose the ability to move the merged objects relative to each other, they can only be moved as a group.

Local or Global space

The merged vertices can either be defined in global (world) space, or in the local space of the destination MeshInstance (provided it is in the SceneTree, which is necessary in order to get its global_transform).

In a static scene, global space may make sense, but when merging sub-parts of a movable object (say a spaceship), you may want the vertices specified relative to the object, so that when you move it the parts move in unison.

Local space is default, but note that local space will flag a warning if you merge before adding the MeshInstance to the SceneTree (because the local space is undefined).

Notes

  • The mesh merging functionality is already available in Rooms & Portals, so it seems to make sense to make it available for general use.
  • The system initially offers only very simplistic merging but should be suitable for many use cases.
  • Suggestions welcome for better names for the functions that are exposed, or improvements to the API (especially for future considerations).
  • The location of the merging code could potentially be moved around a bit if there is a better place for it, perhaps more of a concern is this is later ported to 4.x. I did have a look at putting in ArrayMesh, but the requirements for looking up MeshInstance transforms made this a little messier. An alternative is to push all the mesh merging functionality into a helper class. We can always do this later if desired.
  • It may be desirable at some stage to add something to the UI to give this a more user-orientated experience, but this is not necessary to get the ball rolling for people who want to use this.

Example script:

	print($MeshInstance.is_mergeable_with($MeshInstance2))
	
	var arr = []
	arr.push_back($MeshInstance)
	arr.push_back($MeshInstance2)
	
	$Merged.merge_meshes(arr)

Example project

MergingTest.zip
(just set true or false in the script)

20,000 boxes, either drawn separately or merged:

Separate 28 fps
Merged 664 fps (24 x faster)

@Zireael07
Copy link
Contributor

Zireael07 commented Jan 5, 2022

Nitpick: you linked 901 in Godot issues, not 901 in proposals ;)

@fire
Copy link
Member

fire commented Jan 5, 2022

Does this include mesh splitting, mesh merging by grid, or mesh merging materials? I can share examples how to do it.

@fire
Copy link
Member

fire commented Jan 5, 2022

Your code requires Normals, Tangents, Colors, UVs and UV2s to match.

Normal must exist. If empty Normals, we can force generate assuming smooth shading.

Once we have the normals, we can generate tangents.

We can append Colors without issue.

Given tangents exist from the previous step we can generate or combine UVs, UVs2.

  1. If an UV does not exist, generate it by using the smooth grouping Normals from the previous stages. https://github.com/godot-extended-libraries/godot-fire/tree/feature/mesh-unwrap
  2. Merge atlases https://github.com/godot-extended-libraries/scene_merge/blob/master/merge.cpp#L482 including bleeding by some large number like 16 pixels for 1024x1024 https://github.com/godot-extended-libraries/scene_merge/blob/master/rjm_texbleed.cpp.
  3. Merge and Split by grid (lawnjelly)

Any thoughts?

@lawnjelly
Copy link
Member Author

Does this include mesh splitting, mesh merging by grid, or mesh merging materials? I can share examples how to do it.

Nope, it's purely simple mesh merging, and I'm not currently looking at implementing more than that.

@fire
Copy link
Member

fire commented Jan 5, 2022

@lawnjelly do you need help porting to master?

@lawnjelly
Copy link
Member Author

Any thoughts?

Generating missing attributes is something that could potentially be added later, but it's going outside the scope of the basic functionality, e.g. joining similar trees or bushes.

If you want to spend time to make a more fully featured system, then be my guest, but bear in mind it then becomes much more difficult to get merged in core. (Remember you can do a lot of this stuff in an addon for instance).

@fire
Copy link
Member

fire commented Jan 5, 2022

Splitting into the basic system where merging does the checks and the ensuring the similarity is a better design.

I think the split by grid is also doable.

The forcing similarity code was fairly lengthy in my module, so splitting into three stages can help abstract the flow.

@lawnjelly
Copy link
Member Author

lawnjelly commented Jan 5, 2022

@lawnjelly do you need help porting to master?

Sure if you want to do something similar in master, that would be great, it would probably need some discussion though with the other rendering guys in terms of how it should work etc. Also bear in mind with vulkan especially, the cost of drawcalls is lower, so there will be relatively less benefit to merging, compared to 3.x, and especially with GLES2 which doesn't have instancing.

Splitting into the basic system where merging does the checks and the ensuring the similarity is a better design.

There are two functions available simply because that's how it needs to be done from the rooms conversion system (it needs to identify mergeable MeshInstances at an earlier stage). The bound function always does the checks (only the private internal one can omit the checks).

In this PR we could choose to not bind the is_mergeable_with function, but it is probably handy in terms of user interface etc in its own right. Say if you were selecting meshes in the editor, it would be handy to know beforehand whether they could be merged before calling the function etc.

@lawnjelly lawnjelly marked this pull request as ready for review January 5, 2022 17:25
@lawnjelly lawnjelly requested review from a team as code owners January 5, 2022 17:25
@lawnjelly lawnjelly changed the title [WIP] Bind mesh merging functionality and improvements Bind mesh merging functionality and improvements Jan 5, 2022
@lawnjelly lawnjelly changed the title Bind mesh merging functionality and improvements [WIP] Bind mesh merging functionality and improvements Jan 6, 2022
Mesh merging functionality introduced in Rooms & Portals is exposed for general use. Some improvements to robustness.
@lawnjelly lawnjelly changed the title [WIP] Bind mesh merging functionality and improvements Bind mesh merging functionality and improvements Jan 6, 2022
@lawnjelly
Copy link
Member Author

lawnjelly commented Feb 2, 2022

Closing this, I'll either update this PR or start a new one just for the binding, the other functionality has moved to #57551 .

Binding is now in #57661 .

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.

6 participants