PR https://projects.blender.org/blender/blender/pulls/144233.
Fix#142296 performance regression caused by merging UV positions
based on face area weights, replacing it with mean average weights
computation. The former method resulted in higher probability in making
subdivision cache invalid between frame updates and thus causing expensive
recreation of blender::bke::subdiv::OpenSubdiv_Evaluator object.
While this fix isn't directly answering a question why specific UV position
updates would cause this reevaluation of subdivision object, it fixes
the performance regression caused by #139595 PR.
If the BMesh already has a "custom_normal" attribute with the wrong
type, the call to `BM_lnorspace_update` won't be able to add the
attribute with the expected name, and the bevel code ends up using an
invalid offset to access the data.
For the fix, first just guard against that case. But also make sure the
harden normals functionality still works when the input mesh has free
normals. They will no be converted to tangent space normals as
necessary, in bevel and in other BMesh code that requires that
custom normal storage format.
Pull Request: https://projects.blender.org/blender/blender/pulls/143489
Fix#142045 crash on performing bevel operation on border edges.
When user selected both border edges and neigboring edges
(parallel to those border egdes) for the bevel operation,
the former were omitted during construction of BevVert objects
but their initial UV connectivity were recorded during call
to determine_uv_vert_connectivity function - specifically BMLoop
pointers were stored in BevelParams::uv_vert_maps member.
This later caused issues after rebuilding existing polygons
(bevel_rebuild_existing_polygons) since previously recorded
BMLoop pointers became invalid for border edges but still were
stored in uv_vert_maps (uv_vert_map_pop function was not called
for them since those loops were not related to BevVert objects).
This caused crash when accessing UV positions, when providing
invalid loop pointer to BM_ELEM_CD_GET_FLOAT_P function in bevel_merge_uvs.
The C++ Vector container has the benefits of the older C type,
along with improved performance, better type and memory safety,
and significantly improved ergonomics.
Pull Request: https://projects.blender.org/blender/blender/pulls/141759
Fix#140813 crash, when bevel operation doesn't choose proper representative
face (called frep, facerep or rep_face in code). In such scenario a nullptr
would be assigned to uv_face->attached_frep field in register_uv_face function
and later, as a result of that, BM_face_vert_share_loop function would crash
during call to update_uv_vert_map function.
This commit also includes additional safety check in register_uv_face function,
as well as removes compiler warning about assigned but unused center_bme variable.
This is from PR https://projects.blender.org/blender/blender/pulls/140864
but applied to the 4.5 release branch.
Problem - offset collision check for bevels did not consider the reduced
offset when using bevel edge weights, thus greatly reducing the allowed offset.
Solution - calculate the offset check using the weighted value and not
the full offset.
Alternatives - Since this error was introduced in the patch dd334faa58
a solution would be to revert the patch. This is not optimum since the
patch did correct the inf/inf condition in the offset calculation (ex at
90 degs, tan is 1/0).
Limitations - This patch is a correction to return the bevel function to
it's previous abilities. Further work is needed to properly handle edge
offset interference when looking at n-gons with reflex angles. There are
also situations where the bevel operation exceeds the checked offset distance
(exs. inner arc spread and bevel profiles that are not normal to the bevelled edge).
Fix#79163 bug related to the bevel operation producing disconnected UVs for
new bevel faces. This change replaces previous approach using scattered and
selective usage of functions: bev_merge_uvs, bev_merge_edge_uvs and
bev_merge_end_uvs with one coherent technique for all stages of the bevel operation.
It is utilizing a concept of loop (BMLoop) buckets to keep track of UV vertices
that should be merged at the end of bevel operation by a single call to
bevel_merge_uvs function. This approach doesn't touch initial UV position
calculation done by interpolation algorithm in bev_create_ngon function and
keeps the concept of representative faces (called frep, facerep or rep_face in
code) to help decide to which bucket specific loops should be assigned.
This is from PR https://projects.blender.org/blender/blender/pulls/139595,
which has more explanation and discussion.
Fix#79163 bug related to the bevel operation producing disconnected UVs for
new bevel faces. This change replaces previous approach using scattered and
selective usage of functions: bev_merge_uvs, bev_merge_edge_uvs and
bev_merge_end_uvs with one coherent technique for all stages of the bevel operation.
It is utilizing a concept of loop (BMLoop) buckets to keep track of UV vertices
that should be merged at the end of bevel operation by a single call to
bevel_merge_uvs function. This approach doesn't touch initial UV position
calculation done by interpolation algorithm in bev_create_ngon function and
keeps the concept of representative faces (called frep, facerep or rep_face in
code) to help decide to which bucket specific loops should be assigned.
This is from PR https://projects.blender.org/blender/blender/pulls/139595,
which has more explanation and discussion.
The [Fix#125024: Bevel offset - eliminate divide by 0] (#126309)
in response to [Bevel Modifier creates unwanted geometries] (#125024)
created "divide by 0" situations when checking for clamp overlap geometry
in the bevel modifier. This PR eliminates this undefined behavior.
Original PR by Rob Blair. Modified by make format and added
a needed include.
e.g. stands for "exempli gratia" in Latin which means "for example".
The best way to make sure it makes sense when writing is to just expand
it to "for example". In these cases where the text was "for e.g.", that
leaves us with "for for example" which makes no sense. This commit fixes
all 110 cases, mostly just just replacing the words with "for example",
but also restructuring the text a bit more in a few cases, mostly by
moving "e.g." to the beginning of a list in parentheses.
Pull Request: https://projects.blender.org/blender/blender/pulls/139596
The corner-cost bias used when calculating the shortest path meant
the path could cross over itself.
Resolve by tagging vertices as well as edges to prevent stepping across
vertices used by edges that have been added to the heap.
It's safer to pass a type so that it can be checked if delete should be
used instead. Also changes a few void pointer casts to const_cast so that
if the data becomes typed it's an error.
Pull Request: https://projects.blender.org/blender/blender/pulls/137404
Every call to BM_faces_join and BM_faces_join_pair has been adjusted to
provide the new face pointer, and a BLI_assert_msg has been added so
that doubles are now consistently identified and flagged as a problem.
BM_faces_join is now also capable of automatically reusing a double
when it is found. This new behavior is currently unused at this point.
Future patch-sets will begin to use it, allowing simplification of
calling functions.
Ref: !137406
This fixes most "One Definition Rule" violations inside blender proper
resulting from duplicate structures of the same name. The fixes were
made similar to that of !135491. See also #120444 for how this has come
up in the past.
These were found by using the following compile options:
-flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing
Note: There are still various ODR issues remaining that require
more / different fixes than what was done here.
Pull Request: https://projects.blender.org/blender/blender/pulls/136371
- Refactor tagging logic into a function to remove duplicate code
The two phases of tagging were identical logic just with the variables
and tag values swapped. This change ensures the two loops use matching
behavior.
- Adjust iteration to prevent restarting from the beginning of the mesh
after each island.
- Simplify and speed iteration tests.
testing to ensure v->e is non-null is redundant - verts with no edges
would never have passed `bm_vert_dissolve_fan_test` in the first
place, so can't be tagged as `VERT_INDEX_INIT`.
Re-testing `bm_vert_dissolve_fan_test(v)` is redundant - it was
checked above, and during tagging, the topology does not change.
Topology only changes later, after tagging is complete.
Therefore it's guaranteed to return the same result as the first time.
- Simplify loop logic, rename vars for clarity
offset and nth were constants, and depth was what causes alternation.
However none of that math is necessary - it can simply be done with
call order.
'seek_a' and 'seek_b' were renamed to what they actually are - lists
of verts that are tagged for collapse and ignore, respectively.
Further, by checking if any verts were tagged during the first
iteration pass, the second iteration pass can be avoided entirely if
it will perform no work.
Ref: !135212
When using clangd or running clang-tidy on headers there are
currently many errors. These are noisy in IDEs, make auto fixes
impossible, and break features like code completion, refactoring
and navigation.
This makes source/blender headers work by themselves, which is
generally the goal anyway. But #includes and forward declarations
were often incomplete.
* Add #includes and forward declarations
* Add IWYU pragma: export in a few places
* Remove some unused #includes (but there are many more)
* Tweak ShaderCreateInfo macros to work better with clangd
Some types of headers still have errors, these could be fixed or
worked around with more investigation. Mostly preprocessor
template headers like NOD_static_types.h.
Note that that disabling WITH_UNITY_BUILD is required for clangd to
work properly, otherwise compile_commands.json does not contain
the information for the relevant source files.
For more details see the developer docs:
https://developer.blender.org/docs/handbook/tooling/clangd/
Pull Request: https://projects.blender.org/blender/blender/pulls/132608
Move `CD_CUSTOMLOOPNORMAL` to the newly added
`CD_PROP_INT16_2D` generic attribute type. This is similar to
previous commits moving specific custom data types.
The attribute name is `custom_normal`. When the attribute with
that name is on the face corner domain, the code will interpret it
as stored in the existing deformation-invariant spherical coordinate
space.
The API remains the same, with the additional opportunity to edit
custom normal data as an attribute directly (which admittedly is fairly
unintuitive currently).
See #130484.
Pull Request: https://projects.blender.org/blender/blender/pulls/130689
From Rob Blair's PR https://projects.blender.org/blender/blender/pulls/126309 .
In the code that clamps the bevel amount, the existing code could get
a denominator of 0 (from a tangent of a certain angle).
This code uses a different calculation (diagram in the PR) that clamps
when the projections of four specific edges onto another edge consumes
the whole length of that edge.
This replaces the older dynamic c array macros with blender::Vector in
bmesh_polygon_edgenet. This is the only remaining use of the old array
machinery and removal of `BLI_array.h / .c` can happen immediately
afterwards.
See #103343
Pull Request: https://projects.blender.org/blender/blender/pulls/119975
As of 2a56403cb0, bevel weight data is stored as a generic
attribute. The bevel modifier is currently hard-coded to use the
`bevel_weight_edge` attribute as the data source for the weights.
This commit adds a string textbox where the user can specify alternate
attributes for the bevel's edge weights.
The string name field is added as a search button which provides a list
of all attributes in the original mesh. This is meant to work similarly
to geometry nodes, so mismatched attribute data types and domains are
automatically converted/interpolated as necessary. Attributes created
by previous modifiers can also be used, but they won't appear in the
search menu.
Co-authored-by: Hans Goudey <hans@blender.org>
Pull Request: https://projects.blender.org/blender/blender/pulls/117366
The code for marking seams or sharp in a bevel was nonsense
for terminal edges (beveled edges that are the only beveled
edge at a vertex). It was written assuming that either the
number of segments is 1 or there is a mesh ADJ pattern, which
is not the case if there is a terminal edge with segments > 1.
The immediate fix for infinite loops is to project against trying
to execute this code for terminal edges.
Also made current code clearer and removed duplication.
It is a future TODO to make it do something sensible for terminal
edges instead of a no-op. Incidentally I notice that it also doesn't
work for mitered booleans, but it won't infinite loop in that case.
To fix both of these cases, we should try to count boundary vertices
to skip rather than edges to skip.
This reverts commit f3c32a36bc and the two followups.
The commit caused issues with both the operator and the modifier.
The operator could be fixed, for the modifier this needs deeper
investigation (see #124836 for a bit more info on this).
Until a better solution is found it is just better to go back to
previous behavior.
Reintroduces #103562 for now
Pull Request: https://projects.blender.org/blender/blender/pulls/125499
`BM_mesh_triangulate` is used in exporters (when the "Triangulate"
option is ON), the `Triangulate` modifier and currently also in the
`Triangulate` geometry node (even though there are plans to change this,
see !112264)
So in practice, exporters (Alembic/FBX/OBJ/Collada) were breaking
custom normals for game pipelines (unless everything was triangulated
beforehand).
This change builds upon 93c8955a72 (uses the use
`BM_custom_loop_normals_to_vector_layer` /
`BM_custom_loop_normals_from_vector_layer` pair of calls).
In the case of the `Triangulate` modifier, this had its own try at
preserving custom normals in 7d0fcaa69a -- doing very similar
things but as an option -- this is now removed (so it is always done,
which fits into "interpolate custom data if it's there" design that we have
nowadays).
NOTE: the "Triangulate Faces" operator already did the same
Pull Request: https://projects.blender.org/blender/blender/pulls/121871
Exposed by 6c774feba2
`BM_mesh_decimate_dissolve_ex` sets up `DelimitData` with layer offset
(start), size and end so that `bm_edge_is_contiguous_loop_cd_all` can
check a range of edges for being contiguous.
The way `cd_loop_offset_end` is calculated is wrong though, it does not
take the actual start into account (this has to be added to fix the
bug). When it is wrong, it can happen that start and end are the same,
so no check actually takes place and no delimiting edges are found.
It seems that prior to 6c774feba2 the customdata layer always had an
offset of zero, so never really showed in practice (at least I couldnt
make it break in 3.4), but after 6c774feba2 we can at least observe the
following:
- when creating a bmesh, an offset would to the uv layer would still be
zero in my tests
- however, as soon as we iterate loops of a face (as done in the
report), we get an additional layer `CD_BM_ELEM_PYPTR`
- this then changes the offset
- `BM_uv_map_get_offsets_from_layer` seems to do the right thing afaict
So to resolve, just add the "start" offset to the end, to get the right
range.
NOTE: there is a very similar `DelimitData` used in
`bmesh.ops.join_triangles` and the way in which `bm_edge_delimit_cdata`
sets up te range is exactly like what this PR proposes.
Pull Request: https://projects.blender.org/blender/blender/pulls/121033
