diff --git a/source/blender/blenkernel/BKE_instances.hh b/source/blender/blenkernel/BKE_instances.hh index 9cb441e7f93..6dc6f50d05a 100644 --- a/source/blender/blenkernel/BKE_instances.hh +++ b/source/blender/blenkernel/BKE_instances.hh @@ -126,11 +126,9 @@ class Instances { */ mutable SharedCache> reference_user_counts_; - /* These almost unique ids are generated based on the `id` attribute, which might not contain - * unique ids at all. They are *almost* unique, because under certain very unlikely - * circumstances, they are not unique. Code using these ids should not crash when they are not - * unique but can generally expect them to be unique. */ - mutable SharedCache> almost_unique_ids_cache_; + /* These unique ids are generated based on the `id` attribute, which might not contain + * unique ids at all. */ + mutable SharedCache> unique_ids_cache_; public: Instances(); @@ -199,9 +197,10 @@ class Instances { */ void remove(const IndexMask &mask, const AttributeFilter &attribute_filter); /** - * Get an id for every instance. These can be used e.g. motion blur. + * Get an id for every instance. These can be used e.g. motion blur. This is based on the "id" + * attribute but makes sure that the ids are actually unique. */ - Span almost_unique_ids() const; + Span unique_ids() const; /** * Get cached user counts for every reference. @@ -225,7 +224,7 @@ class Instances { void tag_reference_handles_changed() { reference_user_counts_.tag_dirty(); - almost_unique_ids_cache_.tag_dirty(); + unique_ids_cache_.tag_dirty(); } }; diff --git a/source/blender/blenkernel/intern/instances.cc b/source/blender/blenkernel/intern/instances.cc index 5d9f1fb7fc7..0a589216a43 100644 --- a/source/blender/blenkernel/intern/instances.cc +++ b/source/blender/blenkernel/intern/instances.cc @@ -153,7 +153,7 @@ Instances::Instances(Instances &&other) instances_num_(other.instances_num_), attributes_(std::move(other.attributes_)), reference_user_counts_(std::move(other.reference_user_counts_)), - almost_unique_ids_cache_(std::move(other.almost_unique_ids_cache_)) + unique_ids_cache_(std::move(other.unique_ids_cache_)) { } @@ -162,7 +162,7 @@ Instances::Instances(const Instances &other) instances_num_(other.instances_num_), attributes_(other.attributes_), reference_user_counts_(other.reference_user_counts_), - almost_unique_ids_cache_(other.almost_unique_ids_cache_) + unique_ids_cache_(other.unique_ids_cache_) { } @@ -466,10 +466,15 @@ static Array generate_unique_instance_ids(Span original_ids) break; } if (iteration == max_iteration) { - /* It seems to be very unlikely that we ever run into this case (assuming there are less - * than 2^30 instances). However, if that happens, it's better to use an id that is not - * unique than to be stuck in an infinite loop. */ - unique_ids[instance_index] = original_id; + /* The likelyhood of running into this case is very low even if there is a huge number of + * instances. For correctness, it's still good to systematically find an unused id instead + * of purely relying on randomness. */ + for (const int generated_id : IndexRange(INT32_MAX)) { + if (used_unique_ids.add(generated_id)) { + unique_ids[instance_index] = generated_id; + break; + } + } break; } } @@ -495,9 +500,9 @@ Span Instances::reference_user_counts() const return reference_user_counts_.data(); } -Span Instances::almost_unique_ids() const +Span Instances::unique_ids() const { - almost_unique_ids_cache_.ensure([&](Array &r_data) { + unique_ids_cache_.ensure([&](Array &r_data) { const VArraySpan instance_ids = *this->attributes().lookup("id"); if (instance_ids.is_empty()) { r_data.reinitialize(instances_num_); @@ -506,7 +511,7 @@ Span Instances::almost_unique_ids() const } r_data = generate_unique_instance_ids(instance_ids); }); - return almost_unique_ids_cache_.data(); + return unique_ids_cache_.data(); } static float3 get_transform_position(const float4x4 &transform) diff --git a/source/blender/blenkernel/intern/object_dupli.cc b/source/blender/blenkernel/intern/object_dupli.cc index 2f3e4fe1b9a..46f9182b798 100644 --- a/source/blender/blenkernel/intern/object_dupli.cc +++ b/source/blender/blenkernel/intern/object_dupli.cc @@ -986,12 +986,12 @@ static void make_duplis_geometry_set_impl(const DupliContext *ctx, Span instance_offset_matrices = instances->transforms(); Span reference_handles = instances->reference_handles(); - Span almost_unique_ids = instances->almost_unique_ids(); + Span unique_ids = instances->unique_ids(); Span references = instances->references(); for (int64_t i : instance_offset_matrices.index_range()) { const InstanceReference &reference = references[reference_handles[i]]; - const int id = almost_unique_ids[i]; + const int id = unique_ids[i]; const DupliContext *ctx_for_instance = instances_ctx; /* Set the #preview_instance_index when necessary. */