Move mesh vertex normals out of MVert #91186

New Issue

Hans Goudey · 2021-09-04T21:42:29+02:00

Hans Goudey commented

2021-09-04 21:42:29 +02:00

Currently mesh vertex normals are stored as short no- [x]; in MVert. This is problematic for a few reasons:

The normals aren't stored contiguously, so processing them as generic data can be complicated. Even the few solutions have problems:
- Implement a special case in algorithms for mesh vertex normal inputs. Obviously this isn't feasible.
- Abstract data access with a callback. This has a significant performance cost.
- Copy the normals into a contiguous array before processing, increasing memory usage and adding overhead.
Normals are almost always processed as floats anyway, so a conversion to and from short is required.
Lazily calculating normals becomes more complicated since they are nested in other data.
Even if normals aren't calculated or needed, they are stored in memory, making any operation on MVert slower.

One key fact is that vertex normals are just derived data, they can always be recalculated with a fairly costly but simple accumulation from face normals.

I propose to remove MVert.no and store the cached result of the vertex normal calculation as a separate CD_NORMAL custom data layer.
Calculation should be lazy, available on const meshes, protected by a mutex. Here are some thoughts on specifics:

Updating code A search tells me that MVert.no is used in 137 places, though some of those are larger changes. I expect some changes might be involved, but worth it.
RNA Mesh vertices have a normal property in RNA. At a small performance cost, this can just access normals indirectly with the RNA pointer's owner ID.
Versioning If necessary, we can simply mark normals dirty when loading old meshes to make sure the new data layer is calculated.
Forward Compatibilty If this is an issue, 2.93 could be patched to also mark normals dirty, except only on meshes saved in 3.0.

After this change, MVert will look like this:

typedef struct MVert {
  float co[3];
  char flag, bweight;
  char _pad[2];
} MVert;

The remaining items could easily be replaced with bitmaps or attributes stored elsewhere. Then MVert would have exactly the same layout as float3!
So, moving mesh normals out of MVert is also the biggest step toward storing mesh positions as a contiguous array as well, which has even more performance benefits.

Currently mesh vertex normals are stored as `short no- [x];` in `MVert`. This is problematic for a few reasons: - The normals aren't stored contiguously, so processing them as generic data can be complicated. Even the few solutions have problems: - Implement a special case in algorithms for mesh vertex normal inputs. Obviously this isn't feasible. - Abstract data access with a callback. This has a significant performance cost. - Copy the normals into a contiguous array before processing, increasing memory usage and adding overhead. - Normals are almost always processed as floats anyway, so a conversion to and from `short` is required. - Lazily calculating normals becomes more complicated since they are nested in other data. - Even if normals aren't calculated or needed, they are stored in memory, making any operation on `MVert` slower. One key fact is that vertex normals are just derived data, they can always be recalculated with a fairly costly but simple accumulation from face normals. I propose to remove `MVert.no` and store the cached result of the vertex normal calculation as a separate `CD_NORMAL` custom data layer. Calculation should be lazy, available on const meshes, protected by a mutex. Here are some thoughts on specifics: - **Updating code** A search tells me that `MVert.no` is used in 137 places, though some of those are larger changes. I expect some changes might be involved, but worth it. - **RNA** Mesh vertices have a `normal` property in RNA. At a small performance cost, this can just access normals indirectly with the RNA pointer's owner ID. - **Versioning** If necessary, we can simply mark normals dirty when loading old meshes to make sure the new data layer is calculated. - **Forward Compatibilty** If this is an issue, 2.93 could be patched to also mark normals dirty, except only on meshes saved in 3.0. --- After this change, `MVert` will look like this: ``` typedef struct MVert { float co[3]; char flag, bweight; char _pad[2]; } MVert; ``` The remaining items could easily be replaced with bitmaps or attributes stored elsewhere. Then `MVert` would have exactly the same layout as `float3`! So, moving mesh normals out of `MVert` is also the biggest step toward storing mesh positions as a contiguous array as well, which has even more performance benefits.

Hans Goudey commented

2021-09-04 21:42:29 +02:00

Added subscriber: @HooglyBoogly