Fix: Curves: Custom knots in Curves and GP operators

Current strategy to deal with operators not supporting custom NURBS
knots is to fall back to calculated knots for curves of the custom mode
but with no `CurvesGeometry::custom_knots` allocated. Such curves are
the result of operators that copy only `Point` and `Curve` domains. This
way the problem is only postponed. It is not possible to add new custom
knot curves to such `CurvesGeometry` as custom knot offsets are
calculated all together and there is no way to distinguish between old
curves with lost knots and new ones. This is more a future problem.

The actual problem in `main` can be shown with an attached blend file
(see PR) by applying `Subdivide` to some points and then adding new
`Bezier` curve to the same object. This particular problem could be
addressed somewhere in `realize_instances.cc` but the actual problem
would persist.

This PR handles custom knots in all places where `BKE_defgroup_copy_list`
is iused, and where `bke::curves::copy_only_curve_domain` is called.
Here the assumption is made that only these places can copy custom knots
modes without copying custom knots. Depending on operator logic knots are
handled most often in one of two ways:
 - `bke::curves::nurbs::copy_custom_knots`:
   copies custom knots for all curves excluding `selection`. Knot modes
   for excluded curves are altered from the custom mode to calculated.
   This way only curves modified by the operator will loose custom knots.
 - `bke::curves::nurbs::update_custom_knot_modes;`
   alters all curves to calculated mode.

In some places (e.g. `reorder.cc`) it is possible to deal with knots
without side effects.

PR also adds `BLI_assert` in `load_curve_knots` function to check if
`CurvesGeometry::custom_knots` exists for custom mode curves. Thus
versioning code is needed addressing the issue in files in case such
already exists.
Pull Request: https://projects.blender.org/blender/blender/pulls/139554
This commit is contained in:
Laurynas Duburas
2025-06-04 20:43:15 +02:00
committed by Hans Goudey
parent 2167a65e11
commit 46bc894570
19 changed files with 289 additions and 37 deletions

View File

@@ -865,13 +865,16 @@ int calculate_evaluated_num(
int knots_num(int points_num, int8_t order, bool cyclic);
/**
* Copies custom knots into given `MutableSpan`.
* Depending on KnotsMode calculates knots or copies custom knots into given `MutableSpan`.
* Adds `order - 1` length tail for cyclic curves.
*/
void copy_custom_knots(const int8_t order,
const bool cyclic,
Span<float> custom_knots,
MutableSpan<float> knots);
void load_curve_knots(KnotsMode mode,
int points_num,
int8_t order,
bool cyclic,
IndexRange curve_knots,
Span<float> custom_knots,
MutableSpan<float> knots);
/**
* Calculate the knots for a curve given its properties, based on built-in standards defined by

View File

@@ -560,6 +560,43 @@ void write_all_positions(bke::CurvesGeometry &curves,
} // namespace bezier
namespace nurbs {
/**
* Gathers NURBS custom knots of selected curves from one `CurvesGeometry` instance to another.
* Should be used to implement operator's custom knot copying logic.
* `dst_curve_offset` can be used to append knots to already existing ones in the `CurvesGeometry`.
*/
void gather_custom_knots(const bke::CurvesGeometry &src,
const IndexMask &src_curves,
int dst_curve_offset,
bke::CurvesGeometry &dst);
/**
* Overwrites `NURBS_KNOT_MODE_CUSTOM` to given ones for regular and cyclic curves.
* The purpose is to to update knot modes for curves when knot copying or calculation is not
* possible or too complex. Curve operators not supporting NURBS custom knots should call this
* function with `IndexMask` `CurvesGeometry.curves_range()`, if resulting curves are created by
* copying all attributes. This way `NURBS_KNOT_MODE_CUSTOM` values might be copied though custom
* knots not.
*/
void update_custom_knot_modes(const IndexMask &mask,
const KnotsMode mode_for_regular,
const KnotsMode mode_for_cyclic,
bke::CurvesGeometry &curves);
/**
* Copies NURBS custom knots from one `CurvesGeometry` instance to another excluding
* `exclude_curves`.
* For excluded curves with `NURBS_KNOT_MODE_CUSTOM` knot mode is overwritten to
* `NURBS_KNOT_MODE_NORMAL`.
*/
void copy_custom_knots(const bke::CurvesGeometry &src_curves,
const IndexMask &exclude_curves,
bke::CurvesGeometry &dst_curves);
} // namespace nurbs
/** \} */
/* -------------------------------------------------------------------- */

View File

@@ -52,10 +52,10 @@ int knots_num(const int points_num, const int8_t order, const bool cyclic)
return points_num + order;
}
void copy_custom_knots(const int8_t order,
const bool cyclic,
const Span<float> custom_knots,
MutableSpan<float> knots)
static void copy_custom_knots(const int8_t order,
const bool cyclic,
const Span<float> custom_knots,
MutableSpan<float> knots)
{
knots.slice(0, custom_knots.size()).copy_from(custom_knots);
if (cyclic) {
@@ -112,6 +112,24 @@ void calculate_knots(const int points_num,
}
}
void load_curve_knots(const KnotsMode mode,
const int points_num,
const int8_t order,
const bool cyclic,
const IndexRange curve_knots,
const Span<float> custom_knots,
MutableSpan<float> knots)
{
if (mode == NURBS_KNOT_MODE_CUSTOM) {
BLI_assert(!custom_knots.is_empty());
BLI_assert(!curve_knots.is_empty());
copy_custom_knots(order, cyclic, custom_knots.slice(curve_knots), knots);
}
else {
calculate_knots(points_num, mode, order, cyclic, knots);
}
}
Vector<int> calculate_multiplicity_sequence(const Span<float> knots)
{
Vector<int> multiplicity;

View File

@@ -774,15 +774,14 @@ void CurvesGeometry::ensure_nurbs_basis_cache() const
}
const int knots_num = curves::nurbs::knots_num(points.size(), order, is_cyclic);
knots.reinitialize(knots_num);
/* Some curves edit tools might not support custom knots, for example GP extrude.
* These tools create empty `custom_knots` with mode NURBS_KNOT_MODE_CUSTOM. */
if (mode == NURBS_KNOT_MODE_CUSTOM && !custom_knots.is_empty()) {
bke::curves::nurbs::copy_custom_knots(
order, is_cyclic, custom_knots.slice(custom_knots_by_curve[curve_index]), knots);
}
else {
curves::nurbs::calculate_knots(points.size(), mode, order, is_cyclic, knots);
}
curves::nurbs::load_curve_knots(mode,
points.size(),
order,
is_cyclic,
custom_knots_by_curve[curve_index],
custom_knots,
knots);
curves::nurbs::calculate_basis_cache(
points.size(), evaluated_points.size(), order, is_cyclic, knots, r_data[curve_index]);
}
@@ -1589,6 +1588,15 @@ void CurvesGeometry::remove_curves(const IndexMask &curves_to_delete,
*this = curves_copy_curve_selection(*this, curves_to_copy, attribute_filter);
}
static void reverse_custom_knots(MutableSpan<float> custom_knots)
{
const float last = custom_knots.last();
custom_knots.reverse();
for (float &knot_value : custom_knots) {
knot_value = last - knot_value;
}
}
template<typename T>
static void reverse_curve_point_data(const CurvesGeometry &curves,
const IndexMask &curve_selection,
@@ -1651,6 +1659,17 @@ void CurvesGeometry::reverse_curves(const IndexMask &curves_to_reverse)
return;
});
if (this->nurbs_has_custom_knots()) {
const OffsetIndices custom_knots_by_curve = this->nurbs_custom_knots_by_curve();
MutableSpan<float> custom_knots = this->nurbs_custom_knots_for_write();
curves_to_reverse.foreach_index(GrainSize(256), [&](const int64_t curve) {
const IndexRange curve_knots = custom_knots_by_curve[curve];
if (!custom_knots.is_empty()) {
reverse_custom_knots(custom_knots.slice(curve_knots));
}
});
}
/* In order to maintain the shape of Bezier curves, handle attributes must reverse, but also the
* values for the left and right must swap. Use a utility to swap and reverse at the same time,
* to avoid loading the attribute twice. Generally we can expect the right layer to exist when

View File

@@ -9,6 +9,8 @@
#include "BKE_curves_utils.hh"
#include "BKE_customdata.hh"
#include "BLI_array_utils.hh"
namespace blender::bke::curves {
IndexMask curve_to_point_selection(OffsetIndices<int> points_by_curve,
@@ -214,4 +216,63 @@ void write_all_positions(bke::CurvesGeometry &curves,
} // namespace bezier
namespace nurbs {
void gather_custom_knots(const bke::CurvesGeometry &src,
const IndexMask &src_curves,
const int dst_curve_offset,
bke::CurvesGeometry &dst)
{
const OffsetIndices<int> src_knots_by_curve = src.nurbs_custom_knots_by_curve();
const int start_offset = dst.nurbs_custom_knots_by_curve()[dst_curve_offset].start();
Array<int> dst_offsets(src_curves.size() + 1);
offset_indices::gather_selected_offsets(
src_knots_by_curve, src_curves, start_offset, dst_offsets);
array_utils::gather_group_to_group(src_knots_by_curve,
dst_offsets.as_span(),
src_curves,
src.nurbs_custom_knots(),
dst.nurbs_custom_knots_for_write());
}
void update_custom_knot_modes(const IndexMask &mask,
const KnotsMode mode_for_regular,
const KnotsMode mode_for_cyclic,
bke::CurvesGeometry &curves)
{
const VArray<bool> cyclic = curves.cyclic();
MutableSpan<int8_t> knot_modes = curves.nurbs_knots_modes_for_write();
mask.foreach_index(GrainSize(512), [&](const int64_t curve) {
int8_t &knot_mode = knot_modes[curve];
if (knot_mode == NURBS_KNOT_MODE_CUSTOM) {
knot_mode = cyclic[curve] ? mode_for_cyclic : mode_for_regular;
}
});
curves.nurbs_custom_knots_update_size();
}
void copy_custom_knots(const bke::CurvesGeometry &src_curves,
const IndexMask &exclude_curves,
bke::CurvesGeometry &dst_curves)
{
BLI_assert(src_curves.curves_num() == dst_curves.curves_num());
if (src_curves.nurbs_has_custom_knots()) {
/* Ensure excluded curves don't have NURBS_KNOT_MODE_CUSTOM set. */
bke::curves::nurbs::update_custom_knot_modes(
exclude_curves, NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
IndexMaskMemory memory;
bke::curves::nurbs::gather_custom_knots(
src_curves,
IndexMask::from_difference(
src_curves.nurbs_custom_knot_curves(memory), exclude_curves, memory),
0,
dst_curves);
}
}
} // namespace nurbs
} // namespace blender::bke::curves

View File

@@ -29,7 +29,10 @@
#include "BKE_anim_data.hh"
#include "BKE_animsys.h"
#include "BKE_armature.hh"
#include "BKE_curves.hh"
#include "BKE_customdata.hh"
#include "BKE_fcurve.hh"
#include "BKE_grease_pencil.hh"
#include "BKE_main.hh"
#include "BKE_mesh_legacy_convert.hh"
#include "BKE_node.hh"
@@ -68,6 +71,49 @@ static void version_fix_fcurve_noise_offset(FCurve &fcurve)
}
}
/**
* Fixes situation when `CurvesGeometry` instance has curves with `NURBS_KNOT_MODE_CUSTOM`, but has
* no custom knots.
*/
static void fix_curve_nurbs_knot_mode_custom(Main *bmain)
{
auto fix_curves = [](blender::bke::CurvesGeometry &curves) {
if (curves.custom_knots != nullptr) {
return;
}
int8_t *knot_modes = static_cast<int8_t *>(CustomData_get_layer_named_for_write(
&curves.curve_data, CD_PROP_INT8, "knots_mode", curves.curve_num));
if (knot_modes == nullptr) {
return;
}
for (const int curve : curves.curves_range()) {
int8_t &knot_mode = knot_modes[curve];
if (knot_mode == NURBS_KNOT_MODE_CUSTOM) {
knot_mode = NURBS_KNOT_MODE_NORMAL;
}
}
curves.nurbs_custom_knots_update_size();
};
LISTBASE_FOREACH (Curves *, curves_id, &bmain->hair_curves) {
blender::bke::CurvesGeometry &curves = curves_id->geometry.wrap();
fix_curves(curves);
}
LISTBASE_FOREACH (GreasePencil *, grease_pencil, &bmain->grease_pencils) {
for (GreasePencilDrawingBase *base : grease_pencil->drawings()) {
if (base->type != GP_DRAWING) {
continue;
}
blender::bke::greasepencil::Drawing &drawing =
reinterpret_cast<GreasePencilDrawing *>(base)->wrap();
fix_curves(drawing.strokes_for_write());
}
}
}
static void nlastrips_apply_fcurve_versioning(ListBase &strips)
{
LISTBASE_FOREACH (NlaStrip *, strip, &strips) {
@@ -6184,6 +6230,10 @@ void blo_do_versions_450(FileData * /*fd*/, Library * /*lib*/, Main *bmain)
FOREACH_NODETREE_END;
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 405, 86)) {
fix_curve_nurbs_knot_mode_custom(bmain);
}
/* Always run this versioning (keep at the bottom of the function). Meshes are written with the
* legacy format which always needs to be converted to the new format on file load. To be moved
* to a subversion check in 5.0. */

View File

@@ -106,6 +106,7 @@ static void append_point_knots(const Span<IndexRange> src_ranges,
for (const int appended_curve : dst_to_src_curve.index_range()) {
const int dst_curve = appended_curve + old_curves_num;
if (knot_modes[dst_curve] != NURBS_KNOT_MODE_CUSTOM) {
range++;
continue;
}
const int src_curve = dst_to_src_curve[appended_curve];
@@ -244,12 +245,7 @@ static void append_curve_knots(const IndexMask &mask, bke::CurvesGeometry &curve
{
curves.nurbs_custom_knots_update_size();
const int old_curves_num = curves.curves_num() - mask.size();
const OffsetIndices<int> knots_by_curve = curves.nurbs_custom_knots_by_curve();
MutableSpan<float> knots = curves.nurbs_custom_knots_for_write();
mask.foreach_index(GrainSize(512), [&](const int src_curve, const int appended_curve) {
const int dst_curve = old_curves_num + appended_curve;
knots.slice(knots_by_curve[dst_curve]).copy_from(knots.slice(knots_by_curve[src_curve]));
});
bke::curves::nurbs::gather_custom_knots(curves, mask, old_curves_num, curves);
}
void duplicate_curves(bke::CurvesGeometry &curves, const IndexMask &mask)
@@ -703,6 +699,10 @@ void resize_curves(bke::CurvesGeometry &curves,
});
dst_curves.update_curve_types();
if (dst_curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(
dst_curves.curves_range(), NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
}
/* Move the result into `curves`. */
curves = std::move(dst_curves);

View File

@@ -1217,6 +1217,11 @@ static bke::CurvesGeometry set_start_point(const bke::CurvesGeometry &curves,
dst_attributes);
dst_curves.update_curve_types();
/* TODO: change to copying knots by point. */
if (curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(
dst_curves.curves_range(), NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
}
return dst_curves;
}
@@ -3066,6 +3071,24 @@ static bke::CurvesGeometry extrude_grease_pencil_curves(const bke::CurvesGeometr
dst_attributes);
dst.update_curve_types();
if (src.nurbs_has_custom_knots()) {
IndexMaskMemory memory;
const VArray<int8_t> curve_types = src.curve_types();
const VArray<int8_t> knot_modes = dst.nurbs_knots_modes();
const OffsetIndices<int> dst_points_by_curve = dst.points_by_curve();
const IndexMask include_curves = IndexMask::from_predicate(
src.curves_range(), GrainSize(512), memory, [&](const int64_t curve_index) {
return curve_types[curve_index] == CURVE_TYPE_NURBS &&
knot_modes[curve_index] == NURBS_KNOT_MODE_CUSTOM &&
points_by_curve[curve_index].size() == dst_points_by_curve[curve_index].size();
});
bke::curves::nurbs::update_custom_knot_modes(
include_curves.complement(dst.curves_range(), memory),
NURBS_KNOT_MODE_ENDPOINT,
NURBS_KNOT_MODE_NORMAL,
dst);
bke::curves::nurbs::gather_custom_knots(src, include_curves, 0, dst);
}
return dst;
}

View File

@@ -341,6 +341,10 @@ blender::bke::CurvesGeometry curves_merge_by_distance(const bke::CurvesGeometry
});
});
if (dst_curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(
dst_curves.curves_range(), NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
}
return dst_curves;
}

View File

@@ -6,6 +6,7 @@
#include "BKE_attribute.hh"
#include "BKE_curves.hh"
#include "BKE_curves_utils.hh"
#include "BKE_deform.hh"
#include "GEO_curves_remove_and_split.hh"
@@ -109,6 +110,10 @@ bke::CurvesGeometry remove_points_and_split(const bke::CurvesGeometry &curves,
dst_curves.update_curve_types();
dst_curves.remove_attributes_based_on_types();
if (curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(
dst_curves.curves_range(), NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
}
return dst_curves;
}

View File

@@ -360,7 +360,10 @@ bke::CurvesGeometry extend_curves(bke::CurvesGeometry &src_curves,
}
}
});
if (src_curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(
dst_curves.curves_range(), NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
}
return dst_curves;
}

View File

@@ -516,7 +516,10 @@ static bke::CurvesGeometry fillet_curves(const bke::CurvesGeometry &src_curves,
dst_points_by_curve,
unselected,
dst_attributes);
if (src_curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(
dst_curves.curves_range(), NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
}
return dst_curves;
}

View File

@@ -5,6 +5,7 @@
#include "BLI_array_utils.hh"
#include "BLI_stack.hh"
#include "BKE_curves_utils.hh"
#include "BKE_deform.hh"
#include "GEO_merge_curves.hh"
@@ -305,6 +306,16 @@ bke::CurvesGeometry curves_merge_endpoints(const bke::CurvesGeometry &src_curves
bke::CurvesGeometry merged_curves = join_curves_ranges(ordered_curves, joined_curves_by_new);
merged_curves.cyclic_for_write().copy_from(cyclic);
/**
* `curves_merge_endpoints` seems to be working only with CURVE_TYPE_POLY, still adding this here
* in advance.
*/
if (src_curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(merged_curves.curves_range(),
NURBS_KNOT_MODE_NORMAL,
NURBS_KNOT_MODE_NORMAL,
merged_curves);
}
return merged_curves;
}

View File

@@ -6,6 +6,7 @@
#include "BKE_attribute_filters.hh"
#include "BKE_attribute_math.hh"
#include "BKE_curves.hh"
#include "BKE_curves_utils.hh"
#include "BKE_deform.hh"
#include "BKE_geometry_set.hh"
#include "BKE_instances.hh"
@@ -291,6 +292,12 @@ static void copy_and_reorder_curves(const bke::CurvesGeometry &src_curves,
attribute_filter,
dst_curves.attributes_for_write());
dst_curves.tag_topology_changed();
if (src_curves.nurbs_has_custom_knots()) {
dst_curves.nurbs_custom_knots_update_size();
IndexMaskMemory memory;
bke::curves::nurbs::gather_custom_knots(
src_curves, IndexMask::from_indices(old_by_new_map, memory), 0, dst_curves);
}
}
static void copy_and_reorder_instaces(const bke::Instances &src_instances,

View File

@@ -441,6 +441,7 @@ static CurvesGeometry resample_to_uniform(const CurvesGeometry &src_curves,
resample_to_uniform(src_curves, selection, output_ids, dst_curves);
bke::curves::nurbs::copy_custom_knots(src_curves, selection, dst_curves);
return dst_curves;
}
@@ -475,6 +476,7 @@ CurvesGeometry resample_to_count(const CurvesGeometry &src_curves,
resample_to_uniform(src_curves, selection, output_ids, dst_curves);
bke::curves::nurbs::copy_custom_knots(src_curves, selection, dst_curves);
return dst_curves;
}
@@ -526,6 +528,7 @@ CurvesGeometry resample_to_length(const CurvesGeometry &src_curves,
resample_to_uniform(src_curves, selection, output_ids, dst_curves);
bke::curves::nurbs::copy_custom_knots(src_curves, selection, dst_curves);
return dst_curves;
}
@@ -622,6 +625,7 @@ CurvesGeometry resample_to_evaluated(const CurvesGeometry &src_curves,
attribute.finish();
}
bke::curves::nurbs::copy_custom_knots(src_curves, selection, dst_curves);
return dst_curves;
}

View File

@@ -450,7 +450,7 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
attribute.dst.finish();
}
bke::curves::nurbs::copy_custom_knots(src_curves, selection, dst_curves);
return dst_curves;
}
@@ -623,7 +623,7 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
attribute.dst.finish();
}
bke::curves::nurbs::copy_custom_knots(src_curves, IndexMask(), dst_curves);
return dst_curves;
}
@@ -754,7 +754,7 @@ static bke::CurvesGeometry convert_curves_to_catmull_rom_or_poly(
attribute.dst.finish();
}
bke::curves::nurbs::copy_custom_knots(src_curves, selection, dst_curves);
return dst_curves;
}

View File

@@ -419,6 +419,7 @@ bke::CurvesGeometry subdivide_curves(const bke::CurvesGeometry &src_curves,
attribute.dst.finish();
}
bke::curves::nurbs::copy_custom_knots(src_curves, selection, dst_curves);
return dst_curves;
}

View File

@@ -1070,6 +1070,10 @@ bke::CurvesGeometry trim_curves(const bke::CurvesGeometry &src_curves,
dst_curves.remove_attributes_based_on_types();
dst_curves.tag_topology_changed();
if (src_curves.nurbs_has_custom_knots()) {
bke::curves::nurbs::update_custom_knot_modes(
dst_curves.curves_range(), NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_NORMAL, dst_curves);
}
return dst_curves;
}

View File

@@ -304,14 +304,13 @@ static void populate_curve_props_for_nurbs(const bke::CurvesGeometry &curves,
const int knots_num = bke::curves::nurbs::knots_num(tot_points, order, is_cyclic);
Array<float> temp_knots(knots_num);
if (mode == NURBS_KNOT_MODE_CUSTOM) {
bke::curves::nurbs::copy_custom_knots(
order, is_cyclic, custom_knots.slice(custom_knots_by_curve[i_curve]), temp_knots);
}
else {
bke::curves::nurbs::calculate_knots(tot_points, mode, order, is_cyclic, temp_knots);
}
bke::curves::nurbs::load_curve_knots(mode,
tot_points,
order,
is_cyclic,
custom_knots_by_curve[i_curve],
custom_knots,
temp_knots);
/* Knots should be the concatenation of all batched curves.
* https://graphics.pixar.com/usd/dev/api/class_usd_geom_nurbs_curves.html#details */