Refactor rendering
Background
The edge generation step ends with the _meshes property of the shape being filled with SS2D_Mesh[1] objects, which in turn consist of one or more ArrayMesh objects.
In _draw(), the _meshes array will be flattened [2][3] into another Array[SS2D_Mesh] where every SS2D_Mesh contains only one ArrayMesh.
For every SS2D_Mesh in this list, an SS2D_Shape_Render[4] node will be instantiated. It receives the corresponding mesh and sets respective render properties (material, z-index, ...) on itself and renders the mesh in its own _draw().
I'm unsure how large these ArrayMeshes are, but I'm pretty sure they directly correspond to the SS2D_IndexMaps created during generation, i.e. the edges generated as part of one index map will be part of the same ArrayMesh.
Performance
In a simple shape with 4 vertices and only a single edge material, this process takes around 0.1 ms, which might not sound like much but is actually insanely long for this use-case.
However, this time does not change with the vertex count, e.g. with hundreds of points, the processing time would still be around 0.1 ms.
This is because the costly operations here are not the rendering, but the flattening (which includes resource duplication) and node instantiation. With a single edge material, there will only be a single rendering node with a single mesh. With that in mind, 0.1 ms runtime cost is even more insane. But if we add corner materials, tapers, etc., the complexity rises.
Considering the sharp_corner_tapering example, there are suddenly 21 rendering nodes for the left shape and 10 for the right (including 1 for the fill texture each).
The computation time also increased to 0.5 ms for the left shape and 0.25 ms for the right shape.
As a one-time computation during runtime I'd call this negligible, but in editor when continously editing points and regenerating the shape, this is actually pretty expensive.
Issues
This approach has obviously several downsides:
- High computation cost
- Lots of data juggling and resource duplication
- Deleting and instantiating lots of rendering nodes (but they are mostly cached at least)
- Lots of nodes for the engine to handle
- Lots of draw calls (might get batched by the renderer, though)
- Shapes need to regenerate meshes at runtime (unlike collisions)
- Shapes are not clickable in editor (which is IMO one of the major annoyances with this plugin)
New Approach Proposal
IIRC, we already briefly talked about this approach on Discord.
The idea is to generate a single ArrayMesh, which contains all the meshes and assign it to a single MeshInstance2D for rendering.
ArrayMesh supports multiple surfaces, so different meshes with different materials can be accomodated in the same ArrayMesh, hence only one rendering node is needed.
In order to prevent the number of surfaces from exploding with increasing shape complexity and to maintain the correct z-order, a custom sort function has to be employed which sorts meshes by z-index and material. Sub-meshes with the same material can be merged in the same surface, otherwise a new surface is used.
Instead of flattening the mesh array and creating a bunch of rendering nodes, we sort it and directly generate a single ArrayMesh from it.
It might even be possible to further improve this step between edge generation and rendering but this requires further investigation.
Besides improving performance, this approach also has the following advantages:
- Shapes are now clickable, because
MeshInstance2Dis clickable - Basically out-of-box baking shapes to static meshes by exposing the
MeshInstancein the editor.- Allows instant loading at runtime
- Could use a similar approach like collisions, where the corresponding
MeshInstanceis always exposed in editor with a setting when to regenerate
#76 proposes to use MeshInstances as base class for SS2D_Shape_Render instead of using the _draw() function, but otherwise keeps everything the same.
We could actually do this as a quick first version to get the mentioned advantages (not the performance improvements) pretty much for free without much effort.
However, it would generate several rendering nodes instead of just one when baking.
Amazing. That's a pretty good proposal.
Yep, this proposal sums up every point we've discussed so far, and it would be a great improvement IMO.
One thing I'd add to consider is that we could extend MeshInstance2D in the shape class instead of Node2D and composition. Are there good points for keeping it separate? We probably don't need a "separate node approach" to allow multiples of such. One good point is that it would break backward compatibility with existing projects, but I think we can provide an upgrade script.
EDIT:
I wanted to add, that existing properties of a node can be hidden in _validate_property if we don't want user to mess with MeshInstance2D.mesh or MeshInstance2D.texture.
That would be much better. The only downside I can think of is really the compatibility break. I'm also not sure what's the best way to properly handle the conversion in an automated manner. Did you think of a shell script or an in-editor script?
Sadly, I'm not aware of any automatic methods available in Godot, besides ClassDB::add_compatibility_class, and even that one is not available in GDScript, AFAIK.
I think the most portable method is probably to provide such a conversion in-editor. Scan the project, check each *.tscn in text mode for nodes that have type Node2D and an SS2D_Shape script, and replace the type. Not sure if we can do that automatically without firing this routine each time the project is opened. But what to do about *.scn?
Is *.scn a thing? Never heard of it.
What I also thought of is creating a new node like SS2D_Shape_V2 with the new functionality. We can add an editor button to convert the shape by simply removing the old node and creating the v2 one with the same properties. The old implementation is kept for compatibility.
But I'm not sure if I like this idea over making a clean cut.
Is *.scn a thing? Never heard of it.
That's an old binary format - git unfriendly - it was in use before they introduced tres. And yeah - probably nobody uses it, but I think you can still save as scn. So maybe it's okay to ignore it?
But I'm not sure if I like this idea over making a clean cut.
Yeah, that would be painless to transition for the projects, but leaves us with two classes in the code-base, and a bunch of legacy code. And we would have to ask which shape version is used when issues are discussed. That is, until we remove the deprecated V1.
Yes, I think we can ignore *.scn.
How about we strip all the code from the old implementation, except exported properties and add a _ready function which silently replaces the node with the v2 implementation with corresponding properties.
This way we have a smooth transition and don't need to maintain a legacy implementation.
Still leaves the namespace with old baggage like Shape2D, OpenShape, ClosedShape, at least for some time. And I'd prefer to have some scanner to manually loading and resaving each scene. We don't have to run the compatibility scanner more than once per project, btw. But that other approach has its own merits. It's potentially the least problematic for a clueless user.
True. I also see people being confused between ss2d_shape and ss2d_shape_v2. I'm also waiting to remove various deprecated functions and properties, so that might be the time to do so.
We could create a dialog that will be presented to the user at startup that explains that the new version has breaking changes and provides a button to run the conversion script. In order to detect whether the conversion already took place and the dialog should not be presented a second time, we could introduce a hidden project setting to store the current smartshape version.
Just wanted to toss in that currently within 4.3 adding edge meta materials to an SS2D shape breaks web builds. I haven't been able to figure out why as it sort of acts like the edge meta material is assigned with an empty edge material created (no textures assigned yet) and breaks the entire shape rendering (but colliders remain)
Please try current master branch and create a new issue if the problem persists. We had a related problem fixed on current master.
Please try current master branch and create a new issue if the problem persists. We had a related problem fixed on current master.
I'll try it out. it wasn't life-ruining for us and I know the asset store is notoriously behind for many people (they really need to have an easier submission process that can be automated or I Finish my addon management addon haha)
The fault is not the asset store. The release process is absolutely fine. We just don't do releases as often as we should, which is mostly related to us working in our spare time on this project and not having a fixed release policy. If your issue is the same as the issue fixed in master, we might be more inclined to make a release in near future, though.
The fault is not the asset store. The release process is absolutely fine. We just don't do releases as often as we should, which is mostly related to us working in our spare time on this project and not having a fixed release policy. If your issue is the same as the issue fixed in master, we might be more inclined to make a release in near future, though.
it's not a dig on the godot group at all. The asset store being behind repos is a common issue. I had written a prototype plugin myself that acted as an in-editor manager for github sourced plugins and Calinou was even excited about it. I need to revisit it at some point since it offered auto updating from branch changes and the like for exactly cases like this - it was just too janky for me to release without a refactor.
I'll be able to test web builds fairly soon and report back
There is actually an action to publish on AssetLib: https://github.com/deep-entertainment/godot-asset-lib-action
Oh that's pretty cool, we should try this.
I'm not trying to complain, the tool is great and helps me a lot, thank you big time for creating and sharing it!
I'm currently working on an open world action rpg in Godot 3.5.3 and I was looking for load time bottlenecks and then I realised, that the SS2Ds aren't baked. I was shocked, honestly.
I tried to make a bake tool myself, but it doesn't work, since the nodes aren't exposed to the tree, so I had to postpone and queue the loading of the SS2Ds, which isn't ideal, because it still introduces stuttering, but at least not a huge lag at once.
I went from a 350 ms lag spike down to 60 ms just by postponing and spreading the load.
I would really love to see a bakeable version though, also in Godot 3.
@Reapetitive Godot 3 branch is no longer being updated. So, if this feature gets implemented, unless someone backports it, it won’t be included in the 3.x branch.
There used to be an experimental version with baking for 3.x by @remorse107. You may still find the archive on the Discord if you search for "bake". It may help you get a custom solution for your game.
@Reapetitive As @limbonaut said, 3.x was discontinued after porting to 4.x because the amount of changes made it infeasible to maintain both versions. The 4.x version also improved a lot since then, especially performance-wise.
I'm currently working on this issue and while it didn't go as planned, I found an easy fix to support baked shapes without a massive rewrite. At least that's what I'm theorizing, I haven't got to actually try it so far. You can try to hack it into the 3.x version yourself.
- SS2D instances a bunch of render nodes that each render parts of the whole mesh.
- There is a
set_mesh()function inshape_render.gdwhich receives anSS2D_Mesh(mesh.gd). - These meshes are stored in
_meshesand get assigned in_draw()inshape_base.gd. - Meshes are regenerated in
_on_dirty_update(). - You can make
_meshesan exported property andSS2D_MeshaResourceso the meshes get serialized to your scene file. - You can add additional logic which checks if
_meshesis not empty upon start and in that case skips the regeneration step and just assigns them to the render nodes.
@limbonaut @mphe Thank you guys so much! I found the solution by @remorse107 and rewrote it, managed to bake the meshes with as few resulting nodes as possible and integrated them into my placeholder system, which replaces the nodes with placeholders and loads them later on demand after level/chunk is added to the scene tree.
Sadly, somehow it's still heavier on the loading times, then my previous solution, which just spreads out the process of runtime generation of ss2ds over time. Perhaps there are just too many nodes to process, even if just placeholders. I'll stick to my previous solution for now, as it is pretty performant. I will expand my solution by your idea with export meshes and mesh resource, thank you!
The good thing about this test is - I realised, that many area2Ds are monitorable, when they shouldn't be, and it impacts performance a lot, since there are so many of them.
@mphe Your Idea worked like a charm - Mesh to resource, set all params of mesh to export, set _meshes to export and voila. The shapes are baked. Just check for Engine.editor_hint everywhere except _draw and you need to call update once I think.
My runtime generation is still faster on loading times, than baking the meshes in my case. The materials I use are too heavy to load when baked I guess. For example a wall with like 30 meshes, all of them save the material as a duplicate I guess, which ends up hitting the performance.
I need more time to test some things, I will tell you guys if I could make it work.
Nice to hear that!
It seems off to me that live regeneration is faster than baked meshes. The mesh regeneration in 3.x is very slow, especially with complex shapes. Skipping that step should definitely yield performance improvements in direct comparison. Of course loading shapes distributed over multiple frames is always faster.
The materials I use are too heavy to load when baked I guess
Are talking about shader materials? Shape materials are only relevant during mesh generation, not for rendering. The rendering for baked meshes is the same as the rendering for freshly generated meshes.
- Search for
duplicate()functions in various SS2D scripts if they perform material duplication, then simply remove those lines. - If you haven't already, save your materials as a resource on disk so it is definitely shared among all instances.
My mistake, I copied the Draw function and skipped the step with flat meshes, made export for material from edges in shape base and now it is fast, really fast
@mphe Thank you really much! You literally saved my game. I am still using queuing, I set up, draw and update the shapes one by one over time to smooth things out as far as I can, since my game can be really heavy on materials and chunk/object complexity.
My current solution isn't a general fit for everything, but the shapes usually use multiple textures, but only one material, so it should do the trick.
In case you are curious the game is Curvature, you can look it up on Steam.
You're welcome! If you want, you can submit a PR to the 3.x branch so other poor souls stuck on 3.x can have better performance as well :D.
Do your shapes have collisions? In that case take a look at #159. This patch should be rather easy to backport (or at least relevant parts of it) and should speed up your loading times even further.
In case you are curious the game is Curvature, you can look it up on Steam.
Looks nice!
@mphe yeah, my shapes have collisions, but they are baked, aren't they? At least I am not running the step in runtime.
Honestly, I looked through the code of the patch and I didn't really get what, or rather how I have to integrate it, seems to me, that 3.x version is missing some stuff, that isn't in this patch directly, I am too exhausted though.
Sure, my code is messy atm, I will clean it up soonish and try to post it. The changes I made so far, as I said, only work if you are using one shader material, and I only tested it with open shapes.
I have no Idea how to submit a proper PR though, no Idea what I have to do.
Collision generation is separate from mesh generation. Since it is written to a CollisonPolygon, it is baked, but it will be regenerated by SS2D on load nevertheless. This was improved in the linked patch to make that behavior configurable.
I think you can just get away with another Engine.editor_hint check before regenerating collisions at runtime.
Another part of the PR was tweaking tesselation_tolerance and tesselation_stages to improve performance by producing less points without significant quality loss.
To not derail the original issue here any further, let's move this discussion to Discord.