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: https://projects.blender.org/blender/blender/pulls/105416
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: https://projects.blender.org/blender/blender/pulls/104422
Blender currently has 2 algorithms for merging vertices:
- `BKE_mesh_merge_verts`;
- `blender::geometry::create_merged_mesh`
`BKE_mesh_merge_verts` has a simplified algorithm to work with Array,
Mirror and Screw modifiers. It doesn't support merge results that would
create new faces. However it has shortcuts to be more efficient in
these modifiers.
`blender::geometry::create_merged_mesh` tries to predict all possible
outcomes. So it's a more complex. But it loses in performance to
`BKE_mesh_merge_verts` in some cases.
The performance comparison between these two depends on many factors.
`blender::geometry::create_merged_mesh` works with a context that has
only the affected geometry. Thus a smaller region of the mesh is read
for duplicate checking. Therefore, the smaller the affected geometry,
the more efficient the operation.
By my tests `blender::geometry::create_merged_mesh` beats
`BKE_mesh_merge_verts` when less than 20% of the geometry is affected
in worst case `MESH_MERGE_VERTS_DUMP_IF_EQUAL` or 17% in case of
`MESH_MERGE_VERTS_DUMP_IF_MAPPED` .
For cases where the entire geometry is affected, a 30% loss was noticed,
largely due to the creation of a context that represents the entire mesh.
Co-authored-by: Germano Cavalcante <germano.costa@ig.com.br>
Pull Request #105136
**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
Improve a few messages, but mostly fix typos in many areas of the UI.
See inline comments in the differential revisiion for the rationale
behind the various changes.
Differential Revision: https://developer.blender.org/D16716
The calls in the remesh operator were unnecessary because the mesh is
about to be replaced anyway, and nothing invalidates the caches, and
the call in BMesh -> Mesh conversion was unnecessary because the caches
are cleared at the top of the function already.
The goal is to improve clarity and readability, without
introducing big design changes.
Follows the recent obmat to object_to_world refactor: the
similar naming is used, and it is a run-time only rename,
meaning, there is no affect on .blend files.
This patch does not touch the redundant inversions. Those
can be removed in almost (if not all) cases, but it would
be the best to do it as a separate change.
Differential Revision: https://developer.blender.org/D16367
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
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*/`
Correction of U.dpi to hold actual monitor DPI. Simplify font sizing by
omitting DPI as API argument, always using 72 internally.
See D15961 for more details.
Differential Revision: https://developer.blender.org/D15961
Reviewed by Campbell Barton
Whether faces are hidden and face sets are orthogonal concepts, but
currently sculpt mode stores them together in the face set array.
This means that if anything is hidden, there must be face sets,
and if there are face sets, we have to keep track of what is hidden.
In other words, it adds a bunch of redundant work and state tracking.
On the user level it's nice that face sets and hiding are consistent,
but we don't need to store them together to accomplish that.
This commit uses the `".hide_poly"` attribute from rB2480b55f216c to
read and change hiding in sculpt mode. Face sets don't need to be
negative anymore, and a bunch of "face set <-> hide status" conversion
can be removed. Plus some other benefits:
- We don't need to allocate either array quite as much.
- The hide status can be read from 1/4 the memory as face sets.
- Updates when entering or exiting sculpt mode can be removed.
- More opportunities for early-outs when nothing is hidden.
- Separating concerns makes sculpt code more obvious.
- It will be easier to convert face sets into a generic int attribute.
Differential Revision: https://developer.blender.org/D15950
corresponding data layers and using their values for computations.
Avoiding that should increase performance in many operations that
would otherwise have to read, write, or propagate these values.
It also means decreased memory usage-- not just for sculpt mode
but for any mesh that was in sculpt mode. Previously the mask, face set,
and hide status layers were *always* allocated by sculpt mode.
Here are a few basic tests when masking and face sets are not used:
| Test | Before | After |
| Subsurf Modifier | 148 ms | 126 ms |
| Sculpt Overlay Extraction | 24 ms every redraw | 0 ms |
| Memory usage | 252 MB | 236 MB |
I wouldn't expect any difference when they are used though.
The code changes are mostly just making sculpt features safe for when
the layers aren't stored, and some changes to the conversion to and
from the hide layers. Use of the ".hide_poly" attribute replaces testing
whether face sets are negative in many places.
Differential Revision: https://developer.blender.org/D15937
- Remove "take ownership" argument which was confusing and always true
- The argument made ownership very confusing
- Better to avoid boolean arguments that switch a function's purpose
- Remove "mask" argument which was basically wrong and not used properly
- "EVERYTHING" was used because developers are wary of removing data
- Instead use `CD_MASK_MESH` for its purpose of original mesh data
- Remove use of shallow copied temporary mesh, which is unnecessary now
- Split shape key processing into separate functions and use C++ types
- Copy fields explicitly rather than using memcpy for the whole struct
- Use higher level functions and avoid redundant code
- The whole idea is pretty simple and can be built from standard logic
- Adjust `CustomData` logic to be consistent with "assign" expectations
- Clear the layer data from the source, and moves the anonymous ID
Differential Revision: https://developer.blender.org/D15857
Use `verts` instead of `vertices` and `polys` instead of `polygons`
in the API added in 05952aa94d. This aligns better with
existing naming where the shorter names are much more common.
For copy-on-write, we want to share attribute arrays between meshes
where possible. Mutable pointers like `Mesh.mvert` make that difficult
by making ownership vague. They also make code more complex by adding
redundancy.
The simplest solution is just removing them and retrieving layers from
`CustomData` as needed. Similar changes have already been applied to
curves and point clouds (e9f82d3dc7, 410a6efb74). Removing use of
the pointers generally makes code more obvious and more reusable.
Mesh data is now accessed with a C++ API (`Mesh::edges()` or
`Mesh::edges_for_write()`), and a C API (`BKE_mesh_edges(mesh)`).
The CoW changes this commit makes possible are described in T95845
and T95842, and started in D14139 and D14140. The change also simplifies
the ongoing mesh struct-of-array refactors from T95965.
**RNA/Python Access Performance**
Theoretically, accessing mesh elements with the RNA API may become
slower, since the layer needs to be found on every random access.
However, overhead is already high enough that this doesn't make a
noticible differenc, and performance is actually improved in some
cases. Random access can be up to 10% faster, but other situations
might be a bit slower. Generally using `foreach_get/set` are the best
way to improve performance. See the differential revision for more
discussion about Python performance.
Cycles has been updated to use raw pointers and the internal Blender
mesh types, mostly because there is no sense in having this overhead
when it's already compiled with Blender. In my tests this roughly
halves the Cycles mesh creation time (0.19s to 0.10s for a 1 million
face grid).
Differential Revision: https://developer.blender.org/D15488
SCULPT_undo_push_begin no longer takes an explicit
name. Instead it takes a wmOperator pointer and uses
op->type->name for the name. This is necassary for
the redo panel to work and should fix the entire class
of bugs related to misspelled undo push names.
Cases where the calling operator is not registered
may use SCULPT_undo_push_begin_ex if desired; it
takes a name string as before.
This patch makes constant size a default for size edit operator of voxel remesh.
In turn, pressing CTRL now enables relative scale, the old default.
Patch also changes workspace status text entry with new additions. Note that it is a simple text and not an array of keymaps (for that further changes are needed)
{F13082567}
Reviewed By: Julien Kaspar & Joseph Eagar
Differential Revision: https://developer.blender.org/D14975
Ref D14975
Introduces an option for BKE_unit_value_as_string to skip stripping of zeroes, thus reducing flickering when using edit voxel size widget.
{F13125416}
Reviewed By: Julien Kaspar & Joseph Eagar
Differential Revision: https://developer.blender.org/D15085
Ref D15085
Fix for T84962
Before the patch, edit voxel size always displayed voxel size without units, just as a number in meters.
Now it changes like in the voxel remesh panel and shows correct units
Video:
{F13009428}
In adaptive mode:
{F13009435}
Reviewed By: JulienKaspar
Maniphest Tasks: T84962
Differential Revision: https://developer.blender.org/D14682
This is mostly a cleanup to avoid hardcoding the eager calculation of
normals it isn't necessary, by reducing calls to `BKE_mesh_calc_normals`
and by removing calls to `BKE_mesh_normals_tag_dirty` when the mesh
is newly created and already has dirty normals anyway. This reduces
boilerplate code and makes the "dirty by default" state more clear.
Any regressions from this commit should be easy to fix, though the
lazy calculation is solid enough that none are expected.
Also solves two warnings from the previous similar commit,
f688e3cc31. The change to the grease pencil
modifier is quite suspicious, but doesn't change the behavior,
which was already broken.
Using flags makes checking multiple modifiers at once more convenient
and avoids macros/functions such as IS_EVENT_MOD & WM_event_modifier_flag
which have been removed. It also simplifies checking if modifier keys
have changed.
Use a shorter/simpler license convention, stops the header taking so
much space.
Follow the SPDX license specification: https://spdx.org/licenses
- C/C++/objc/objc++
- Python
- Shell Scripts
- CMake, GNUmakefile
While most of the source tree has been included
- `./extern/` was left out.
- `./intern/cycles` & `./intern/atomic` are also excluded because they
use different header conventions.
doc/license/SPDX-license-identifiers.txt has been added to list SPDX all
used identifiers.
See P2788 for the script that automated these edits.
Reviewed By: brecht, mont29, sergey
Ref D14069
As described in T91186, this commit moves mesh vertex normals into a
contiguous array of float vectors in a custom data layer, how face
normals are currently stored.
The main interface is documented in `BKE_mesh.h`. Vertex and face
normals are now calculated on-demand and cached, retrieved with an
"ensure" function. Since the logical state of a mesh is now "has
normals when necessary", they can be retrieved from a `const` mesh.
The goal is to use on-demand calculation for all derived data, but
leave room for eager calculation for performance purposes (modifier
evaluation is threaded, but viewport data generation is not).
**Benefits**
This moves us closer to a SoA approach rather than the current AoS
paradigm. Accessing a contiguous `float3` is much more efficient than
retrieving data from a larger struct. The memory requirements for
accessing only normals or vertex locations are smaller, and at the
cost of more memory usage for just normals, they now don't have to
be converted between float and short, which also simplifies code
In the future, the remaining items can be removed from `MVert`,
leaving only `float3`, which has similar benefits (see T93602).
Removing the combination of derived and original data makes it
conceptually simpler to only calculate normals when necessary.
This is especially important now that we have more opportunities
for temporary meshes in geometry nodes.
**Performance**
In addition to the theoretical future performance improvements by
making `MVert == float3`, I've done some basic performance testing
on this patch directly. The data is fairly rough, but it gives an idea
about where things stand generally.
- Mesh line primitive 4m Verts: 1.16x faster (36 -> 31 ms),
showing that accessing just `MVert` is now more efficient.
- Spring Splash Screen: 1.03-1.06 -> 1.06-1.11 FPS, a very slight
change that at least shows there is no regression.
- Sprite Fright Snail Smoosh: 3.30-3.40 -> 3.42-3.50 FPS, a small
but observable speedup.
- Set Position Node with Scaled Normal: 1.36x faster (53 -> 39 ms),
shows that using normals in geometry nodes is faster.
- Normal Calculation 1.6m Vert Cube: 1.19x faster (25 -> 21 ms),
shows that calculating normals is slightly faster now.
- File Size of 1.6m Vert Cube: 1.03x smaller (214.7 -> 208.4 MB),
Normals are not saved in files, which can help with large meshes.
As for memory usage, it may be slightly more in some cases, but
I didn't observe any difference in the production files I tested.
**Tests**
Some modifiers and cycles test results need to be updated with this
commit, for two reasons:
- The subdivision surface modifier is not responsible for calculating
normals anymore. In master, the modifier creates different normals
than the result of the `Mesh` normal calculation, so this is a bug
fix.
- There are small differences in the results of some modifiers that
use normals because they are not converted to and from `short`
anymore.
**Future improvements**
- Remove `ModifierTypeInfo::dependsOnNormals`. Code in each modifier
already retrieves normals if they are needed anyway.
- Copy normals as part of a better CoW system for attributes.
- Make more areas use lazy instead of eager normal calculation.
- Remove `BKE_mesh_normals_tag_dirty` in more places since that is
now the default state of a new mesh.
- Possibly apply a similar change to derived face corner normals.
Differential Revision: https://developer.blender.org/D12770
The mirror modifiers merge option caused unnecessary re-ordering
to the vertex array with original vertices merging into their copies.
While this wasn't an error, it meant creating a 1:1 mapping from input
vertices to their final output wasn't reliable (when looping over
vertices first to last) as is done in
BKE_editmesh_vert_coords_when_deformed.
As merging in either direction is supported, keep the source meshes
vertices in-order since it allows the vertex coordinates to be extracted.
NOTE: Since this change introduce issues for some cases (e.g. bound
modifiers like SurfaceDeform), this change is only applied to newly
created modifiers, existing ones will still use the old incorrect merge
behavior.
Reviewed By: @brecht
Maniphest Tasks: T93321, T91444
Differential Revision: https://developer.blender.org/D13355
Because mesh vertex and face normals are just derived data, they can
be calculated lazily instead of eagerly. Often normal calculation is
a relatively expensive task, and the calculation is often redundant
if the mesh is deformed afterwards anyway.
Instead, normals should be calculated only when they are needed. This
commit moves in that direction by adding a new function to tag a mesh's
normals dirty and replacing normal calculation with it in some places.
Differential Revision: https://developer.blender.org/D12107
These functions do not change their source or input mesh, so it can
be passed with const, which means in one case that a function doesn't
have to be responsible for freeing its argument mesh, which is a clearly
better separation of concerns.
