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

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

Open
fire opened this issue Sep 7, 2022 · 1 comment

Comments

@fire
Copy link
Member

fire commented Sep 7, 2022

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.

tefusion/godot-subdiv#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.

@fire fire changed the title Expose all Expose all Mesh, MeshInstance3D, ImporterMeshInstance3D, and ImporterMesh virtual methods Sep 7, 2022
@tefusion
Copy link

tefusion commented Sep 7, 2022

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.

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