Commit Graph

23 Commits

Author SHA1 Message Date
16fbadde36 Mesh: Replace MLoop struct with generic attributes
Implements #102359.

Split the `MLoop` struct into two separate integer arrays called
`corner_verts` and `corner_edges`, referring to the vertex each corner
is attached to and the next edge around the face at each corner. These
arrays can be sliced to give access to the edges or vertices in a face.
Then they are often referred to as "poly_verts" or "poly_edges".

The main benefits are halving the necessary memory bandwidth when only
one array is used and simplifications from using regular integer indices
instead of a special-purpose struct.

The commit also starts a renaming from "loop" to "corner" in mesh code.

Like the other mesh struct of array refactors, forward compatibility is
kept by writing files with the older format. This will be done until 4.0
to ease the transition process.

Looking at a small portion of the patch should give a good impression
for the rest of the changes. I tried to make the changes as small as
possible so it's easy to tell the correctness from the diff. Though I
found Blender developers have been very inventive over the last decade
when finding different ways to loop over the corners in a face.

For performance, nearly every piece of code that deals with `Mesh` is
slightly impacted. Any algorithm that is memory bottle-necked should
see an improvement. For example, here is a comparison of interpolating
a vertex float attribute to face corners (Ryzen 3700x):

**Before** (Average: 3.7 ms, Min: 3.4 ms)
```
threading::parallel_for(loops.index_range(), 4096, [&](IndexRange range) {
  for (const int64_t i : range) {
    dst[i] = src[loops[i].v];
  }
});
```

**After** (Average: 2.9 ms, Min: 2.6 ms)
```
array_utils::gather(src, corner_verts, dst);
```

That's an improvement of 28% to the average timings, and it's also a
simplification, since an index-based routine can be used instead.
For more examples using the new arrays, see the design task.

Pull Request: blender/blender#104424
2023-03-20 15:55:13 +01:00
bad85fe8c7 Cleanup: Avoid keeping mesh normals references after dirty tag
Without assumptions of a mesh's direct ownership of its normals,
this was error prone, since normals could potentially be reallocated
when they are tagged dirty or retrieved with write access.
2023-03-15 21:39:50 -04:00
f9132e2bbf Cleanup: Remove redundant mesh normals dirty tag function
Since normals are derived data, it's always a change to something
else that will cause them to change, like the winding order of a face
or vertex positions. So it's clearer to use tags for those things
directly. It's correct to remove the tag in one place since dirty is
the default state of a new mesh.
2023-03-15 15:57:27 -04:00
92b607d686 CustomData: add separate function to add layer from existing data
This simplifies the usage of the API and is preparation for #104478.

The `CustomData_add_layer` and `CustomData_add_layer_named` now have corresponding
`*_with_data` functions that should be used when creating the layer from existing data.

Pull Request: blender/blender#105708
2023-03-14 15:30:26 +01:00
1dc57a89e9 Mesh: Move functions to C++ header
Refactoring mesh code, it has become clear that local cleanups and
simplifications are limited by the need to keep a C public API for
mesh functions. This change makes code more obvious and makes further
refactoring much easier.

- Add a new `BKE_mesh.hh` header for a C++ only mesh API
- Introduce a new `blender::bke::mesh` namespace, documented here:
  https://wiki.blender.org/wiki/Source/Objects/Mesh#Namespaces
- Move some functions to the new namespace, cleaning up their arguments
- Move code to `Array` and `float3` where necessary to use the new API
- Define existing inline mesh data access functions to the new header
- Keep some C API functions where necessary because of RNA
- Move all C++ files to use the new header, which includes the old one

In the future it may make sense to split up `BKE_mesh.hh` more, but for
now keeping the same name as the existing header keeps things simple.

Pull Request: blender/blender#105416
2023-03-12 22:29:15 +01:00
5876573e14 Mesh: Move face shade smooth flag to a generic attribute
Currently the shade smooth status for mesh faces is stored as part of
`MPoly::flag`. As described in #95967, this moves that information
to a separate boolean attribute. It also flips its status, so the
attribute is now called `sharp_face`, which mirrors the existing
`sharp_edge` attribute. The attribute doesn't need to be allocated
when all faces are smooth. Forward compatibility is kept until
4.0 like the other mesh refactors.

This will reduce memory bandwidth requirements for some operations,
since the array of booleans uses 12 times less memory than `MPoly`.
It also allows faces to be stored more efficiently in the future, since
the flag is now unused. It's also possible to use generic functions to
process the values. For example, finding whether there is a sharp face
is just `sharp_faces.contains(true)`.

The `shade_smooth` attribute is no longer accessible with geometry nodes.
Since there were dedicated accessor nodes for that data, that shouldn't
be a problem. That's difficult to version automatically since the named
attribute nodes could be used in arbitrary combinations.

**Implementation notes:**
- The attribute and array variables in the code use the `sharp_faces`
  term, to be consistent with the user-facing "sharp faces" wording,
  and to avoid requiring many renames when #101689 is implemented.
- Cycles now accesses smooth face status with the generic attribute,
  to avoid overhead.
- Changing the zero-value from "smooth" to "flat" takes some care to
  make sure defaults are the same.
  - Versioning for the edge mode extrude node is particularly complex.
    New nodes are added by versioning to propagate the attribute in its
    old inverted state.
- A lot of access is still done through the `CustomData` API rather
  than the attribute API because of a few functions. That can be
  cleaned up easily in the future.
- In the future we would benefit from a way to store attributes as a
  single value for when all faces are sharp.

Pull Request: blender/blender#104422
2023-03-08 15:36:18 +01:00
f1f47df56b Cleanup: Remove unused variable 2023-03-01 15:58:01 -05:00
d939ecc46f Cleanup: Access mesh data with spans
Similar to cb62ab5b28, in newly C++ files.
2023-03-01 15:58:01 -05:00
b37111c574 Cleanup: Use consistent "vert" term for mesh normals
Use "vert" instead of "vertex" when referring to mesh normals. This was
discussed as part of 1af62cb3bf but never completely
implemented.
2023-02-27 15:52:29 -05:00
cb62ab5b28 Cleanup: Access mesh edges, faces, and loops with spans
Using spans instead of raw pointers helps to differentiate ararys from
pointers to single elements, gives bounds checking in debug builds, and
conveniently stores the number of elements in the same variable.

Also make variable naming consistent. For example, use `loops` instead
of `mloop`. The plural helps to clarify that the variable is an array.

I didn't change positions because there is a type mismatch between
C and C++ code that is ugly to manage. All remaining code can be
converted to C++, then that change will be simpler.

Pull Request #105138
2023-02-23 17:14:03 +01:00
91346755ce Cleanup: use '#' prefix for issues instead of 'T'
Match the convention from Gitea instead of Phabricator's T for tasks.
2023-02-12 14:56:05 +11:00
33c30af742 Cleanup: comments in struct declarations
Use a consistent style for declaring the names of struct members
in their declarations. Note that this convention was already used in
many places but not everywhere.

Remove spaces around the text (matching commented arguments) with
the advantage that the the spell checking utility skips these terms.
Making it possible to extract & validate these comments automatically.

Also use struct names for `bAnimChannelType` & `bConstraintTypeInfo`
which were using brief descriptions.
2023-01-16 13:27:35 +11:00
3a3d9488a1 Refactor: Const correct Custom Data API, prepare for CoW
Currently you can retrieve a mutable array from a const CustomData.
That makes code unsafe since the compiler can't check for correctness
itself. Fix that by introducing a separate function to retrieve mutable
arrays from CustomData. The new functions have the `_for_write`
suffix that make the code's intention clearer.

Because it makes retrieving write access an explicit step, this change
also makes proper copy-on-write possible for attributes.

Notes:
- The previous "duplicate referenced layer" functions are redundant
  with retrieving layers with write access
- The custom data functions that give a specific index only have
  `for_write` to simplify the API

Differential Revision: https://developer.blender.org/D14140
2023-01-13 17:22:07 -06:00
dd9e1eded0 Mesh: Move sharp edge flag to generic attribute
Move the `ME_SHARP` flag for mesh edges to a generic boolean
attribute. This will help allow changing mesh edges to just a pair
of integers, giving performance improvements. In the future it could
also give benefits for normal calculation, which could more easily
check if all or no edges are marked sharp, which is helpful considering
the plans in T93551.

The attribute is generally only allocated when it's necessary. When
leaving edit mode, it will only be created if an edge is marked sharp.
The data can be edited with geometry nodes just like a regular edge
domain boolean attribute.

The attribute is named `sharp_edge`, aiming to reflect the similar
`select_edge` naming and to allow a future `sharp_face` name in
a separate commit.

Ref T95966

Differential Revision: https://developer.blender.org/D16921
2023-01-10 16:12:14 -05:00
1af62cb3bf Mesh: Move positions to a generic attribute
**Changes**
As described in T93602, this patch removes all use of the `MVert`
struct, replacing it with a generic named attribute with the name
`"position"`, consistent with other geometry types.

Variable names have been changed from `verts` to `positions`, to align
with the attribute name and the more generic design (positions are not
vertices, they are just an attribute stored on the point domain).

This change is made possible by previous commits that moved all other
data out of `MVert` to runtime data or other generic attributes. What
remains is mostly a simple type change. Though, the type still shows up
859 times, so the patch is quite large.

One compromise is that now `CD_MASK_BAREMESH` now contains
`CD_PROP_FLOAT3`. With the general move towards generic attributes
over custom data types, we are removing use of these type masks anyway.

**Benefits**
The most obvious benefit is reduced memory usage and the benefits
that brings in memory-bound situations. `float3` is only 3 bytes, in
comparison to `MVert` which was 4. When there are millions of vertices
this starts to matter more.

The other benefits come from using a more generic type. Instead of
writing algorithms specifically for `MVert`, code can just use arrays
of vectors. This will allow eliminating many temporary arrays or
wrappers used to extract positions.

Many possible improvements aren't implemented in this patch, though
I did switch simplify or remove the process of creating temporary
position arrays in a few places.

The design clarity that "positions are just another attribute" brings
allows removing explicit copying of vertices in some procedural
operations-- they are just processed like most other attributes.

**Performance**
This touches so many areas that it's hard to benchmark exhaustively,
but I observed some areas as examples.
* The mesh line node with 4 million count was 1.5x (8ms to 12ms) faster.
* The Spring splash screen went from ~4.3 to ~4.5 fps.
* The subdivision surface modifier/node was slightly faster
RNA access through Python may be slightly slower, since now we need
a name lookup instead of just a custom data type lookup for each index.

**Future Improvements**
* Remove uses of "vert_coords" functions:
  * `BKE_mesh_vert_coords_alloc`
  * `BKE_mesh_vert_coords_get`
  * `BKE_mesh_vert_coords_apply{_with_mat4}`
* Remove more hidden copying of positions
* General simplification now possible in many areas
* Convert more code to C++ to use `float3` instead of `float[3]`
  * Currently `reinterpret_cast` is used for those C-API functions

Differential Revision: https://developer.blender.org/D15982
2023-01-10 00:10:43 -05:00
067fe443d8 Cleanup: consistent naming for normals
Use consistent naming for {vert/poly/loop/face}_normals.
2022-12-17 13:06:43 +11:00
1a8516163f Cleanup: Simplify handling of loop to poly map in normal calculation
A Loop to poly map was passed as an optional output to the loop normal
calculation. That meant it was often recalculated more than necessary.
Instead, treat it as an optional argument. This also helps relieve
unnecessary responsibilities from the already-complicated loop normal
calculation code.
2022-11-11 23:27:36 -06:00
f17fbf8065 Refactor: Rename Object->obmat to Object->object_to_world
Motivation is to disambiguate on the naming level what the matrix
actually means. It is very easy to understand the meaning backwards,
especially since in Python the name goes the opposite way (it is
called `world_matrix` in the Python API).

It is important to disambiguate the naming without making developers
to look into the comment in the header file (which is also not super
clear either). Additionally, more clear naming facilitates the unit
verification (or, in this case, space validation) when reading an
expression.

This patch calls the matrix `object_to_world` which makes it clear
from the local code what is it exactly going on. This is only done
on DNA level, and a lot of local variables still follow the old
naming.

A DNA rename is setup in a way that there is no change on the file
level, so there should be no regressions at all.

The possibility is to add `_matrix` or `_mat` suffix to the name
to make it explicit that it is a matrix. Although, not sure if it
really helps the readability, or is it something redundant.

Differential Revision: https://developer.blender.org/D16328
2022-11-01 10:48:18 +01:00
c34c6d3e25 Mesh: Move runtime data out of DNA
This commit replaces the `Mesh_Runtime` struct embedded in `Mesh`
with `blender::bke::MeshRuntime`. This has quite a few benefits:
- It's possible to use C++ types like `std::mutex`, `Array`,
  `BitVector`, etc. more easily
- Meshes saved in files are slightly smaller
- Copying and writing meshes is a bit more obvious without
  clearing of runtime data, etc.

The first is by far the most important. It will allows us to avoid a
bunch of manual memory management boilerplate that is error-prone and
annoying. It should also simplify future CoW improvements for runtime
data.

This patch doesn't change anything besides changing `mesh.runtime.data`
to `mesh.runtime->data`. The cleanups above will happen separately.

Differential Revision: https://developer.blender.org/D16180
2022-10-12 20:55:57 -05:00
548a2cbe06 Cleanup: Clang tidy
Also remove unnecessary struct keywords in C++ files.
2022-10-05 13:48:01 -05:00
97746129d5 Cleanup: replace UNUSED macro with commented args in C++ code
This is the conventional way of dealing with unused arguments in C++,
since it works on all compilers.

Regex find and replace: `UNUSED\((\w+)\)` -> `/*$1*/`
2022-10-03 17:38:16 -05:00
55387197a7 Cleanup: use function style casts for C++ 2022-10-03 11:03:46 +11:00
15e6673ccd Cleanup: Move three modifier files to C++ 2022-10-02 11:17:40 -05:00