tefusion / godot-subdiv

Fast Subdivision in Godot with opensubdiv
https://godotengine.org/asset-library/asset/1488
MIT License
42 stars 1 forks source link

Extend from ArrayMesh and MeshInstance3D? #12

Closed fire closed 1 year ago

fire commented 2 years ago

Can you explain why we're not doing this?

The advantage is I can take advantage of gltf2 export and the entire scene system is based ImporterMesh3D -> MeshInstance3D

fire commented 2 years ago

Correction, I think we can use ImmediateMesh via Mesh

tefusion commented 2 years ago

I made a custom GeometryInstance to not cause conflicts with the original MeshInstance3D. If extending from ArrayMesh you could add that to a normal MeshInstance3D. The data that is contained within the QuadMesh and QuadMeshInstance is not fit for the renderer though, you need an extra index array for uv's to be able to store a compressed vertex array (see gltf quad importer)

So the data within the QuadMesh won't work with many of the ArrayMesh functions.

tefusion commented 2 years ago

ImmediateMesh might be an option though, I'll look into it.

tefusion commented 2 years ago

I made it inherit from Mesh directly for now in c059952d75ab9e1ae770c12914edc8dc5db27b87 and the commit before. I think it's possible to inherit from ArrayMesh, but most methods would have to be overwritten so I'll first add blend shapes and stuff for now. (So let's keep this open).

Another issue the MeshInstance has, there is currently no access to protected variables like skin_ref that we need to make skinning work. The current solution has another skin ref internally because of that.

fire commented 2 years ago

We can propose to make skin_ref overrideable. Ref\<Mesh> is enough I think.

fire commented 2 years ago

See design using ImporterMesh in the other issue.

tefusion commented 2 years ago

We can propose to make skin_ref overrideable. Ref is enough I think.

We don't even need that much. To not have to rerun resolve skeleton path the only thing necessary is some getter to https://github.com/godotengine/godot/blob/29325935198f981e1f59a774cfa5eff6564140b7/scene/3d/mesh_instance_3d.h#L46

fire commented 2 years ago

Hm https://github.com/godotengine/godot/blob/29325935198f981e1f59a774cfa5eff6564140b7/scene/3d/importer_mesh_instance_3d.h#L55

tefusion commented 2 years ago

Hm https://github.com/godotengine/godot/blob/29325935198f981e1f59a774cfa5eff6564140b7/scene/3d/importer_mesh_instance_3d.h#L55

Ref to skin != SkinRef. It might be possible to run skinning directly with pose data and that skin from the node to avoid creating another one, thanks.

fire commented 2 years ago

I don't remember what's the difference between skin ref and skin.

I only use skins primarily.

tefusion commented 1 year ago

I reverted the change with 465856b6e424abadbd6c166b889aca83f12722ed to inherit from MeshInstance3D after all. There are too many hacky solutions currently needed to make it work well. I misunderstood the way meshes worked in godot, a better option is now done with 1f0b170114c6a577af5249f093bba7471b4ca277 implementing the BakedSubdivMesh (doesn't actually bake bone data yet, but idea is when the subdivision level changes it puts a mesh with baked data to the RenderingServer and lets godot do skinning) which works similar to ImmediateMesh, as you suggested. This one would even work like any normal Mesh with MeshInstance3D if get_rid gets properly exposed as a virtual method either from Mesh or Resource. I couldn't add GDExtension virtual call stuff within the Resource in core though so might take longer/be a more complicated issue, I'll open an issue later.

fire commented 1 year ago

Cheers!