Fix: Instances: ensure unique instance ids
The previous code to get unique instance ids did not actually ensure 100% uniqueness. Now the code does and hence the "almost" from the name is removed. In practice, this change likely does not have any impact because the fallback case is very unlikely to be reached. Pull Request: https://projects.blender.org/blender/blender/pulls/144324
This commit is contained in:
@@ -126,11 +126,9 @@ class Instances {
|
||||
*/
|
||||
mutable SharedCache<Array<int>> 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<Array<int>> almost_unique_ids_cache_;
|
||||
/* These unique ids are generated based on the `id` attribute, which might not contain
|
||||
* unique ids at all. */
|
||||
mutable SharedCache<Array<int>> 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<int> almost_unique_ids() const;
|
||||
Span<int> 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();
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -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<int> generate_unique_instance_ids(Span<int> 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<int> Instances::reference_user_counts() const
|
||||
return reference_user_counts_.data();
|
||||
}
|
||||
|
||||
Span<int> Instances::almost_unique_ids() const
|
||||
Span<int> Instances::unique_ids() const
|
||||
{
|
||||
almost_unique_ids_cache_.ensure([&](Array<int> &r_data) {
|
||||
unique_ids_cache_.ensure([&](Array<int> &r_data) {
|
||||
const VArraySpan<int> instance_ids = *this->attributes().lookup<int>("id");
|
||||
if (instance_ids.is_empty()) {
|
||||
r_data.reinitialize(instances_num_);
|
||||
@@ -506,7 +511,7 @@ Span<int> 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)
|
||||
|
||||
@@ -986,12 +986,12 @@ static void make_duplis_geometry_set_impl(const DupliContext *ctx,
|
||||
|
||||
Span<float4x4> instance_offset_matrices = instances->transforms();
|
||||
Span<int> reference_handles = instances->reference_handles();
|
||||
Span<int> almost_unique_ids = instances->almost_unique_ids();
|
||||
Span<int> unique_ids = instances->unique_ids();
|
||||
Span<InstanceReference> 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. */
|
||||
|
||||
Reference in New Issue
Block a user