Mesh: Remove unnecessary edge draw flag #104417

Hans Goudey · 2023-02-07T18:45:26+01:00

Hans Goudey commented

2023-02-07 18:45:26 +01:00

As described in #95966, replace the ME_EDGEDRAW flag with a bit vector
in mesh runtime data. Currently the the flag is only ever set to false for the
"optimal display" feature of the subdivision surface modifier. When creating
an "original" mesh in the main data-base, the flag is always supposed to be true.

The bit vector is now created by the modifier only as necessary, and is cleared
for topology-changing operations. This fixes incorrect interpolation of the flag
as noted in #104376. Generally it isn't possible to interpolate it through
topology-changing operations.

After this, only the seam status needs to be removed from edges before we
can replace them with the generic int2 type (or something similar) and
reduce memory usage by 1/3.

As described in #95966, replace the `ME_EDGEDRAW` flag with a bit vector in mesh runtime data. Currently the the flag is only ever set to false for the "optimal display" feature of the subdivision surface modifier. When creating an "original" mesh in the main data-base, the flag is always supposed to be true. The bit vector is now created by the modifier only as necessary, and is cleared for topology-changing operations. This fixes incorrect interpolation of the flag as noted in #104376. Generally it isn't possible to interpolate it through topology-changing operations. After this, only the seam status needs to be removed from edges before we can replace them with the generic `int2` type (or something similar) and reduce memory usage by 1/3. Related: - 10131a6f6238e259ce673249 - 145839aa42f775f161f5c6a0 In the future `BM_ELEM_DRAW` could be removed as well. Currently it is used and aliased by other defines in some non-obvious ways though.

Hans Goudey added 2 commits 2023-02-07 18:45:28 +01:00

Mesh: Remove unnecessary edge draw flag e88318823c

Merge branch 'main' into refactor-mesh-edge-draw-flag-remove 6e8bbbdfaf

Hans Goudey requested review from Philipp Oeser 2023-02-07 18:46:21 +01:00

Hans Goudey requested review from Campbell Barton 2023-02-07 18:49:39 +01:00

Campbell Barton commented

2023-02-08 10:56:47 +01:00

Works well, my only concern is that none of these assignments are re-using existing memory where flags have existing values, where edge.flag = ME_EDGEDRAW; should be replaced by edge.flag = 0; however as far as I can see this is not the case.

Works well, my only concern is that none of these assignments are re-using existing memory where flags have existing values, where `edge.flag = ME_EDGEDRAW;` should be replaced by `edge.flag = 0;` however as far as I can see this is not the case.

Campbell Barton approved these changes 2023-02-08 10:59:47 +01:00

Hans Goudey requested review from Jeroen Bakker 2023-02-08 14:47:19 +01:00

Hans Goudey commented

2023-02-09 04:04:12 +01:00

Good point, I'll look through the patch to make sure it's okay in that respect. For newly allocated meshes, edges are allocated with CD_SET_DEFAULT, which means the flag is already zeroed and doesn't have to be manually zeroed (this affects the primitive nodes, for example).

Good point, I'll look through the patch to make sure it's okay in that respect. For newly allocated meshes, edges are allocated with `CD_SET_DEFAULT`, which means the flag is already zeroed and doesn't have to be manually zeroed (this affects the primitive nodes, for example).

Hans Goudey added 2 commits 2023-02-09 04:04:13 +01:00

Merge branch 'main' into refactor-mesh-edge-draw-flag-remove 18cbb2a232

Set flags to zero where it wasn't obviously redundant 9a907b56bc

Jeroen Bakker approved these changes 2023-02-09 09:54:55 +01:00

Jeroen Bakker left a comment

Just some code-readability tweaks.

source/blender/blenkernel/intern/mesh_convert.cc

						
				@ -122,3 +122,3 @@

				      BLI_edgehashIterator_setValue(ehi, POINTER_FROM_UINT(e_index));

				      medge->flag = ME_EDGEDRAW;

				      medge->flag = 0;

Jeroen Bakker commented

2023-02-09 09:42:27 +01:00

Just pattern checking. In other places you remove the assignment of flag, but here it is explicitly unset.

Hans Goudey commented

2023-02-09 15:47:22 +01:00

Yes, here I'm not totally sure that the new edges are initialized so it should be cleared to be sure.

source/blender/bmesh/intern/bmesh_mesh_convert.cc

						
				@ -125,3 +125,3 @@

				  const char hflag = e->head.hflag;

				  return ((hflag & BM_ELEM_SEAM) ? ME_SEAM : 0) | ((hflag & BM_ELEM_DRAW) ? ME_EDGEDRAW : 0);

				  return ((hflag & BM_ELEM_SEAM) ? ME_SEAM : 0);