[DRAFT] Add a way to fully create a ConcavePolygonShape3D in calling thread #100413
+278
−11
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a work-in-progress, mainly for discussion. I don't consider it ready for now.
Attempts to solve the issue mentionned in #99895 (comment)
This gives control about when the heavy work is executed when creating a lof or mesh colliders at runtime, typically in procedural terrains. So creating chunks in multiple background threads, away from frame-dependent stuff, becomes easy.
Before this, a typical chunk would look like this in the profiler. Lots of these, executed in serial, either on the main thread or the physics thread:
![image](https://private-user-images.githubusercontent.com/1311555/395817174-1d5e9f2e-6150-4b84-a5ca-8a2043686a53.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1MTQ5NjEsIm5iZiI6MTczOTUxNDY2MSwicGF0aCI6Ii8xMzExNTU1LzM5NTgxNzE3NC0xZDVlOWYyZS02MTUwLTRiODQtYTVjYS04YTIwNDM2ODZhNTMucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIxNCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMTRUMDYzMTAxWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NzNkY2EzMWNiODY0ODU0Yjc4N2ZlNmIyNjJjYTNmMTJjYzg3ODk5MDU5ZDE2YjNhODQ1MWEzODA0NWJlZTg4NSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.mExisHio0yF13xYeBHeUasxAwJHw4RRhH2CPY01iUFM)
After this, I can do the following in multiple background threads (including priority sorting and cancellation in case of spams, which wouldn't be easy if it was deferred):
![image](https://private-user-images.githubusercontent.com/1311555/395816306-be2ebd89-3d4b-44bb-a2d6-c3a87dcaad8b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1MTQ5NjEsIm5iZiI6MTczOTUxNDY2MSwicGF0aCI6Ii8xMzExNTU1LzM5NTgxNjMwNi1iZTJlYmQ4OS0zZDRiLTQ0YmItYTJkNi1jM2E4N2RjYWFkOGIucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIxNCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMTRUMDYzMTAxWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MjI4MDRmYWMwODBlY2Q1MWE0MjhhZTRjMGMwMjllNTQxZjJlM2EzYmJhM2JkNWQyODI1NzE4ZTdkYjNkNDkwMCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.CKwgpOQVjxO5J4vWPqoTdBt0LhGDPaQTm_llRW9gEs0)
PhysicsServer3D
andConcavePolygonShape3D
, which expects all properties to be specified at once. Contrary toshape_set_data
, it is not deferred even when the physics server is wrapped insidePhysicsServer3DWrapMT
Indexify
internallyThere are more details and questions in TODO comments in the code.
While this seems to work, I'm not super happy about the approach. It is a very specific API, separate from how everything else is done. I actually wish that shapes (especially those involving heavy cooking like meshes) should be creatable in that specific way by default, while also using their properties like usual. Like, somehow making
set_shape_data
non-deferred when called the first time, and deferred if called more after? It wouldn't only give control when balancing threaded tasks in projects like mine, but it would also do the expected thing when loading scenes or resources with Godot's threaded loader.However, so far I could not think of any other way (not without refactoring more things), because in Godot, properties can be set anywhere anytime, and the fact that the
run in separate thread
option is implemented by deferring every relevant method. So it is quite difficult to guarantee that the work is done on the caller thread, while still benefiting from multi-threaded physics simulation, while also not disrupting the physics engine if the shape has already been attached to a body (not my case, but this is a possibility to guard for; and it is guaranteed to not be the case in a function that fully creates a shape in one call).Side note: I currently realize a part of this code could be split into a separate PR that could be easier to merge: support for indexed mesh when creating a mesh shape. As you can see in the profiling screenshot, re-indexing a mesh is expensive and it currently happens because the Godot API is expecting it to be so. If an index buffer could be passed optionally, it would optimize that step, and wouldn't break compatibility.