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

blend shape operations on ArrayMesh work incorrectly #31595

Closed
slapin opened this issue Aug 23, 2019 · 15 comments
Closed

blend shape operations on ArrayMesh work incorrectly #31595

slapin opened this issue Aug 23, 2019 · 15 comments

Comments

@slapin
Copy link
Contributor

slapin commented Aug 23, 2019

Code below (on git master)

        if (p_blend_shapes.size()) {
                //validate format for morphs
                for (int i = 0; i < p_blend_shapes.size(); i++) {

                        uint32_t bsformat = 0;
                        Array arr = p_blend_shapes[i];
                        for (int j = 0; j < arr.size(); j++) {

                                if (arr[j].get_type() != Variant::NIL)
                                        bsformat |= (1 << j);
                        }

                        ERR_FAIL_COND((bsformat) != (format & (VS::ARRAY_FORMAT_INDEX - 1)));
                }
        }

generates a lot of errors for simple gdscript:

# helper is ArrayMesh loaded from file via load()
	var ret_mesh: = ArrayMesh.new()
	var helper_surf_array = helper.surface_get_arrays(0)
	var bshapes = helper.surface_get_blend_shape_arrays(0)
	for k in range(helper.get_blend_shape_count()):
		ret_mesh.add_blend_shape(helper.get_blend_shape_name(k))
	ret_mesh.add_surface_from_arrays(Mesh.PRIMITIVE_TRIANGLES, helper_surf_array, bshapes)
	return ret_mesh

which prevents any reasonable manipulation with meshes with blend shapes. Any ideas on fixing this? This used to work fine a few months ago. Original mesh is imported from Blender-2.80 release using godot-blender-exporter aka .escn exporter and works perfectly fine.

@akien-mga
Copy link
Member

Code below (on git master)

For the sake of completeness, that's here:

if (p_blend_shapes.size()) {
//validate format for morphs
for (int i = 0; i < p_blend_shapes.size(); i++) {
uint32_t bsformat = 0;
Array arr = p_blend_shapes[i];
for (int j = 0; j < arr.size(); j++) {
if (arr[j].get_type() != Variant::NIL)
bsformat |= (1 << j);
}
ERR_FAIL_COND((bsformat) != (format & (VS::ARRAY_FORMAT_INDEX - 1)));
}
}

@slapin
Copy link
Contributor Author

slapin commented Aug 23, 2019

Full function code if that matters (triangulate_uv just triangulates UVs over vertex distances):

func convert_triangles(base: ArrayMesh, helper: ArrayMesh) -> ArrayMesh:
	var v2idxb = {}
	var v2idxh = {}
	var v2uv = {}
	var sc = 0
	var ret_mesh: = ArrayMesh.new()
	var base_surf_array = base.surface_get_arrays(sc)
	for idx in range(base_surf_array[ArrayMesh.ARRAY_VERTEX].size()):
		var v = base_surf_array[ArrayMesh.ARRAY_VERTEX][idx]
		v2idxb[v] = idx
	var helper_surf_array = helper.surface_get_arrays(sc)
	var bshapes = helper.surface_get_blend_shape_arrays(sc)
	for idx in range(helper_surf_array[ArrayMesh.ARRAY_VERTEX].size()):
		var v = helper_surf_array[ArrayMesh.ARRAY_VERTEX][idx]
		v2idxh[v] = idx
	# find 3 closest vertices on base mesh
	for k in v2idxh.keys():
		var res = []
		var ds = []
		for l in v2idxb.keys():
			if res.size() < 3:
				res.push_back(l)
				ds.push_back(k.distance_to(l))
			else:
				var idx = -1
				for m in range(ds.size()):
					if idx < 0:
						idx = m
					elif ds[idx] < ds[m]:
						idx = m
				var d = k.distance_to(l)
				if d < ds[idx]:
					res[idx] = l
					ds[idx] = d
		var vs = PoolVector3Array(res)
		var uvs = PoolVector2Array()
		for p in res:
			var idx = v2idxb[p]
			var uvb = base_surf_array[ArrayMesh.ARRAY_TEX_UV][idx]
			uvs.push_back(uvb)
		var uvh = triangulate_uv(k, vs, uvs)
		helper_surf_array[ArrayMesh.ARRAY_TEX_UV][v2idxh[k]] = uvh
		for h in bshapes.size():
			bshapes[h][ArrayMesh.ARRAY_TEX_UV][v2idxh[k]] = uvh
	for k in range(helper.get_blend_shape_count()):
		ret_mesh.add_blend_shape(helper.get_blend_shape_name(k))
		ret_mesh.blend_shape_mode
	ret_mesh.add_surface_from_arrays(Mesh.PRIMITIVE_TRIANGLES, helper_surf_array, bshapes)
	return ret_mesh

@slapin
Copy link
Contributor Author

slapin commented Aug 23, 2019

I just did not want pople to be frighened by larger portions of code, as bugs like that will hang forever, but still would add that for completeness.

@akien-mga
Copy link
Member

@slapin Yeah I just added the link to the relevant file and lines so that interested contributors can browse the code freely, without having to grep for it to find the relevant file and line again.

@slapin
Copy link
Contributor Author

slapin commented Aug 23, 2019

@akien-mga thanks, my github-fu is not advanced to this extent.

@akien-mga
Copy link
Member

It's a bit offtopic, but just FYI as it's a convenient feature: you can browse to a file through GitHub's web UI, select lines by Shift+clicking on line numbers on the left side, and there's a "..." options menu with "Copy permalink" which gives you a link to those lines at the current HEAD commit: https://github.com/godotengine/godot/blob/791d7f78b52f5b828aa5541897e12c6a1861ef6f/servers/visual_server.cpp#L975-L989

Screenshot_20190823_125259

And when you paste it in an issue/PR, GitHub automatically converts it to the widget seen above with the code.

@slapin
Copy link
Contributor Author

slapin commented Aug 23, 2019 via email

@GeorgeS2019
Copy link

@slapin @akien-mga

can both of you have a look at this finding to see if this explain why ArrayMesh is not able to set blendshape in Csharp

=================================================

In Csharp, BlendShape are get and set through two classes
ArrayMesh.cs and VisualServer.cs

Interestingly, in ArrayMesh.cs there is no corresponding SetBlendShapeWeight method
Could this explain why the above bug happens?

================================================
There is this method in VisualServer.cs

/// <summary>
/// <para>Sets the weight for a given blend shape associated with this instance.</para>
/// </summary>
[GodotMethod("instance_set_blend_shape_weight")]
public static void InstanceSetBlendShapeWeight(RID instance, int shape, float weight)
{
     NativeCalls.godot_icall_3_630(method_bind_262, ptr, RID.GetPtr(instance), shape, ref weight);
}

@KoBeWi
Copy link
Member

KoBeWi commented Dec 24, 2020

Can anyone still reproduce this bug in Godot 3.2.3 or any later release?

If yes, please ensure that an up-to-date Minimal Reproduction Project (MRP) is included in this report (a MRP is a zipped Godot project with the minimal elements necessary to reliably trigger the bug). You can upload ZIP files in an issue comment with a drag and drop.

@lyuma
Copy link
Contributor

lyuma commented Apr 12, 2021

Still present on Godot 4.0.dev 9ca0d66

Here's the code I used to workaround the problem

	var prim: int = mesh.surface_get_primitive_type(surf_idx)
	var fmt_compress_flags: int = mesh.surface_get_format(surf_idx)
	var arr: Array = mesh.surface_get_arrays(surf_idx) 
	var bsarr: Array = mesh.surface_get_blend_shape_arrays(surf_idx)
	var lods: Dictionary = {} # mesh.surface_get_lods(surf_idx) isn't exposed :'-(
	# surface_get_blend_shape_arrays seems to truncate each result at 7 elements or so. Let's add them back.
	for bsidx in range(len(bsarr)):
		bsarr[bsidx].resize(len(arr))
		for i in range(len(arr)):
			if i >=  ArrayMesh.ARRAY_INDEX or typeof(arr[i]) == TYPE_NIL:
				bsarr[bsidx][i] = null
			elif typeof(bsarr[bsidx][i]) == TYPE_NIL or len(bsarr[bsidx][i]) == 0:
				bsarr[bsidx][i] = arr[i].duplicate()
				bsarr[bsidx][i].resize(0)
				bsarr[bsidx][i].resize(len(arr[i]))

	... PERFORM VERTEX MODIFICATIONS HERE ...

	mesh.add_surface_from_arrays(prim, arr, bsarr, lods, fmt_compress_flags)

Godot isn't the happiest about this: it prints this error when I add arrays... but the above code does seem to workaround the bug.

ERROR: Storage buffer supplied (binding: 2) is invalid.
at: (drivers\vulkan\rendering_device_vulkan.cpp:5152)

@lukostello
Copy link
Contributor

#54062 is at least tangentially related. If the solution I propose to fix it is implemented it would heavily effect this code. I am recommending blend shapes be inside the mesh resource for reasons given in this post and inside the project I made in the most recent comment.

@MJacred
Copy link
Contributor

MJacred commented Jul 20, 2022

ERROR: Storage buffer supplied (binding: 2) is invalid.
at: (drivers\vulkan\rendering_device_vulkan.cpp:5152)

@lyuma: Could you retest with Godot 3.4.5 RC 1, please? It has #60832 -> which unbinds vertex buffer
Sounds closely related. Might resolve that error

@akien-mga
Copy link
Member

Is this this reproducible in latest 4.0 builds?

@akien-mga akien-mga modified the milestones: 4.0, 4.x Feb 26, 2023
@smix8
Copy link
Contributor

smix8 commented May 2, 2023

I think this is fixed in Godot 4, I can not reproduce the issue in recent master with a valid BlendShape Array.

A BlendShape array requires format Mesh::ARRAY_FORMAT_BLEND_SHAPE_MASK.
"Mask of mesh channels permitted in blend shapes."

This means only array index Mesh::ARRAY_VERTEX + Mesh::ARRAY_NORMAL + Mesh::ARRAY_TANGENT are allowed for BlendShape arrays.

The script example code was adding other mesh arrays as well hence why it printend errors.

@Calinou
Copy link
Member

Calinou commented May 2, 2023

Closing per @smix8's comment.

@Calinou Calinou closed this as completed May 2, 2023
@Calinou Calinou modified the milestones: 4.x, 4.1 May 2, 2023
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

9 participants