godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.17k stars 98 forks source link

Expose all Mesh, MeshInstance3D, ImporterMeshInstance3D, and ImporterMesh virtual methods #5367

Open fire opened 2 years ago

fire commented 2 years ago

Describe the project you are working on

https://github.com/tefusion/godot-subdiv @tefusion

This is an implementation of Add support for subdivision surfaces using OpenSubdiv #784.

Quad surfaces and opensubdiv is important for reduction in control points while keeping detail in high quality 3d meshing.

I'm working on a Social VR game.

Subidiv

Skinning

Describe the problem or limitation you are having in your project

We are trying to add subdivision blend shapes and need to override at least find_blend_shape_by_name and set_blend_shape_value.

https://github.com/tefusion/godot-subdiv/issues/8

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Expose Mesh, MeshInstance3D, ImporterMeshInstance3D, and ImporterMesh as virtual methods to be overridable.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Make the methods virtual, and make the methods overridable.

If this enhancement will not be used often, can it be worked around with a few lines of script?

We're trying to write a script.

Is there a reason why this should be core and not an add-on in the asset library?

This to avoid merging opensubdiv into Godot Engine.

tefusion commented 2 years ago

To sum this up a bit: What we currently need is set_blend_shape_value to be virtual.

Main reason is in the current implementation of subdivisions in godot (and what will likely stay that way) everything that changes the mesh can't be handled by the RenderingServer and instead gets handled on CPU and then creates subdivisions upon that data. This results in the problem that the blend shapes within the MeshInstance3D are currently not being used and instead offered fake empty data from the mesh to avoid the set_blend_shape_value call.

If those calls were virtual instead there'd be no need to handle this in such a roundabout way, which causes other regressions like not being able to use BlendShape tracks directly with the AnimationPlayer.

I'm also pretty sure godot-subdiv won't be the only project to benefit from this change. On some discussions I've seen problems with the size of meshes with a lot of blend shape data. By allowing to overwrite these calls there'd be the possibility of an extension to pack those blend shapes (don't want to go too far off topic, but ~9x less space requirement should be achievable by keeping an index_map and only storing vertex_data, it's what we currently do) and then apply with CPU.