From 3462b4c7aeeab656cc0e97e5d20061a31e0d17c4 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 20 Jun 2021 17:01:09 +1000 Subject: Docs: add additional notes on tessellation, update comments --- source/blender/blenkernel/intern/mesh_tessellate.c | 69 +++++++++++----------- .../intern/draw_cache_extract_mesh_render_data.c | 13 ++-- 2 files changed, 43 insertions(+), 39 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/mesh_tessellate.c b/source/blender/blenkernel/intern/mesh_tessellate.c index 0cd1b211e4d..797db4df97f 100644 --- a/source/blender/blenkernel/intern/mesh_tessellate.c +++ b/source/blender/blenkernel/intern/mesh_tessellate.c @@ -49,6 +49,8 @@ /* -------------------------------------------------------------------- */ /** \name MFace Tessellation + * + * #MFace is a legacy data-structure that should be avoided, use #MLoopTri instead. * \{ */ /** @@ -67,11 +69,10 @@ void BKE_mesh_loops_to_tessdata(CustomData *fdata, uint (*loopindices)[4], const int num_faces) { - /* Note: performances are sub-optimal when we get a NULL mface, - * we could be ~25% quicker with dedicated code... - * Issue is, unless having two different functions with nearly the same code, - * there's not much ways to solve this. Better imho to live with it for now. :/ --mont29 - */ + /* NOTE(mont29): performances are sub-optimal when we get a NULL #MFace, + * we could be ~25% quicker with dedicated code. + * The issue is, unless having two different functions with nearly the same code, + * there's not much ways to solve this. Better IMHO to live with it for now (sigh). */ const int numUV = CustomData_number_of_layers(ldata, CD_MLOOPUV); const int numCol = CustomData_number_of_layers(ldata, CD_MLOOPCOL); const bool hasPCol = CustomData_has_layer(ldata, CD_PREVIEW_MLOOPCOL); @@ -139,7 +140,7 @@ void BKE_mesh_loops_to_tessdata(CustomData *fdata, } if (hasLoopTangent) { - /* need to do for all uv maps at some point */ + /* Need to do for all UV maps at some point. */ float(*ftangents)[4] = CustomData_get_layer(fdata, CD_TANGENT); float(*ltangents)[4] = CustomData_get_layer(ldata, CD_TANGENT); @@ -154,12 +155,15 @@ void BKE_mesh_loops_to_tessdata(CustomData *fdata, } /** - * Recreate tessellation. + * Recreate #MFace Tessellation. * * \param do_face_nor_copy: Controls whether the normals from the poly * are copied to the tessellated faces. * * \return number of tessellation faces. + * + * \note This doesn't use multi-threading like #BKE_mesh_recalc_looptri since + * it's not used in many places and #MFace should be phased out. */ int BKE_mesh_tessface_calc_ex(CustomData *fdata, CustomData *ldata, @@ -170,13 +174,10 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, int totpoly, const bool do_face_nor_copy) { - /* use this to avoid locking pthread for _every_ polygon - * and calling the fill function */ - #define USE_TESSFACE_SPEEDUP -#define USE_TESSFACE_QUADS /* NEEDS FURTHER TESTING */ +#define USE_TESSFACE_QUADS -/* We abuse MFace->edcode to tag quad faces. See below for details. */ +/* We abuse #MFace.edcode to tag quad faces. See below for details. */ #define TESSFACE_IS_QUAD 1 const int looptri_num = poly_to_tri_count(totpoly, totloop); @@ -193,9 +194,9 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, mpoly = CustomData_get_layer(pdata, CD_MPOLY); mloop = CustomData_get_layer(ldata, CD_MLOOP); - /* allocate the length of totfaces, avoid many small reallocs, - * if all faces are tri's it will be correct, quads == 2x allocs */ - /* take care. we are _not_ calloc'ing so be sure to initialize each field */ + /* Allocate the length of `totfaces`, avoid many small reallocation's, + * if all faces are triangles it will be correct, `quads == 2x` allocations. */ + /* Take care since memory is _not_ zeroed so be sure to initialize each field. */ mface_to_poly_map = MEM_malloc_arrayN((size_t)looptri_num, sizeof(*mface_to_poly_map), __func__); mface = MEM_malloc_arrayN((size_t)looptri_num, sizeof(*mface), __func__); lindices = MEM_malloc_arrayN((size_t)looptri_num, sizeof(*lindices), __func__); @@ -208,7 +209,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, uint l1, l2, l3, l4; uint *lidx; if (mp_totloop < 3) { - /* do nothing */ + /* Do nothing. */ } #ifdef USE_TESSFACE_SPEEDUP @@ -217,7 +218,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, mface_to_poly_map[mface_index] = poly_index; \ mf = &mface[mface_index]; \ lidx = lindices[mface_index]; \ - /* set loop indices, transformed to vert indices later */ \ + /* Set loop indices, transformed to vert indices later. */ \ l1 = mp_loopstart + i1; \ l2 = mp_loopstart + i2; \ l3 = mp_loopstart + i3; \ @@ -239,7 +240,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, mface_to_poly_map[mface_index] = poly_index; \ mf = &mface[mface_index]; \ lidx = lindices[mface_index]; \ - /* set loop indices, transformed to vert indices later */ \ + /* Set loop indices, transformed to vert indices later. */ \ l1 = mp_loopstart + 0; /* EXCEPTION */ \ l2 = mp_loopstart + 1; /* EXCEPTION */ \ l3 = mp_loopstart + 2; /* EXCEPTION */ \ @@ -293,7 +294,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, zero_v3(normal); - /* calc normal, flipped: to get a positive 2d cross product */ + /* Calculate the normal, flipped: to get a positive 2D cross product. */ ml = mloop + mp_loopstart; co_prev = mvert[ml[mp_totloop - 1].v].co; for (j = 0; j < mp_totloop; j++, ml++) { @@ -305,7 +306,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, normal[2] = 1.0f; } - /* project verts to 2d */ + /* Project verts to 2D. */ axis_dominant_v3_to_m3_negate(axis_mat, normal); ml = mloop + mp_loopstart; @@ -315,7 +316,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, BLI_polyfill_calc_arena(projverts, mp_totloop, 1, tris, arena); - /* apply fill */ + /* Apply fill. */ for (j = 0; j < totfilltri; j++) { uint *tri = tris[j]; lidx = lindices[mface_index]; @@ -323,7 +324,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, mface_to_poly_map[mface_index] = poly_index; mf = &mface[mface_index]; - /* set loop indices, transformed to vert indices later */ + /* Set loop indices, transformed to vert indices later. */ l1 = mp_loopstart + tri[0]; l2 = mp_loopstart + tri[1]; l3 = mp_loopstart + tri[2]; @@ -359,7 +360,7 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, BLI_assert(totface <= looptri_num); - /* not essential but without this we store over-alloc'd memory in the CustomData layers */ + /* Not essential but without this we store over-allocated memory in the #CustomData layers. */ if (LIKELY(looptri_num != totface)) { mface = MEM_reallocN(mface, sizeof(*mface) * (size_t)totface); mface_to_poly_map = MEM_reallocN(mface_to_poly_map, @@ -368,14 +369,14 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, CustomData_add_layer(fdata, CD_MFACE, CD_ASSIGN, mface, totface); - /* CD_ORIGINDEX will contain an array of indices from tessfaces to the polygons - * they are directly tessellated from */ + /* #CD_ORIGINDEX will contain an array of indices from tessellation-faces to the polygons + * they are directly tessellated from. */ CustomData_add_layer(fdata, CD_ORIGINDEX, CD_ASSIGN, mface_to_poly_map, totface); CustomData_from_bmeshpoly(fdata, ldata, totface); if (do_face_nor_copy) { /* If polys have a normals layer, copying that to faces can help - * avoid the need to recalculate normals later */ + * avoid the need to recalculate normals later. */ if (CustomData_has_layer(pdata, CD_NORMAL)) { float(*pnors)[3] = CustomData_get_layer(pdata, CD_NORMAL); float(*fnors)[3] = CustomData_add_layer(fdata, CD_NORMAL, CD_CALLOC, NULL, totface); @@ -387,13 +388,11 @@ int BKE_mesh_tessface_calc_ex(CustomData *fdata, /* NOTE: quad detection issue - fourth vertidx vs fourth loopidx: * Polygons take care of their loops ordering, hence not of their vertices ordering. - * Currently, our tfaces' fourth vertex index might be 0 even for a quad. However, - * we know our fourth loop index is never 0 for quads (because they are sorted for polygons, - * and our quads are still mere copies of their polygons). - * So we pass NULL as MFace pointer, and BKE_mesh_loops_to_tessdata - * will use the fourth loop index as quad test. - * ... - */ + * Currently, our tfaces' fourth vertex index might be 0 even for a quad. + * However, we know our fourth loop index is never 0 for quads + * (because they are sorted for polygons, and our quads are still mere copies of their polygons). + * So we pass NULL as MFace pointer, and #BKE_mesh_loops_to_tessdata + * will use the fourth loop index as quad test. */ BKE_mesh_loops_to_tessdata(fdata, ldata, NULL, mface_to_poly_map, lindices, totface); /* NOTE: quad detection issue - fourth vertidx vs fourth loopidx: @@ -431,7 +430,7 @@ void BKE_mesh_tessface_calc(Mesh *mesh) mesh->totface, mesh->totloop, mesh->totpoly, - /* calc normals right after, don't copy from polys here */ + /* Calculate normals right after, don't copy from polys here. */ false); BKE_mesh_update_customdata_pointers(mesh, true); @@ -441,6 +440,8 @@ void BKE_mesh_tessface_calc(Mesh *mesh) /* -------------------------------------------------------------------- */ /** \name Loop Tessellation + * + * Fill in #MLoopTri data-structure. * \{ */ /** diff --git a/source/blender/draw/intern/draw_cache_extract_mesh_render_data.c b/source/blender/draw/intern/draw_cache_extract_mesh_render_data.c index b0eb0c8bcb3..bccf894cc53 100644 --- a/source/blender/draw/intern/draw_cache_extract_mesh_render_data.c +++ b/source/blender/draw/intern/draw_cache_extract_mesh_render_data.c @@ -67,8 +67,8 @@ static void mesh_render_data_loose_geom_load(MeshRenderData *mr, MeshBufferExtra static void mesh_render_data_loose_geom_ensure(const MeshRenderData *mr, MeshBufferExtractionCache *cache) { - /* Early exit: Are loose geometry already available. Only checking for loose verts as loose edges - * and verts are calculated at the same time.*/ + /* Early exit: Are loose geometry already available. + * Only checking for loose verts as loose edges and verts are calculated at the same time. */ if (cache->loose_geom.verts) { return; } @@ -230,7 +230,7 @@ static void mesh_render_data_mat_offset_build(MeshRenderData *mr, MeshBufferExtr typedef struct MatOffsetUserData { MeshRenderData *mr; - /* struct is extended during allocation to hold mat_tri_len for each material. */ + /** This struct is extended during allocation to hold mat_tri_len for each material. */ int mat_tri_len[0]; } MatOffsetUserData; @@ -252,7 +252,7 @@ static void mesh_render_data_mat_offset_build_threaded(MeshRenderData *mr, int face_len, TaskParallelRangeFunc range_func) { - /* Extending the MatOffsetUserData with an int per material slot. */ + /* Extending the #MatOffsetUserData with an int per material slot. */ size_t userdata_size = sizeof(MatOffsetUserData) + (mr->mat_len) * sizeof(*cache->mat_offsets.tri); MatOffsetUserData *userdata = MEM_callocN(userdata_size, __func__); @@ -348,6 +348,9 @@ void mesh_render_data_update_looptris(MeshRenderData *mr, if (mr->extract_type != MR_EXTRACT_BMESH) { /* Mesh */ if ((iter_type & MR_ITER_LOOPTRI) || (data_flag & MR_DATA_LOOPTRI)) { + /* NOTE(campbell): It's possible to skip allocating tessellation, + * the tessellation can be calculated as part of the iterator, see: P2188. + * The overall advantage is small (around 1%), so keep this as-is. */ mr->mlooptri = MEM_mallocN(sizeof(*mr->mlooptri) * mr->tri_len, "MR_DATATYPE_LOOPTRI"); if (mr->poly_normals != NULL) { BKE_mesh_recalc_looptri_with_normals(me->mloop, @@ -575,7 +578,7 @@ void mesh_render_data_free(MeshRenderData *mr) MEM_SAFE_FREE(mr->mlooptri); MEM_SAFE_FREE(mr->loop_normals); - /* Loose geometry are owned by MeshBufferExtractionCache. */ + /* Loose geometry are owned by #MeshBufferExtractionCache. */ mr->ledges = NULL; mr->lverts = NULL; -- cgit v1.2.3