Changeset View
Changeset View
Standalone View
Standalone View
source/blender/blenlib/intern/edgehash.c
| Show First 20 Lines • Show All 188 Lines • ▼ Show 20 Lines | else if (index == SLOT_DUMMY) { | ||||
| eh->dummy_count--; | eh->dummy_count--; | ||||
| return edgehash_insert_at_slot(eh, slot, edge, value); | return edgehash_insert_at_slot(eh, slot, edge, value); | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| BLI_INLINE EdgeHashEntry *edgehash_lookup_entry(EdgeHash *eh, uint v0, uint v1) | BLI_INLINE EdgeHashEntry *edgehash_lookup_entry(EdgeHash *eh, uint v0, uint v1) | ||||
| { | { | ||||
| /* This is an invalid edge; normally this does not happen in Blender, but it can be part of an | |||||
| * imported mesh with invalid geometry. See T76514. */ | |||||
| if (v0 == v1) { | |||||
JacquesLucke: I'm not really a fan of adding this check into this core code path. This function is quite hot… | |||||
| return NULL; | |||||
| } | |||||
| Edge edge = init_edge(v0, v1); | Edge edge = init_edge(v0, v1); | ||||
| ITER_SLOTS (eh, edge, slot, index) { | ITER_SLOTS (eh, edge, slot, index) { | ||||
| if (EH_INDEX_HAS_EDGE(eh, index, edge)) { | if (EH_INDEX_HAS_EDGE(eh, index, edge)) { | ||||
| return &eh->entries[index]; | return &eh->entries[index]; | ||||
| } | } | ||||
| else if (index == SLOT_EMPTY) { | else if (index == SLOT_EMPTY) { | ||||
| return NULL; | return NULL; | ||||
| ▲ Show 20 Lines • Show All 434 Lines • Show Last 20 Lines | |||||
I'm not really a fan of adding this check into this core code path. This function is quite hot / executed often. While branch prediction should make this branch very cheap, unnecessary branches should be avoided anyway.
How many API calls depend on this behavior? If it's just one during mesh validation, I'd prefer to move this check there. Different users of the API might have to handle this case differently anyway.
We don't allow v0 == v1 in init_edge to detect invalid API usage. I think it's bad to allow v0 == v1 in some API functions, but not in others.