Differential D5877 Diff 18463 source/blender/editors/object/object_remesh.c

Changeset View

Standalone View

source/blender/editors/object/object_remesh.c

Context not available.
	int success;	int success;
	} QuadriFlowJob;	} QuadriFlowJob;

		static bool mesh_is_manifold_consistent(Mesh *mesh)
		{
		/* In this check we count boundary edges as manifold. Additionally, we also
		* check that the direction of the faces are consistent and doesn't suddenly
		* flip
		*/

		bool is_manifold_consistent = true;
		const MLoop *mloop = mesh->mloop;
		const MPoly *mpoly = mesh->mpoly;
		char edge_faces = (char )MEM_malloc_arrayN(
		mesh->totedge, sizeof(char), "remesh_manifold_check");
		sergeyUnsubmitted Done Inline Actions Sounds like a quite expensive operation to do. Would think that allocating a single array of integers with a size of number of edges, and then iterate over mloop and keep counts of edges is cheaper. sergey: Sounds like a quite expensive operation to do. Would think that allocating a single array of…
		int edge_vert = (int )MEM_malloc_arrayN(
		mesh->totedge, sizeof(unsigned int), "remesh_consistent_check");
		brechtUnsubmitted Done Inline Actions Use `MEM_calloc` instead of manually initializing to zero. brecht: Use `MEM_calloc` instead of manually initializing to zero.

		for (unsigned int i = 0; i < mesh->totedge; i++) {
		edge_faces[i] = 0;
		sergeyUnsubmitted Done Inline Actions `is_manifold`. Also comment actually belongs to a function, not a variable. sergey: `is_manifold`. Also comment actually belongs to a function, not a variable.
		edge_vert[i] = -1;
		}

		for (unsigned int poly_index = 0; poly_index < mesh->totpoly; poly_index++) {
		const MPoly *poly = &mpoly[poly_index];
		for (unsigned int corner = 0; corner < poly->totloop; corner++) {
		const MLoop *loop = &mloop[poly->loopstart + corner];
		edge_faces[loop->e] += 1;
		if (edge_faces[loop->e] > 2) {
		is_manifold_consistent = false;
		break;
		brechtUnsubmitted Done Inline Actions This break only breaks out of one loop, add `poly_index < mesh->totpoly && is_manifold_consistent` to the first loop to make it break out of both. brecht: This break only breaks out of one loop, add `poly_index < mesh->totpoly &&…
		}

		if (edge_vert[loop->e] == -1) {
		edge_vert[loop->e] = loop->v;
		}
		else if (edge_vert[loop->e] == loop->v) {
		/* Mesh has flips in the surface so it is non consistent */
		is_manifold_consistent = false;
		break;
		}
		}
		}

		if (is_manifold_consistent) {
		/* check for wire edges */
		JacquesLuckeUnsubmitted Done Inline Actions I think it could be quite a bit faster in some cases (but never slower) to just iterate over all loops, instead of having this nested loop. Also, doesn't this function fit better in `mesh_validate.c`? JacquesLucke: I think it could be quite a bit faster in some cases (but never slower) to just iterate over…
		zeddbAuthorUnsubmitted Done Inline Actions If you think than an extra bool check will be that much slower, I can simply set `poly_index` to `totpoly` to end the loop if you wish. We discussed moving this to `mesh_validate.c`. IIRC we decided to wait until something else needed this too and write a some more general functions for this then. zeddb: If you think than an extra bool check will be that much slower, I can simply set `poly_index`…
		JacquesLuckeUnsubmitted Done Inline Actions Nah, that would be quite hacky. The two benefits of just iterating over all loops are: The code is more simple. Only one loop is required and you could just break out of it. It's faster. Not mainly because of the extra condition (which is highly predictable), but because the memory access pattern is more predictable. By iterating over the polygons in an outer loop, you might have to jump around in the loops array, resulting in a worse cache hit rate. Note, I don't care too much about whether this is changed or not. I just happened to see this function and thought about how it could be made better. JacquesLucke: Nah, that would be quite hacky. The two benefits of just iterating over all loops are: 1. The…
		zeddbAuthorUnsubmitted Done Inline Actions Ah sorry, I misunderstood you. I can try to just have one for loop instead that loops over all mloops! I do really appreciate that you go through the changes and propose changes like these. Thanks! zeddb: Ah sorry, I misunderstood you. I can try to just have one for loop instead that loops over all…
		zeddbAuthorUnsubmitted Done Inline Actions This is now pushed to master: rBfc9e921495312ace23af11a69e738a8a7adbaeed Thanks again for pointing this out. zeddb: This is now pushed to master: rBfc9e921495312ace23af11a69e738a8a7adbaeed Thanks again for…
		for (unsigned int i = 0; i < mesh->totedge; i++) {
		if (edge_faces[i] == 0) {
		is_manifold_consistent = false;
		break;
		}
		}
		}

		MEM_freeN(edge_faces);
		MEM_freeN(edge_vert);

		return is_manifold_consistent;
		}

	static void quadriflow_free_job(void *customdata)	static void quadriflow_free_job(void *customdata)
	{	{
	QuadriFlowJob *qj = customdata;	QuadriFlowJob *qj = customdata;
Context not available.
	Mesh *mesh = ob->data;	Mesh *mesh = ob->data;
	Mesh *new_mesh;	Mesh *new_mesh;

		/* Check if the mesh is manifold. Quadriflow requires manifold meshes */
		if (!mesh_is_manifold_consistent(mesh)) {
		qj->success = -2;
		return;
		}

	new_mesh = BKE_mesh_remesh_quadriflow_to_mesh_nomain(mesh,	new_mesh = BKE_mesh_remesh_quadriflow_to_mesh_nomain(mesh,
	qj->target_faces,	qj->target_faces,
	qj->seed,	qj->seed,
Context not available.

	WM_set_locked_interface(G_MAIN->wm.first, false);	WM_set_locked_interface(G_MAIN->wm.first, false);

	if (qj->success > 0) {	switch (qj->success) {
	DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY);	case 1:
	WM_reportf(RPT_INFO, "QuadriFlow: Completed remeshing!");	DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY);
	}	WM_reportf(RPT_INFO, "QuadriFlow: Completed remeshing!");
	else {	break;
	if (qj->success == 0) {	case 0:
	WM_reportf(RPT_ERROR, "QuadriFlow: remeshing failed!");	WM_reportf(RPT_ERROR, "QuadriFlow: remeshing failed!");
	}	break;
	else {	case -1:
	WM_report(RPT_WARNING, "QuadriFlow: remeshing canceled!");	WM_report(RPT_WARNING, "QuadriFlow: remeshing canceled!");
	}	break;
		case -2:
		WM_report(RPT_WARNING,
		"QuadriFlow: The mesh needs to be manifold and have face normals that point in a "
		"consistent direction.");
		break;
	}	}
	}	}

Context not available.