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

Changeset View

Standalone View

source/blender/editors/object/object_remesh.c

Context not available.
	#include "BKE_customdata.h"	#include "BKE_customdata.h"
	#include "BKE_mesh_remesh_voxel.h"	#include "BKE_mesh_remesh_voxel.h"

		#include "bmesh.h"

	#include "DEG_depsgraph.h"	#include "DEG_depsgraph.h"
	#include "DEG_depsgraph_build.h"	#include "DEG_depsgraph_build.h"

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

		static bool mesh_is_manifold(Mesh *mesh)
		{
		BMesh *bm = BKE_mesh_to_bmesh_ex(mesh,
		&(struct BMeshCreateParams){0},
		&(struct BMeshFromMeshParams){
		.calc_face_normal = false,
		.cd_mask_extra = {.vmask = CD_MASK_ORIGINDEX,
		.emask = CD_MASK_ORIGINDEX,
		.pmask = CD_MASK_ORIGINDEX},
		});
		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…

		BMIter iter;
		brechtUnsubmitted Done Inline Actions Use `MEM_calloc` instead of manually initializing to zero. brecht: Use `MEM_calloc` instead of manually initializing to zero.
		BMEdge *e;

		bool manifold = true; /* in this case we count boundary edges as manifold */
		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.

		BM_ITER_MESH (e, &iter, bm, BM_EDGES_OF_MESH) {
		const BMLoop *l = e->l;
		const BMLoop *l_other;
		if (l) {
		if ((l_other = l->radial_next) == l) {
		/* boundary edge */
		continue;
		}
		else if ((l_other->radial_next == l) && (l_other->v != l->v)) {
		/* edge with two faces and both faces face in the same direction */
		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 &&…
		continue;
		}
		else {
		/* edge with three or more connected faces or a non continuous face normal direction */
		manifold = false;
		break;
		}
		}
		else {
		/* wire edge */
		manifold = false;
		break;
		}
		}
		BM_mesh_free(bm);
		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…
		return manifold;
		}

	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(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");
		break;
	}	}
	}	}

Context not available.