Struct of Arrays Refactor for Mesh Vertices #93602

New Issue

Hans Goudey · 2021-12-03T15:07:21+01:00

Hans Goudey commented

2021-12-03 15:07:21 +01:00

Currently, MVert stores quite a bit of data:

typedef struct MVert {
  float co[3];
  short no[3];
  char flag, bweight;
} MVert;

This is inefficient, because it means vertex positions aren't stored in a contiguous array.
It also means all of the other data isn't stored contiguously. Because of that, access to all of the data
is slower than it should be. We also pay the cost of storing data when it's unused.

We should convert MVert to be the same as float3, and move the other elements into separate arrays.
This can significantly simplify code, and makes it easier to achieve better performance for mesh editing.

Normals

Normals are covered by #91186 (Move mesh vertex normals out of MVert). This task is for the rest of the data stored in the struct.

Bevel Weight

In the longer term, this should be stored as an optional named generic float attribute (see #89054).
In the shorter term, this can be stored as CD_BWEIGHT. This means slightly increased memory usage when this data is used,
but it also gives increased precision, and it doesn't have to be allocated when it isn't used.

291c313f80

Flag

In the flag is:

  /*  SELECT = (1 << 0), */
  ME_VERT_TMP_TAG = (1 << 2),
  ME_HIDE = (1 << 4),
  ME_VERT_FACEDOT = (1 << 5),
  ME_VERT_PBVH_UPDATE = (1 << 7),

Selection
It's not clear to me how selection interacts with mselect (MSelect), but perhaps it can be moved there.
It can also be moved to a separate boolean custom data layer with a name that keeps it from being displayed in the UI.

TMP_TAG 90a23dec46
Since this is only used locally in one area, it can be easily replaced with Array<bool> or the C equivalent.

ME_HIDE 2480b55f21
This can be moved to a separate boolean custom data layer, likely with a reserved name.

Face Dot 8c25889bb6
Currently this is only set by subdivision code. I'm not clear on the details.
Face dot vertices could be moved to a separate array, since they are just runtime data.

PBVH Update Tag 7682d7de04
This is just used in sculpt and paint code, it could be relatively simply moved to a bitmask or a separate boolean array.

Internal Mesh API

When there is a position attribute, functions that refer to mesh "coords" and "vertices" should be changed to refer to "positions" instead.
This makes the internal API consistent with more public naming of attributes. This should affect the functions added in D14760.

Currently, `MVert` stores quite a bit of data: ``` typedef struct MVert { float co[3]; short no[3]; char flag, bweight; } MVert; ``` This is inefficient, because it means vertex positions aren't stored in a contiguous array. It also means all of the other data isn't stored contiguously. Because of that, access to all of the data is slower than it should be. We also pay the cost of storing data when it's unused. We should convert `MVert` to be the same as `float3`, and move the other elements into separate arrays. This can significantly simplify code, and makes it easier to achieve better performance for mesh editing. ### Normals Normals are covered by #91186 (Move mesh vertex normals out of MVert). This task is for the rest of the data stored in the struct. ### Bevel Weight In the longer term, this should be stored as an optional named generic float attribute (see #89054). In the shorter term, this can be stored as `CD_BWEIGHT`. This means slightly increased memory usage when this data is used, but it also gives increased precision, and it doesn't have to be allocated when it isn't used. 291c313f80 ### Flag In the flag is: ``` /* SELECT = (1 << 0), */ ME_VERT_TMP_TAG = (1 << 2), ME_HIDE = (1 << 4), ME_VERT_FACEDOT = (1 << 5), ME_VERT_PBVH_UPDATE = (1 << 7), ``` **Selection** It's not clear to me how selection interacts with `mselect` (`MSelect`), but perhaps it can be moved there. It can also be moved to a separate boolean custom data layer with a name that keeps it from being displayed in the UI. **TMP_TAG** 90a23dec46 Since this is only used locally in one area, it can be easily replaced with `Array<bool>` or the C equivalent. **ME_HIDE** 2480b55f21 This can be moved to a separate boolean custom data layer, likely with a reserved name. **Face Dot** 8c25889bb6 Currently this is only set by subdivision code. I'm not clear on the details. Face dot vertices could be moved to a separate array, since they are just runtime data. **PBVH Update Tag** 7682d7de04 This is just used in sculpt and paint code, it could be relatively simply moved to a bitmask or a separate boolean array. ### Internal Mesh API When there is a `position` attribute, functions that refer to mesh "coords" and "vertices" should be changed to refer to "positions" instead. This makes the internal API consistent with more public naming of attributes. This should affect the functions added in [D14760](https://archive.blender.org/developer/D14760).

Hans Goudey commented

2021-12-03 15:07:21 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'