Cleanup talked about in the previous semi-related PR, #114501
- saacos, saasin, sasqrt have been 100% identical to saacosf,
saasinf, sasqrtf since 2012.
- For all the above, there exist more intuitively named safe_acosf,
safe_asinf, safe_sqrtf that do the same thing, so switch all code to those.
Pull Request: https://projects.blender.org/blender/blender/pulls/114593
Function Module Inclusive Time Exclusive Time
--------------------------------------------------------------------------
mesh_render_data_update_normals blender 297.51 0.00
315 -> 297
acos() usage in all places related to normal calculations shows up in the
profiler. Given that "angle between faces" is only additional heuristic
weight in there (the effect of it at all is very subtle), approximate but
faster version of acos() might be just fine. Especially since some other
parts of Blender (e.g. mikktspace) use approximate acos in a conceptually
the same part.
- Adds safe_acos_approx() to BLI_math_base.hh. Implementation the same
as already exists in Cycles; max error 0.00258 degrees. Between 2x and 4x
faster in my tests.
- Changes all normals related calculations to use the function above instead
of saacos.
Computing normals on a Stanford Lucy (14m verts) mesh:
- Mac (arm64, M1 Max): 247ms -> 229ms
- Win (x64, Ryzen 5950X): 276ms -> 250ms
All places that are about "normal calculation" were changed, including e.g.
Corrective Smooth modifier. Applying that one to the same 14m vertices mesh,
Mac M1 Max: 9.96s -> 9.76s
Tiny changes in several test output expectations w.r.t. normals are
observed, these were reviewed and updated expectations checked in svn.
Pull Request: https://projects.blender.org/blender/blender/pulls/114501
Extract:
- "Attribute", when creating a new attribute with
`GEOMETRY_OT_attribute_add()`: make the default name in the operator
a null string, and set it to "Attribute" translated inside an invoke
method instead.
- Also for new attributes, from `BKE_id_attribute_calc_unique_name()`,
for instance to create a default vertex color layer when going into
Vertex Paint mode: use `DATA_()` instead of `IFACE_()`, since it
represents user data.
Disambiguate:
- "Weight" can be the thickness of font glyphs.
- "Mark as Asset" and "Clear Asset" are operator names already
extracted using the Operator context. They were recently added to a
manual translation in the UI, but the existing one can be reused.
- "Second" as a time unit in the context of frame snapping.
Some messages reported by Satoshi Yamasaki in #43295.
Pull Request: https://projects.blender.org/blender/blender/pulls/114159
Root of the issue was that a1d7ec7139 (from !110109, new comptibility
handling) missed to update code of `handle_subversion_warning`, file
version numbers should be checked here now, not the minversion ones.
On top of that, this report has been changed to a `RPT_WARNING` instead
of a `RPT_ERROR`, and reports 'print level' for file reading was set to
`WARNING` level way too late to have any effect in existing code...
This commit removes knowledge about face sets from the PBVH,
and changes iteration over faces to not depend on the PBVH face
iterator abstraction. Though this adds slightly more boilerplate to
iteration over faces, it makes the whole process more data oriented
and allows use of index-based utilities like `gather` and `scatter`
in the mesh case, and simpler iteration over BMesh faces for
dynamic topology.
Setting face sets is now specialized per PBVH type in a few places
in a similar way. The general goal is to reduce branching and function
calls at the lowest level of hot loops, and to make code more aware
of the data structures it uses, both for performance and clarity.
Since the remaining uses of the face iterator are removed,
the iterator itself is removed too.
Related commits:
- 97f2b01ea9
- 756dea7ca1
- a6a2af5fdd
Pull Request: https://projects.blender.org/blender/blender/pulls/114417
The issue crash was caused by non-safe cast from a bare C array
float[4][4] to float4x4*. Such cast is not safe because it might
break alignment.
Modified the code so that the transform matrices are passed by
value.
Technically it could mean higher memory usage, but odds that the
actual geometry which is a part of boolean operation consumes much
more memory. Additionally, avoiding indirection could potentially
lead to better performance.
Pull Request: https://projects.blender.org/blender/blender/pulls/114421
Adds a new scene tool setting `use_grease_pencil_multi_frame_editing`.
The `foreach_*_drawing` functions are moved to the `ed::greasepencil` namespace in the editor, since they are now context sensitive and depend on the toolsetting. They are now named `retrieve_editable_drawings` and `retrieve_visible_drawings` and return
an array of drawings instead of calling a callback function.
Pull Request: https://projects.blender.org/blender/blender/pulls/114283
Issue was reported with a 3.6 file crashing 3.3LTS, but the underlying
problem still exists in current codebase, and could also potentially be
caused by other edge-cases.
Pull Request: https://projects.blender.org/blender/blender/pulls/114397
Blender 4.0 added new socket types that get written into legacy node
group interfaces by forward compatibility code. Such unsupported socket
types have to be handled by the socket declaration system and ignored
during execution.
Ported blender-v3.6-release fix#114056
Pull Request: https://projects.blender.org/blender/blender/pulls/114401
Create the face set layer in the BMesh when the face set brush is first
used. This requires iterating through all faces to set the default value
of 1 because the default of 0 (SCULPT_FACE_SET_NONE) has different
behavior defined elsewhere.
Pair-reviewed in person with Sergey
These two values are already stored in the mesh, and they have no
relation to the PBVH's goal as an acceleration structure. Similar to
a6a2af5fdd, remove the values from the PBVH and
just access them from the mesh as necessary.
Pair-reviewed in person with Sergey
The user-level goal is to make it so boundaries are preserved in a
much better manner.
At this point of the project boundaries are considered edges which
are marked as seam or sharp. It is possible to expand this logic to
more cases like boundary between face sets. For now the main focus
in the algorithm itself.
The rough idea is the following:
- Splitting an edge is "lossless" for boundary, so there is no need in
any special handling of edges there.
- When collapsing an edge prefer to collapse edges which are not
boundary but adjacent to it. The vertex which is adjacent to boundary
does is preserved and is not moved during the edge collapse algorithm.
- Then collapse boundary edges.
There are a bit of tricky parts, especially with the flaps. They are
explained in the code and ASCII diagrams are provided for better clarity.
Co-authored-by: Hans Goudey <hans@blender.org>
Pull Request: https://projects.blender.org/blender/blender/pulls/113836
The issue was that the undo system detected that node trees have been changed
even though they have not. This is because the memory dump of the node tree
changed after unrelated changes. The root issue was that
`bNodeTreeInterface::write` also wrote the struct, and the pointer of the
`bNodeTreeInterface` itself. While this pointer is generally stable, it is not
during file-write, because when IDs are written, a shallow copy is created
first (see `BLO_write_init_id_buffer_from_id`). The temporary buffer for this
shallow copy is allocated on the heap, so its pointer may change over time.
The fix is to simply not write the `bNodeTreeInterface` separately, since it's
already embedded in `bNodeTree`.
Pull Request: https://projects.blender.org/blender/blender/pulls/114322
The last good commit was 8474716abb.
After this commits from main were pushed to blender-v4.0-release. These are
being reverted.
Commits a4880576dc from to b26f176d1a that happend afterwards were meant for
4.0, and their contents is preserved.
Creating a new window then a new scene would crash on undo
because the undo step loaded would not contain the new windows scene.
Regression introduced since 3.6.
This issue happend when a geometry nodes modifier wouldn't output grease
pencil geometry. In this case, we write an emtpy grease pencil
data-block to the evaluated `geometry_set`. But we don't update the
`eval_frame` that indicates on what frame the grease pencil data-block
was evaluated on.
The fix makes sure we update the `eval_frame` even if the output is
empty.
Additionally, there was a missing return in the
`get_evaluated_grease_pencil_drawing_deformation` function that lead
to an empty `deformation.positions` span.
The fix makes sure we return the correct deformation with the correct
positions.
When retrieving a color attribute to paint it, the layer data needs to
be unshared so that it doesn't modify data from separate meshes.
In the past I think we've unknowingly avoided this problem by porting
code to the new attribute API. I've been refactoring `PBVH` and
`SculptSession` to make that possible (removing the long-lived
layer pointers in the two structs). But that wouldn't fit as a bug fix.
In the meantime, make sure the color attribute data is un-shared
and separate "for read" and "for write" versions of the function.
Pull Request: https://projects.blender.org/blender/blender/pulls/114119
A curve with only two points shouldn't be considered cyclic. Otherwise
a duplicate edge is added between the points. This fixes the utility
that calculates the segment count to account for this and adjusts
other code to account for that as necessary. Curves now evaluate
correctly to a single evaluated segment in this case (in the case of
Bezier and NURBS curves, it depends on the resolution though).
Co-authored-by: Dalai Felinto <dalai@blender.org>
Pull Request: https://projects.blender.org/blender/blender/pulls/114095
The PBVH is given the evaluated positions from the end of the modifier
stack, but then updates normals from the original mesh based on those
positions. This causes a feedback loop, even when in vertex paint mode.
As mentioned in a comment, calculating these normals and duplicating
the arrays is quite wasteful. But it's necessary since the PBVH always
assumes we are interested in modifying the positions, and also always
retrieves the normals from the original mesh too. Those things would
be good to untangle at some point too, but for now this change is small
and helps to clarify the situation as well.
Caused by: b339e3937d
Pull Request: https://projects.blender.org/blender/blender/pulls/114118
The goal is to make it possible to have more or less reliable way
to perform performance comparisons of changes in algorithms used
in dynamic topology.
This change allows to run Blender with the following arguments:
blender --log "sculpt.detail,pbvh.bmesh" --log-level 2
and see timing of dynamic topology flood-fill operation, as well as
individual timings of edge subdivision/collapsing.
Pull Request: https://projects.blender.org/blender/blender/pulls/114099
No functional changes.
Changing old C code to C++ by returning a `Vector` from `ANIM_setting_get_rna_values`.
This reduces the argument count for that function and simplifies the code.
Pull Request: https://projects.blender.org/blender/blender/pulls/113931
The issue was that a node was supposed to propagate an anonymous
attribute that is only created further to the right in the tree. This does not
during inferencing, where uses of fields can only come to the right of its
creation. Note, all fields coming out of the repeat input/output node are
new field sources during inferencing.
Now, only field sources that are passed from the outside into the repeat zone
can be propagated from the repeat output to the repeat input node.
Solving this also showed another issue where anonymous attributes are
not properly propagated through a repeat zone where there is no link between
the repeat input and output node. In such cases, data is still propagated between
those two nodes when the number of iterations is zero.
Known limitations to be addressed separately:
* We are not warning/keeping track of the named layers.
* There is no lookup for layers (groups) yet.
Ref !113908.
This field supports either a Layer, or a Layer Group name as input,
and outputs a selection.
The nodes that use this should add the name to the list of Named
Dependencies of the node-tree.
Ref !113908.
