griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #139042: use index map instead of sorting transform system data

Browse Source 
Avoid modifying the order of transform system data. Instead, create an
index map and use that to traverse the data arrays in sorted order.

The issue observed in #139042 stems from the assumption, in _some_ of
the code, that `tc->data[i]`, `tc->data_ext[i]`, and `tc->data_2d[i]`
all contain information about the same "transformable thing". Since
`tc->data` was sorted (by selection state and, optionally for
proportional editing, by distance) but the other arrays were not, this
caused issues.

The most obvious solution, sorting all arrays the same way, turned out
to be hard to do, as some elements in one array have pointers to
elements in another array. Reordering those arrays would therefore
also make it necessary to find and update those pointers.

Instead, I decided to implement a sorted index map. The arrays can
then be kept in their original order, and the index map can be used to
visit them in sorted order.

Pull Request: https://projects.blender.org/blender/blender/pulls/140132
This commit is contained in:
Sybren A. Stüvel
2025-06-19 11:20:08 +02:00
parent 4fbcdba21e
commit df6d345bb4
7 changed files with 133 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
source/blender/editors/transform/transform.cc
View File

@@ -1948,7 +1948,14 @@ bool initTransform(bContext *C, TransInfo *t, wmOperator *op, const wmEvent *eve
if (t->flag & T_PROP_EDIT) {
bool has_selected_any = false;
FOREACH_TRANS_DATA_CONTAINER (t, tc) {
if (tc->data->flag & TD_SELECTED) {
if (tc->data_len == 0) {
continue;
}
BLI_assert(tc->sorted_index_map);
const int first_selected_index = tc->sorted_index_map[0];
TransData *td = &tc->data[first_selected_index];
if (td->flag & TD_SELECTED) {
has_selected_any = true;
break;
}

15
source/blender/editors/transform/transform.hh
View File

@@ -717,6 +717,21 @@ struct TransDataContainer {
};
TransCustomDataContainer custom;
/**
* Array of indices for the `data`, `data_ext`, and `data_2d` arrays.
*
* When using this index map to traverse the arrays, they will be sorted primarily by selection
* state (selected before unselected). Depending on the sort function used (see below),
* unselected items are then sorted by their "distance" for proportional editing.
*
* NOTE: this is set to `nullptr` by default; use one of the sorting functions below to
* initialize the array.
*
* \see #sort_trans_data_selected_first Sorts only by selection state.
* \see #sort_trans_data_dist Sorts by selection state and distance.
*/
int *sorted_index_map;
};
struct TransInfo {

134
source/blender/editors/transform/transform_convert.cc
View File

@@ -12,6 +12,8 @@
#include "MEM_guardedalloc.h"
#include "BLI_array_utils.hh"
#include "BLI_function_ref.hh"
#include "BLI_kdtree.h"
#include "BLI_linklist_stack.h"
#include "BLI_listbase.h"
@@ -81,51 +83,57 @@ void transform_around_single_fallback(TransInfo *t)
/** \name Proportional Editing
* \{ */
static int trans_data_compare_dist(const void *a, const void *b)
/**
* Construct `tc->sorted_index_map` so that its indices visit `tc->{data,data_ext,data_2d}` in
* sorted order, given the compare function.
*/
static void make_sorted_index_map(TransDataContainer *tc, FunctionRef<bool(int, int)> compare)
{
const TransData *td_a = (const TransData *)a;
const TransData *td_b = (const TransData *)b;
BLI_assert(tc->sorted_index_map == nullptr);
tc->sorted_index_map = MEM_malloc_arrayN<int>(tc->data_len, __func__);
if (td_a->dist < td_b->dist) {
return -1;
}
if (td_a->dist > td_b->dist) {
return 1;
}
return 0;
}
static int trans_data_compare_rdist(const void *a, const void *b)
{
const TransData *td_a = (const TransData *)a;
const TransData *td_b = (const TransData *)b;
if (td_a->rdist < td_b->rdist) {
return -1;
}
if (td_a->rdist > td_b->rdist) {
return 1;
}
return 0;
const MutableSpan sorted_index_span(tc->sorted_index_map, tc->data_len);
array_utils::fill_index_range(sorted_index_span);
std::sort(sorted_index_span.begin(), sorted_index_span.end(), compare);
}
/**
* Construct an index map to visit `tc->data`, `tc->data_ext`, and `tc->data_2d` in order of
* selection state (selected first). Unselected items are visited by either their `dist` or `rdist`
* property, depending on a flag in `t`.
*/
static void sort_trans_data_dist_container(const TransInfo *t, TransDataContainer *tc)
{
TransData *start = tc->data;
int i;
for (i = 0; i < tc->data_len && start->flag & TD_SELECTED; i++) {
start++;
}
if (i < tc->data_len) {
if (t->flag & T_PROP_CONNECTED) {
qsort(start, size_t(tc->data_len) - i, sizeof(TransData), trans_data_compare_dist);
const bool use_dist = (t->flag & T_PROP_CONNECTED);
const auto compare = [&](const int a, const int b) {
/* If both selected, then they are equivalent. To keep memory access sequential (and thus more
* predictable for pre-caching) when iterating the arrays, keep them sorted by array index. */
const bool is_selected_a = tc->data[a].flag & TD_SELECTED;
const bool is_selected_b = tc->data[b].flag & TD_SELECTED;
if (is_selected_a && is_selected_b) {
return a < b;
}
else {
qsort(start, size_t(tc->data_len) - i, sizeof(TransData), trans_data_compare_rdist);
/* Selected comes before unselected. */
if (is_selected_a) {
return true;
}
}
if (is_selected_b) {
return false;
}
/* If both are unselected, only then the distance matters. */
if (use_dist) {
return tc->data[a].dist < tc->data[b].dist;
}
return tc->data[a].rdist < tc->data[b].rdist;
};
/* The "sort by distance" is often preceeded by "calculate distance", which is
* often preceeded by "sort selected first". */
MEM_SAFE_FREE(tc->sorted_index_map);
make_sorted_index_map(tc, compare);
}
void sort_trans_data_dist(TransInfo *t)
{
@@ -135,33 +143,29 @@ void sort_trans_data_dist(TransInfo *t)
}
/**
* Make #TD_SELECTED first in the array.
* Construct an index map to visit `tc->data`, `tc->data_ext`, and `tc->data_2d` in order of
* selection state (selected first).
*/
static void sort_trans_data_selected_first_container(TransDataContainer *tc)
{
TransData *sel, *unsel;
TransData temp;
unsel = tc->data;
sel = &tc->data[tc->data_len - 1];
while (sel > unsel) {
while (unsel->flag & TD_SELECTED) {
unsel++;
if (unsel == sel) {
return;
}
BLI_assert_msg(tc->sorted_index_map == nullptr,
"Expected sorting by selection state to only happen once");
const auto compare = [&](const int a, const int b) {
/* If the selection state is the same, they are equivalent. To keep memory
* access sequential (and thus more predictable for pre-caching) when
* iterating the arrays, keep them sorted by array index. */
const bool is_selected_a = tc->data[a].flag & TD_SELECTED;
const bool is_selected_b = tc->data[b].flag & TD_SELECTED;
if (is_selected_a == is_selected_b) {
return a < b;
}
while (!(sel->flag & TD_SELECTED)) {
sel--;
if (unsel == sel) {
return;
}
}
temp = *unsel;
*unsel = *sel;
*sel = temp;
sel--;
unsel++;
}
/* If A is selected, a comes before b, so return true.
* If B is selected, a comes after b, so return false. */
return is_selected_a;
};
make_sorted_index_map(tc, compare);
}
static void sort_trans_data_selected_first(TransInfo *t)
{
@@ -230,8 +234,9 @@ static void set_prop_dist(TransInfo *t, const bool with_dist)
/* Count number of selected. */
int td_table_len = 0;
FOREACH_TRANS_DATA_CONTAINER (t, tc) {
TransData *td = tc->data;
for (a = 0; a < tc->data_len; a++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (td->flag & TD_SELECTED) {
td_table_len++;
}
@@ -252,8 +257,9 @@ static void set_prop_dist(TransInfo *t, const bool with_dist)
int td_table_index = 0;
FOREACH_TRANS_DATA_CONTAINER (t, tc) {
TransData *td = tc->data;
for (a = 0; a < tc->data_len; a++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (td->flag & TD_SELECTED) {
/* Initialize, it was malloced. */
td->rdist = 0.0f;

18
source/blender/editors/transform/transform_convert_mesh.cc
View File

@@ -2161,8 +2161,9 @@ Array<TransDataVertSlideVert> transform_mesh_vert_slide_data_create(
const TransDataContainer *tc, Vector<float3> &r_loc_dst_buffer)
{
int td_selected_len = 0;
TransData *td = tc->data;
for (int i = 0; i < tc->data_len; i++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
/* The selected ones are sorted at the beginning. */
break;
@@ -2173,8 +2174,8 @@ Array<TransDataVertSlideVert> transform_mesh_vert_slide_data_create(
Array<TransDataVertSlideVert> r_sv(td_selected_len);
r_loc_dst_buffer.reserve(r_sv.size() * 4);
td = tc->data;
for (int i = 0; i < tc->data_len; i++, td++) {
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
/* The selected ones are sorted at the beginning. */
break;
@@ -2296,8 +2297,9 @@ Array<TransDataEdgeSlideVert> transform_mesh_edge_slide_data_create(const TransD
/* Ensure valid selection. */
BMIter iter;
BMVert *v;
TransData *td = tc->data;
for (int i = 0; i < tc->data_len; i++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
/* The selected ones are sorted at the beginning. */
break;
@@ -2332,8 +2334,8 @@ Array<TransDataEdgeSlideVert> transform_mesh_edge_slide_data_create(const TransD
Array<TransDataEdgeSlideVert> r_sv(td_selected_len);
TransDataEdgeSlideVert *sv = r_sv.data();
int sv_index = 0;
td = tc->data;
for (int i = 0; i < tc->data_len; i++, td++) {
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
continue;
}

12
source/blender/editors/transform/transform_convert_mesh_uv.cc
View File

@@ -502,8 +502,9 @@ struct UVGroups {
/* Now, count and set the index for the corners being transformed. */
this->sd_len = 0;
TransData *td = tc->data;
for (int i = 0; i < tc->data_len; i++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
/* The selected ones are sorted at the beginning. */
break;
@@ -519,7 +520,7 @@ struct UVGroups {
groups_offs_buffer_.reserve(this->sd_len);
groups_offs_indices_.reserve((this->sd_len / 4) + 2);
td = tc->data;
TransData *td = tc->data;
for (int i = 0; i < tc->data_len; i++, td++) {
BMLoop *l_orig = static_cast<BMLoop *>(td->extra);
if (BM_elem_index_get(l_orig) == -1) {
@@ -839,8 +840,9 @@ Array<TransDataEdgeSlideVert> transform_mesh_uv_edge_slide_data_create(const Tra
/* First we just need to "clean up" the neighboring loops.
* This way we can identify where a group of sliding edges starts and where it ends. */
TransData *td = tc->data;
for (int i = 0; i < tc->data_len; i++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
/* The selected ones are sorted at the beginning. */
break;

1
source/blender/editors/transform/transform_generics.cc
View File

@@ -790,6 +790,7 @@ void postTrans(bContext *C, TransInfo *t)
MEM_SAFE_FREE(tc->data_mirror);
MEM_SAFE_FREE(tc->data_ext);
MEM_SAFE_FREE(tc->data_2d);
MEM_SAFE_FREE(tc->sorted_index_map);
}
}

33
source/blender/editors/transform/transform_snap.cc
View File

@@ -1371,21 +1371,24 @@ void tranform_snap_target_median_calc(const TransInfo *t, float r_median[3])
zero_v3(r_median);
FOREACH_TRANS_DATA_CONTAINER (t, tc) {
TransData *td = tc->data;
int i;
float v[3];
zero_v3(v);
for (i = 0; i < tc->data_len && td->flag & TD_SELECTED; i++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
break;
}
add_v3_v3(v, td->center);
}
if (i == 0) {
if (tc->data_len == 0) {
/* Is this possible? */
continue;
}
mul_v3_fl(v, 1.0 / i);
mul_v3_fl(v, 1.0 / tc->data_len);
if (tc->use_local_mat) {
mul_m4_v3(tc->mat, v);
@@ -1460,10 +1463,14 @@ static void snap_source_closest_fn(TransInfo *t)
/* Object mode. */
if (t->options & CTX_OBJECT) {
int i;
FOREACH_TRANS_DATA_CONTAINER (t, tc) {
TransData *td;
for (td = tc->data, i = 0; i < tc->data_len && td->flag & TD_SELECTED; i++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
break;
}
std::optional<Bounds<float3>> bounds;
if ((t->options & CTX_OBMODE_XFORM_OBDATA) == 0) {
@@ -1516,9 +1523,13 @@ static void snap_source_closest_fn(TransInfo *t)
}
else {
FOREACH_TRANS_DATA_CONTAINER (t, tc) {
TransData *td = tc->data;
int i;
for (i = 0; i < tc->data_len && td->flag & TD_SELECTED; i++, td++) {
BLI_assert(tc->sorted_index_map);
for (const int i : Span(tc->sorted_index_map, tc->data_len)) {
TransData *td = &tc->data[i];
if (!(td->flag & TD_SELECTED)) {
break;
}
float loc[3];
float dist;