Blender grid rendering interprets voxel transforms in such a way that the voxel
values are located at the center of a voxel. This is inconsistent with OpenVDB
where the values are located at the lower corners for the purpose or sampling
and related algorithms.
While it is possible to offset grids when communicating with the OpenVDB
library, this is also error-prone and does not add any major advantage.
Every time a grid is passed to OpenVDB we currently have to take care to
transform by half a voxel to ensure correct sampling weights are used that match
the density displayed by the viewport rendering.
This patch changes volume grid generation, conversion, and rendering code so
that grid transforms match the corner-located values in OpenVDB.
- The volume primitive cube node aligns the grid transform with the location of
the first value, which is now also the same as min/max bounds input of the
node.
- Mesh<->Grid conversion does no longer require offsetting grid transform and
mesh vertices respectively by 0.5 voxels.
- Texture space for viewport rendering is offset by half a voxel, so that it
covers the same area as before and voxel centers remain at the same texture
space locations.
Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Pull Request: https://projects.blender.org/blender/blender/pulls/138449
The semantics of checking "has_value()" (etc.) are much better than
checking for an empty span when dealing with the result of an attribute
lookup. This mainly affects the Bezier curve handle position attributes
currently. Plenty of places assume those attributes exist now. In a
couple places the code is a bit safer now, otherwise it's just a bit
more obvious.
Pull Request: https://projects.blender.org/blender/blender/pulls/144506
This follows the other CMake "modernization" commits, this time for
`bf_intern_openvdb` and the OpenVDB dependency itself.
The difference with this one is that `intern/openvdb` becomes an
"optional" dependency itself. This is because downstream consumers often
want to include this dependency rather than openvdb directly, so this
target must also be optional. Optional, in this case, means the target
always exists but may be entirely empty.
Summary
- If you are using BKE APIs to access openvdb features, then use the
`bf::blenkernel` target
- If you are only using `intern/openvdb` APIs then use the
`bf::intern::optional::openvdb` target (rare)
- For all other cases, use the `bf::dependencies::optional::openvdb`
target (rare)
context: https://devtalk.blender.org/t/cmake-cleanup/30260
Pull Request: https://projects.blender.org/blender/blender/pulls/137071
Strokes would lose their deform group values after moving them to a
different layer, because `vertex_group_names` weren't transferred to
the newly created `CurvesGeometry` inside `execute_realize_curve_tasks`.
This commit adds another version of `copy_vertex_group_names` for
Curves, and separates the duplicate code into `copy_vertex_group_name`,
which is used by both meshes and curves.
This also fixed strokes losing deform weights in multiple situations, such as:
- When performing a layer merge.
- When applying generative modifiers like Mirror or Array
Pull Request: https://projects.blender.org/blender/blender/pulls/142881
The change changes the way the converted curves are evaluated.
It may be better and a valid choice, but it needs to be considered
more carefully since it would be a breaking change.
Hides Resolution field in "Curve Data" panel if at least one poly curve
is selected. For these curves, the field is ignored. Moreover its value
is `0` for them, for all other curve types minimum is `1`.
Also conversion of `CURVE_TYPE_POLY` to `CURVE_TYPE_NURBS` is fixed to
emit curves of order 2. This gives same shape as input had. Resolution
is set to `1`, because "Curve to Mesh" node ignores curves with
Resolution `0`.
Pull Request: https://projects.blender.org/blender/blender/pulls/144239
Many nodes operate on all the instances that are passed into them. For example,
the Subdivision Surface node subdivides the mesh at the root but also instanced
meshes. This works well for most nodes, but there are a few nodes were the old
`modify_geometry_sets` function was not very well defined and it was tricky to
use correctly.
The fundamental problem was that the behavior is not obvious when a node creates
or modifies instances and how those are integrated with the already existing
instances.
This patch solves this with the following changes:
* Remove the old `GeometrySet::modify_geometry_sets` and related
`*_during_modify` methods.
* Add a new `blender::geometry::foreach_real_geometry` function that is similar
to the old `modify_geometry_sets` but has a more well-defined interface:
* It never passes instances into the callback. So existing instances can't be
modified with it.
* The callback is allowed to create new instances. This will automatically be
merged back with potentially already existing instances. The callback does
not have to worry about accidentally invalidating existing instances like
before.
* A few existing usages used `modify_geometry_sets` to actually modify existing
instances (usually just removing attributes). Those can't use the new
`foreach_real_geometry`, so they just get a custom simple recursive
implementation instead of using a generic function.
Pull Request: https://projects.blender.org/blender/blender/pulls/143898
As part of the addition of free normals, the join geometry and realize
instances nodes were updated to properly join custom normals. When one
mesh input had tangent space normals and another had no custom normals,
I chose to use free custom normals for the output mesh since that has
drastically better performance. However, it turns out users get into
that situation much more often than I expected, and because many areas
still don't support free custom normals very well, and their presense
isn't obvious, this causes confusion.
This commit changes this code to output tangent space custom normals
whenever any of the input meshes have tangent space custom normals.
That also maintains the most information for propagation later, since the
"default" status of (0,0) custom normals is maintained.
Fixes#143368
Pull Request: https://projects.blender.org/blender/blender/pulls/143498
When transforming a geometry, we often apply the transposed inverse
to normals / custom normal data. However, that matrix can still contain
scale from the original matrix. That scale has to be removed so we can
avoid also scaling the normals.
I used the opportunity to remove the duplication between mesh and curves
processing of the custom normals, and to formalize an optimization to
skip the final normalization of each vector if the transform is such
that it isn't necessary. The new functions don't fit beautifully into
their public headers, but I don't know of a better place for them.
Pull Request: https://projects.blender.org/blender/blender/pulls/142896
The "curve_type" attribute in curves geometry is built-in and only valid with
a `int8` type on the `Curves` attribute domain. Adding it with a different type
on instance geometry is fine though, but causes invalid attribute writer access
when realizing the instances.
Pull Request: https://projects.blender.org/blender/blender/pulls/142218
The code added to postprocess the manifold result to remove
extra vertices introduced on edges had a logic bug in it.
It would sometimes dissolve a vertex from a triangle.
I thought that would be very rare and just repeated a vertex to
fill out the triangle, but that leads to a mesh that doesn't validate.
Anyway, my logic to prevent a single vertex from being dissolved from
a triangle was wrong (if the triangle was seen second while looking).
I fixed that, and then the even rarer case of two vertices being
dissolved from a quad showed up. I have prevented all such cases
by a loop that disables all vertex dissolving in faces where it
would leave less than three vertices.
Also added an assert (so, debug-mode only) that the mesh produced
by manifold boolean is valid.
The case of subtracing a plane is handled specially as the
plane is not manifold, but the library has a TrimByPlane function.
The special handling code did not deal with the empty result case
properly.
Also, there was no error return checking from Manifold in this special
case, so that was added.
Also, the general error handling just assumed that any Manifold error
on the inputs was a "not manifold" error, which while probably true,
should not be assumed, so that was fixed too.
To resolve, simply pass the matrix to `is_plane` and transform the
points before creating a plane from them.
NOTE: we also have a crash in main when the plane is "outside" the
target, will report that separately.
Pull Request: https://projects.blender.org/blender/blender/pulls/142336
Fix for unreported issue with Grease Pencil interpolation tool: on cyclic curves
the last point is interpolated between the end points of the curve, especially
noticeable with sequence interpolation.
This required handling a corner case in the curve sample mapping function.
This function is complex and hard to verify with the operator alone, leading to
frequent issues and discovery of yet more corner cases. For this reason i
refactored the sampling function and added new unit tests.
This should help avoid regressions and make it clear how the function is
expected to behave in various corner cases.
The `sample_curve_padded` function has been moved into the geometry module,
since the `sculpt_paint` module does not have tests yet and is intended mostly
for higher-level operator code. The function has been split to separate out the
"reverse" sampling mode, which reduces complexity. Reverse sampling is done by
first reversing the input curve points, doing regular sampling, and then
reversing the resulting samples.
The function can now sample to larger or smaller sample arrays:
- Larger output arrays have a point aligned with each source point as before,
with the rest of the points evenly distributed over the source curve.
This ensures that the output curve matches the source as closely as possible,
especially for poly curves.
- Smaller output arrays are uniformly sampled along the length of the source
curve.
Pull Request: https://projects.blender.org/blender/blender/pulls/141946
This commit moves the freestyle edge and face mark tags to become
generic attributes, similar to other changes over the past years. The
attributes are called "freestyle_edge" and "freestyle_face", and they're
now propagated like regular boolean attributes.
Compatibility wise, forward and backward blend file compatibility are
maintained (for forward compatibility this is implemented a bit
differently than in the past because of the ongoing `AttributeStorage`
transition). In the Python API, `use_freestyle_mark` has been removed;
the attribute API should be used instead (just like bevel weights).
The BMesh (`freestyle`) accessors are removed too.
The conversions benefit from the fact that bit-wise, the old structs are
the same as `bool`, so we can convert to the old and new formats without
reallocating arrays.
Pull Request: https://projects.blender.org/blender/blender/pulls/141996
Change `eCustomDataType` to `bke::AttrType` for uses of the attribute
API (the `AttributeAccessor` one anyway). I didn't touch any values that
might be saved in files; those should be handled on a case by case basis.
Part of #122398
Pull Request: https://projects.blender.org/blender/blender/pulls/141301
This commit moves Curves and Grease Pencil to use `AttributeStorage`
instead of `CustomData`, except for vertex groups. This PR mostly
involves extending the changes from the above commit for point clouds
to generalize to other geometry types.
This is mostly straightforward, though a couple non-trivial places of
note are the joining of Grease Pencil objects (`merge_attributes`), the
"default render fallback" UV for curves objects which was previously
unused at the UI level and just ended up being the first attribute, and
the `update_curve_types()` call in the curves versioning function.
Similar to:
- fa03c53d4a
- f74e304b00
Part of #122398.
Pull Request: https://projects.blender.org/blender/blender/pulls/140936
In a fix to manifold boolean, commit a20f367379, the code
sometimes dissolved vertices in triangles and then didn't remap
those vertices. This prevents the dissolve in the first place.
The boolean modifier Exact solver has a solver option "Materials"
with choices "Index Based" and "Transfer". The former uses
only materials that were in the first operand object/mesh.
The Transfer option copies new materials as needed from other
object/mesh operands and uses those on the pieces of faces from
those operands that survive into the output.
Users very often use boolean to cut away from a main mesh, and
in such cases usually don't care about the materials on the cutter
operand, and don't want materials from them transferred, so the
"Index Based" choice is the default in the modifier.
It was regarded as in important bug/lack that the new Manifold
solver did not have such an option, so this commit adds one.
The Boolean Geometry Node at the moment does not have an option
and always uses the "Transfer" method, for all three solvers.
It is a matter of discussion whether such an option should be added
in the node also, so this commit does not include such a change.
The Manifold solver, up to this point, ignored the material_remaps
argument and relied on the realize_instance code to remap the
materials (it uses the Transfer strategy).
This change overrides that remapping with the explicit mapping
handed in through the API, if the mapping has non-zero size.
Since the old way (ignoring the mapping argument) worked fine for
the Boolean Geometry Node, I changed that code to make the map
have size zero in the node, in the case that the solver is Manifold.
This is a little hacky but I couldn't think of anything much better.
Long term it might be nice to have the internal boolean API not take
in remaps at all, but rather a remapping strategy choice. One thing
that makes that difficult right now is that the modifier can get
materials from either the object or the mesh (at least that used
to be true) and the internal boolean api only knows about meshes.
Another thing that would have made this task easier (for me) would
be to have realize_instances take in a material mapping strategy
as a parameter.
The code added in commit ffc204d1fa to dissolve redundant 2-edged
vertices after a manifold boolean assumed that after dissolving such
vertices a valid face would remain. This is not true of the face
started out degenerate (all vertices on the same line).
Fixed by checking for such cases and in any case not creating
any faces with less than three vertices.
The code added in commit ffc204d1fa to dissolve redundant 2-edged
vertices after a manifold boolean assumed that after dissolving such
vertices a valid face would remain. This is not true of the face
started out degenerate (all vertices on the same line).
Fixed by checking for such cases and in any case not creating
any faces with less than three vertices.
The calls to `to_geometry_set` in this file can create a temporary
Instances struct for collections. That instances component will contain
two attributes, which are currently referenced in the attributes map
even after the temporary compoment storage goes out of scope. A simple
fix is to avoid adding these attributes to the map in the first place.
An alternative that would also be more efficient would be to handle each
instance reference type explicitly, without converting it to a temporary
geometry set. That seems to significantly complicate the code though;
for now it doesn't seem worth it.
Pull Request: https://projects.blender.org/blender/blender/pulls/140999
This is discussed more in PR #140773.
The cause of the breakage was the change of the Manifold library
version from 3.0.1 to 3.1.0. That change is very positive otherwise
because we can remove the "use runids" workaround to prevent bad
face merging, and that removal is also part of this commit.
Removing that changes the time to do a big sphere-sphere test
from 660ms to 340ms.
The problem that needed fixing is that the new library version appears
not to do some aggressive simplification that the old version did,
and as a result, when we dissolve triangulation edges after the boolean
is done, it sometimes leaves valence-2 vertices on original edges.
To fix that, new code detects and then dissolves such vertices.
This is discussed more in PR #140773.
The cause of the breakage was the change of the Manifold library
version from 3.0.1 to 3.1.0. That change is very positive otherwise
because we can remove the "use runids" workaround to prevent bad
face merging, and that removal is also part of this commit.
Removing that changes the time to do a big sphere-sphere test
from 660ms to 340ms.
The problem that needed fixing is that the new library version appears
not to do some aggressive simplification that the old version did,
and as a result, when we dissolve triangulation edges after the boolean
is done, it sometimes leaves valence-2 vertices on original edges.
To fix that, new code detects and then dissolves such vertices.
Small constant trivial referencing objects should be passed by value.
Current state of code most likely legacy from the times there was an optional strings.
Pull Request: https://projects.blender.org/blender/blender/pulls/138058
Replaces pointer based EXPECT_EQ_ARRAY with EXPECT_EQ_SPAN in most cases
as they already used spans (or span compatible datastructures).
Currently EXPECT_EQ_ARRAY only takes in one size variable and doesn't
compare the number of elements between arguments (requiring an
additional line to do so).
This should make the code cleaner and safer. Goal is also to promote
the use Spans in new test code.
Pull Request: https://projects.blender.org/blender/blender/pulls/140340
