Fix #108117: Crash in uv editor with hidden geometry

Many "UV island" style operations internally use #UvElementMap, including:

- Transform tools
- Smart-Stitch
- UV Pinch, UV Grab and UV Relax sculpt tools.

Normally, every UV in the mesh is included in the #UvElementMap.
However, with hidden geometry, only the visible geometry is included in the map. [0]
This change enforces stricter usage, reducing the chance of crashes in other areas.

Regression from [0] which was a fix for "UV Island calculation doesn't ignore hidden faces" [1].

[0]: 8f543a73ab
[1]: #99659

Pull Request: blender/blender#108130
This commit is contained in:
2023-05-22 23:23:23 +02:00
committed by Chris Blackbourn
parent e25d58a866
commit ae9ac99d2b
6 changed files with 61 additions and 46 deletions

View File

@@ -142,8 +142,15 @@ struct UvElementMap *BM_uv_element_map_create(struct BMesh *bm,
bool use_seams, bool use_seams,
bool do_islands); bool do_islands);
void BM_uv_element_map_free(struct UvElementMap *element_map); void BM_uv_element_map_free(struct UvElementMap *element_map);
/**
* Return the #UvElement associated with a given #BMLoop, or NULL if no association exists.
*
* \param element_map: The #UvElementMap to look in.
* \param l: The loop to search for.
* \return The #UvElement associated with #l, or NULL if not found. (e.g. the vertex is hidden.)
*/
struct UvElement *BM_uv_element_get(const struct UvElementMap *element_map, struct UvElement *BM_uv_element_get(const struct UvElementMap *element_map,
const struct BMFace *efa,
const struct BMLoop *l); const struct BMLoop *l);
struct UvElement *BM_uv_element_get_head(struct UvElementMap *element_map, struct UvElement *BM_uv_element_get_head(struct UvElementMap *element_map,
struct UvElement *child); struct UvElement *child);

View File

@@ -686,8 +686,8 @@ static int bm_uv_edge_select_build_islands(UvElementMap *element_map,
/* Scan forwards around the BMFace that contains element->l. */ /* Scan forwards around the BMFace that contains element->l. */
if (!uv_selected || uvedit_edge_select_test(scene, element->l, offsets)) { if (!uv_selected || uvedit_edge_select_test(scene, element->l, offsets)) {
UvElement *next = BM_uv_element_get(element_map, element->l->next->f, element->l->next); UvElement *next = BM_uv_element_get(element_map, element->l->next);
if (next->island == INVALID_ISLAND) { if (next && next->island == INVALID_ISLAND) {
UvElement *tail = element_map->head_table[next - element_map->storage]; UvElement *tail = element_map->head_table[next - element_map->storage];
stack_uv[stacksize_uv++] = tail; stack_uv[stacksize_uv++] = tail;
while (tail) { while (tail) {
@@ -702,8 +702,8 @@ static int bm_uv_edge_select_build_islands(UvElementMap *element_map,
/* Scan backwards around the BMFace that contains element->l. */ /* Scan backwards around the BMFace that contains element->l. */
if (!uv_selected || uvedit_edge_select_test(scene, element->l->prev, offsets)) { if (!uv_selected || uvedit_edge_select_test(scene, element->l->prev, offsets)) {
UvElement *prev = BM_uv_element_get(element_map, element->l->prev->f, element->l->prev); UvElement *prev = BM_uv_element_get(element_map, element->l->prev);
if (prev->island == INVALID_ISLAND) { if (prev && prev->island == INVALID_ISLAND) {
UvElement *tail = element_map->head_table[prev - element_map->storage]; UvElement *tail = element_map->head_table[prev - element_map->storage];
stack_uv[stacksize_uv++] = tail; stack_uv[stacksize_uv++] = tail;
while (tail) { while (tail) {
@@ -1186,11 +1186,11 @@ void BM_uv_element_map_free(UvElementMap *element_map)
} }
} }
UvElement *BM_uv_element_get(const UvElementMap *element_map, const BMFace *efa, const BMLoop *l) UvElement *BM_uv_element_get(const UvElementMap *element_map, const BMLoop *l)
{ {
UvElement *element = element_map->vertex[BM_elem_index_get(l->v)]; UvElement *element = element_map->vertex[BM_elem_index_get(l->v)];
while (element) { while (element) {
if (element->l->f == efa) { if (element->l == l) {
return element; return element;
} }
element = element->next; element = element->next;

View File

@@ -379,9 +379,8 @@ static void relaxation_iteration_uv(UvSculptData *sculptdata,
const UvElement *storage = sculptdata->elementMap->storage; const UvElement *storage = sculptdata->elementMap->storage;
for (int j = 0; j < total_uvs; j++) { for (int j = 0; j < total_uvs; j++) {
const UvElement *ele_curr = storage + j; const UvElement *ele_curr = storage + j;
const BMFace *efa = ele_curr->l->f; const UvElement *ele_next = BM_uv_element_get(sculptdata->elementMap, ele_curr->l->next);
const UvElement *ele_next = BM_uv_element_get(sculptdata->elementMap, efa, ele_curr->l->next); const UvElement *ele_prev = BM_uv_element_get(sculptdata->elementMap, ele_curr->l->prev);
const UvElement *ele_prev = BM_uv_element_get(sculptdata->elementMap, efa, ele_curr->l->prev);
const float *v_curr_co = ele_curr->l->v->co; const float *v_curr_co = ele_curr->l->v->co;
const float *v_prev_co = ele_prev->l->v->co; const float *v_prev_co = ele_prev->l->v->co;
@@ -598,11 +597,13 @@ static void uv_sculpt_stroke_exit(bContext *C, wmOperator *op)
op->customdata = nullptr; op->customdata = nullptr;
} }
static int uv_element_offset_from_face_get( static int uv_element_offset_from_face_get(UvElementMap *map,
UvElementMap *map, BMFace *efa, BMLoop *l, int island_index, const bool doIslands) BMLoop *l,
int island_index,
const bool do_islands)
{ {
UvElement *element = BM_uv_element_get(map, efa, l); UvElement *element = BM_uv_element_get(map, l);
if (!element || (doIslands && element->island != island_index)) { if (!element || (do_islands && element->island != island_index)) {
return -1; return -1;
} }
return element - map->storage; return element - map->storage;
@@ -686,14 +687,15 @@ static UvSculptData *uv_sculpt_stroke_init(bContext *C, wmOperator *op, const wm
/* Mouse coordinates, useful for some functions like grab and sculpt all islands */ /* Mouse coordinates, useful for some functions like grab and sculpt all islands */
UI_view2d_region_to_view(&region->v2d, event->mval[0], event->mval[1], &co[0], &co[1]); UI_view2d_region_to_view(&region->v2d, event->mval[0], event->mval[1], &co[0], &co[1]);
/* we need to find the active island here */ /* We need to find the active island here. */
if (do_island_optimization) { if (do_island_optimization) {
UvElement *element;
UvNearestHit hit = uv_nearest_hit_init_max(&region->v2d); UvNearestHit hit = uv_nearest_hit_init_max(&region->v2d);
uv_find_nearest_vert(scene, obedit, co, 0.0f, &hit); uv_find_nearest_vert(scene, obedit, co, 0.0f, &hit);
element = BM_uv_element_get(data->elementMap, hit.efa, hit.l); UvElement *element = BM_uv_element_get(data->elementMap, hit.l);
island_index = element->island; if (element) {
island_index = element->island;
}
} }
/* Count 'unique' UVs */ /* Count 'unique' UVs */
@@ -759,18 +761,18 @@ static UvSculptData *uv_sculpt_stroke_init(bContext *C, wmOperator *op, const wm
counter = 0; counter = 0;
BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) { BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) {
BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) { BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) {
int offset1, itmp1 = uv_element_offset_from_face_get( int itmp1 = uv_element_offset_from_face_get(
data->elementMap, efa, l, island_index, do_island_optimization); data->elementMap, l, island_index, do_island_optimization);
int offset2, itmp2 = uv_element_offset_from_face_get( int itmp2 = uv_element_offset_from_face_get(
data->elementMap, efa, l->next, island_index, do_island_optimization); data->elementMap, l->next, island_index, do_island_optimization);
/* Skip edge if not found(unlikely) or not on valid island */ /* Skip edge if not found(unlikely) or not on valid island */
if (itmp1 == -1 || itmp2 == -1) { if (itmp1 == -1 || itmp2 == -1) {
continue; continue;
} }
offset1 = uniqueUv[itmp1]; int offset1 = uniqueUv[itmp1];
offset2 = uniqueUv[itmp2]; int offset2 = uniqueUv[itmp2];
/* Using an order policy, sort UVs according to address space. /* Using an order policy, sort UVs according to address space.
* This avoids having two different UvEdges with the same UVs on different positions. */ * This avoids having two different UvEdges with the same UVs on different positions. */

View File

@@ -298,7 +298,7 @@ static void createTransUVs(bContext *C, TransInfo *t)
if (island_center) { if (island_center) {
UvElement *element = BM_uv_element_get(elementmap, efa, l); UvElement *element = BM_uv_element_get(elementmap, efa, l);
if (element->flag == false) { if (element && !element->flag) {
float *luv = BM_ELEM_CD_GET_FLOAT_P(l, offsets.uv); float *luv = BM_ELEM_CD_GET_FLOAT_P(l, offsets.uv);
add_v2_v2(island_center[element->island].co, luv); add_v2_v2(island_center[element->island].co, luv);
island_center[element->island].co_num++; island_center[element->island].co_num++;

View File

@@ -5451,8 +5451,10 @@ static void uv_isolate_selected_islands(const Scene *scene,
BM_elem_flag_enable(efa, BM_ELEM_TAG); BM_elem_flag_enable(efa, BM_ELEM_TAG);
BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) { BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) {
if (!uvedit_edge_select_test(scene, l, offsets)) { if (!uvedit_edge_select_test(scene, l, offsets)) {
UvElement *element = BM_uv_element_get(elementmap, efa, l); UvElement *element = BM_uv_element_get(elementmap, l);
is_island_not_selected[element->island] = true; if (element) {
is_island_not_selected[element->island] = true;
}
} }
} }
} }
@@ -5463,9 +5465,9 @@ static void uv_isolate_selected_islands(const Scene *scene,
continue; continue;
} }
BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) { BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) {
UvElement *element = BM_uv_element_get(elementmap, efa, l); UvElement *element = BM_uv_element_get(elementmap, l);
/* Deselect all elements of islands which are not completely selected. */ /* Deselect all elements of islands which are not completely selected. */
if (is_island_not_selected[element->island] == true) { if (element && is_island_not_selected[element->island]) {
BM_ELEM_CD_SET_BOOL(l, offsets.select_vert, false); BM_ELEM_CD_SET_BOOL(l, offsets.select_vert, false);
BM_ELEM_CD_SET_BOOL(l, offsets.select_edge, false); BM_ELEM_CD_SET_BOOL(l, offsets.select_edge, false);
} }

View File

@@ -677,10 +677,10 @@ static void stitch_uv_edge_generate_linked_edges(GHash *edge_hash, StitchState *
/* check to see if other vertex of edge belongs to same vertex as */ /* check to see if other vertex of edge belongs to same vertex as */
if (BM_elem_index_get(iter1->l->next->v) == elemindex2) { if (BM_elem_index_get(iter1->l->next->v) == elemindex2) {
iter2 = BM_uv_element_get(element_map, iter1->l->f, iter1->l->next); iter2 = BM_uv_element_get(element_map, iter1->l->next);
} }
else if (BM_elem_index_get(iter1->l->prev->v) == elemindex2) { else if (BM_elem_index_get(iter1->l->prev->v) == elemindex2) {
iter2 = BM_uv_element_get(element_map, iter1->l->f, iter1->l->prev); iter2 = BM_uv_element_get(element_map, iter1->l->prev);
} }
if (iter2) { if (iter2) {
@@ -1163,7 +1163,7 @@ static int stitch_process_data(StitchStateContainer *ssc,
BM_ITER_MESH (efa, &iter, bm, BM_FACES_OF_MESH) { BM_ITER_MESH (efa, &iter, bm, BM_FACES_OF_MESH) {
/* just to test if face was added for processing. /* just to test if face was added for processing.
* uvs of unselected vertices will return NULL */ * uvs of unselected vertices will return NULL */
UvElement *element = BM_uv_element_get(state->element_map, efa, BM_FACE_FIRST_LOOP(efa)); UvElement *element = BM_uv_element_get(state->element_map, BM_FACE_FIRST_LOOP(efa));
if (element) { if (element) {
int numoftris = efa->len - 2; int numoftris = efa->len - 2;
@@ -1795,8 +1795,12 @@ static UvEdge *uv_edge_get(BMLoop *l, StitchState *state)
{ {
UvEdge tmp_edge; UvEdge tmp_edge;
UvElement *element1 = BM_uv_element_get(state->element_map, l->f, l); UvElement *element1 = BM_uv_element_get(state->element_map, l);
UvElement *element2 = BM_uv_element_get(state->element_map, l->f, l->next); UvElement *element2 = BM_uv_element_get(state->element_map, l->next);
if (!element1 || !element2) {
return NULL;
}
int uv1 = state->map[element1 - state->element_map->storage]; int uv1 = state->map[element1 - state->element_map->storage];
int uv2 = state->map[element2 - state->element_map->storage]; int uv2 = state->map[element2 - state->element_map->storage];
@@ -1906,10 +1910,9 @@ static StitchState *stitch_init(bContext *C,
} }
BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) { BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) {
UvElement *element = BM_uv_element_get(state->element_map, efa, l); UvElement *element = BM_uv_element_get(state->element_map, l);
int itmp1 = element - state->element_map->storage; int itmp1 = element - state->element_map->storage;
int itmp2 = BM_uv_element_get(state->element_map, efa, l->next) - int itmp2 = BM_uv_element_get(state->element_map, l->next) - state->element_map->storage;
state->element_map->storage;
UvEdge *edge; UvEdge *edge;
int offset1 = map[itmp1]; int offset1 = map[itmp1];
@@ -2015,8 +2018,8 @@ static StitchState *stitch_init(bContext *C,
faceIndex = state_init->to_select[selected_count].faceIndex; faceIndex = state_init->to_select[selected_count].faceIndex;
elementIndex = state_init->to_select[selected_count].elementIndex; elementIndex = state_init->to_select[selected_count].elementIndex;
efa = BM_face_at_index(em->bm, faceIndex); efa = BM_face_at_index(em->bm, faceIndex);
element = BM_uv_element_get( element = BM_uv_element_get(state->element_map,
state->element_map, efa, BM_iter_at_index(NULL, BM_LOOPS_OF_FACE, efa, elementIndex)); BM_iter_at_index(NULL, BM_LOOPS_OF_FACE, efa, elementIndex));
stitch_select_uv(element, state, 1); stitch_select_uv(element, state, 1);
} }
} }
@@ -2031,13 +2034,12 @@ static StitchState *stitch_init(bContext *C,
faceIndex = state_init->to_select[selected_count].faceIndex; faceIndex = state_init->to_select[selected_count].faceIndex;
elementIndex = state_init->to_select[selected_count].elementIndex; elementIndex = state_init->to_select[selected_count].elementIndex;
efa = BM_face_at_index(em->bm, faceIndex); efa = BM_face_at_index(em->bm, faceIndex);
element = BM_uv_element_get( element = BM_uv_element_get(state->element_map,
state->element_map, efa, BM_iter_at_index(NULL, BM_LOOPS_OF_FACE, efa, elementIndex)); BM_iter_at_index(NULL, BM_LOOPS_OF_FACE, efa, elementIndex));
uv1 = map[element - state->element_map->storage]; uv1 = map[element - state->element_map->storage];
element = BM_uv_element_get( element = BM_uv_element_get(
state->element_map, state->element_map,
efa,
BM_iter_at_index(NULL, BM_LOOPS_OF_FACE, efa, (elementIndex + 1) % efa->len)); BM_iter_at_index(NULL, BM_LOOPS_OF_FACE, efa, (elementIndex + 1) % efa->len));
uv2 = map[element - state->element_map->storage]; uv2 = map[element - state->element_map->storage];
@@ -2070,7 +2072,7 @@ static StitchState *stitch_init(bContext *C,
BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) { BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) {
BM_ITER_ELEM_INDEX (l, &liter, efa, BM_LOOPS_OF_FACE, i) { BM_ITER_ELEM_INDEX (l, &liter, efa, BM_LOOPS_OF_FACE, i) {
if (uvedit_uv_select_test(scene, l, offsets)) { if (uvedit_uv_select_test(scene, l, offsets)) {
UvElement *element = BM_uv_element_get(state->element_map, efa, l); UvElement *element = BM_uv_element_get(state->element_map, l);
if (element) { if (element) {
stitch_select_uv(element, state, 1); stitch_select_uv(element, state, 1);
} }
@@ -2111,7 +2113,7 @@ static StitchState *stitch_init(bContext *C,
} }
BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) { BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) {
UvElement *element = BM_uv_element_get(state->element_map, efa, BM_FACE_FIRST_LOOP(efa)); UvElement *element = BM_uv_element_get(state->element_map, BM_FACE_FIRST_LOOP(efa));
if (element) { if (element) {
state->tris_per_island[element->island] += (efa->len > 2) ? efa->len - 2 : 0; state->tris_per_island[element->island] += (efa->len > 2) ? efa->len - 2 : 0;
@@ -2484,8 +2486,10 @@ static StitchState *stitch_select(bContext *C,
} }
/* This works due to setting of tmp in find nearest uv vert */ /* This works due to setting of tmp in find nearest uv vert */
UvElement *element = BM_uv_element_get(state->element_map, hit.efa, hit.l); UvElement *element = BM_uv_element_get(state->element_map, hit.l);
stitch_select_uv(element, state, false); if (element) {
stitch_select_uv(element, state, false);
}
return state; return state;
} }