From dd6c918b2ccc6ca546e9a8c5cb5ac169a59e8316 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sat, 25 Nov 2017 20:28:12 +0100 Subject: Fix broken atomic_cas lock in own recent commit in bmesh. Using atomic cas correctly is really hairy... ;) In this case, the returned value from cas needs to validate *two* conditions, it must not be FLT_MAX (which is our 'locked' value and would mean another thread has already locked it), but it also must be equal to previously stored value... This means we need two steps per loop here, hence using a 'for' loop instead of a 'while' one now. Note that collisions are (as expected) very rare, less than 1 for 10k typically, so did not catch the issue initially (also because I was mostly working with release build to check on performances...). --- source/blender/bmesh/intern/bmesh_mesh.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'source') diff --git a/source/blender/bmesh/intern/bmesh_mesh.c b/source/blender/bmesh/intern/bmesh_mesh.c index f6c235271bf..c0cd2e31753 100644 --- a/source/blender/bmesh/intern/bmesh_mesh.c +++ b/source/blender/bmesh/intern/bmesh_mesh.c @@ -412,18 +412,24 @@ static void mesh_verts_calc_normals_accum_cb(void *userdata, MempoolIterData *mp * It also assumes that collisions between threads are highly unlikely, * else performances would be quite bad here. */ float virtual_lock = v_no[0]; - while ((virtual_lock = atomic_cas_float(&v_no[0], virtual_lock, FLT_MAX)) == FLT_MAX) { + for (virtual_lock = v_no[0]; + (virtual_lock == FLT_MAX) || (atomic_cas_float(&v_no[0], virtual_lock, FLT_MAX) != virtual_lock); + virtual_lock = v_no[0]) + { /* This loops until following conditions are met: * - v_no[0] has same value as virtual_lock (i.e. it did not change since last try). * - v_no_[0] was not FLT_MAX, i.e. it was not locked by another thread. */ } + BLI_assert(v_no[0] == FLT_MAX); /* Now we own that normal value, and can change it. * But first scalar of the vector must not be changed yet, it's our lock! */ virtual_lock += f_no[0] * fac; v_no[1] += f_no[1] * fac; v_no[2] += f_no[2] * fac; /* Second atomic operation to 'release' our lock on that vector and set its first scalar value. */ + /* Note that we do not need to loop here, since we 'locked' v_no[0], + * nobody should have changed it in the mean time. */ virtual_lock = atomic_cas_float(&v_no[0], FLT_MAX, virtual_lock); BLI_assert(virtual_lock == FLT_MAX); -- cgit v1.2.3 From 3c1f3c02c60f6ace330c53299640a6ca6499b6b9 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sat, 25 Nov 2017 23:14:54 +0100 Subject: Fix for Fix (c): broken atomic lock in own bmesh code. That was a nasty one, Debug build would never have any issue (even tried with 64 threads!), but Release build would deadlock nearly immediately, even with only 2 threads! What happened here (I think) is that gcc optimizer would generate a specific path endlessly looping when initial value of virtual_lock was FLT_MAX, by-passing re-assignment from v_no[0] and the atomic cas completely. Which would have been correct, should v_no[0] not have been shared (and modified) by multiple threads. ;) Idea of that (broken) for loop was to avoid completely calling the atomic cas as long as v_no[0] was locked by some other thread, but... Guess the avoided/missing memory barrier was the root of the issue here. Lesson of the evening: Remember kids, do not trust your compiler to understand all possible threading-related side effects, and be explicit rather than elegant when using atomic ops! Side-effect lesson: do check both release and debug builds when messing with said atomic ops... --- source/blender/bmesh/intern/bmesh_mesh.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'source') diff --git a/source/blender/bmesh/intern/bmesh_mesh.c b/source/blender/bmesh/intern/bmesh_mesh.c index c0cd2e31753..0f97321d9b9 100644 --- a/source/blender/bmesh/intern/bmesh_mesh.c +++ b/source/blender/bmesh/intern/bmesh_mesh.c @@ -412,14 +412,16 @@ static void mesh_verts_calc_normals_accum_cb(void *userdata, MempoolIterData *mp * It also assumes that collisions between threads are highly unlikely, * else performances would be quite bad here. */ float virtual_lock = v_no[0]; - for (virtual_lock = v_no[0]; - (virtual_lock == FLT_MAX) || (atomic_cas_float(&v_no[0], virtual_lock, FLT_MAX) != virtual_lock); - virtual_lock = v_no[0]) - { + while (true) { /* This loops until following conditions are met: * - v_no[0] has same value as virtual_lock (i.e. it did not change since last try). - * - v_no_[0] was not FLT_MAX, i.e. it was not locked by another thread. + * - v_no[0] was not FLT_MAX, i.e. it was not locked by another thread. */ + const float vl = atomic_cas_float(&v_no[0], virtual_lock, FLT_MAX); + if (vl == virtual_lock && vl != FLT_MAX) { + break; + } + virtual_lock = vl; } BLI_assert(v_no[0] == FLT_MAX); /* Now we own that normal value, and can change it. -- cgit v1.2.3 From 941deaca7ad8fa269228a2110ead97aeea775809 Mon Sep 17 00:00:00 2001 From: Joshua Leung Date: Sun, 26 Nov 2017 13:05:33 +1300 Subject: Fix T53393: Change from 'd' key to 'draw' panel button causes pencil to be activated immediately instead of upon LMB --- source/blender/editors/gpencil/gpencil_paint.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'source') diff --git a/source/blender/editors/gpencil/gpencil_paint.c b/source/blender/editors/gpencil/gpencil_paint.c index cff198db6c0..f54356dfed3 100644 --- a/source/blender/editors/gpencil/gpencil_paint.c +++ b/source/blender/editors/gpencil/gpencil_paint.c @@ -2745,6 +2745,8 @@ static const EnumPropertyItem prop_gpencil_drawmodes[] = { void GPENCIL_OT_draw(wmOperatorType *ot) { + PropertyRNA *prop; + /* identifiers */ ot->name = "Grease Pencil Draw"; ot->idname = "GPENCIL_OT_draw"; @@ -2761,11 +2763,12 @@ void GPENCIL_OT_draw(wmOperatorType *ot) ot->flag = OPTYPE_UNDO | OPTYPE_BLOCKING; /* settings for drawing */ - PropertyRNA *prop; ot->prop = RNA_def_enum(ot->srna, "mode", prop_gpencil_drawmodes, 0, "Mode", "Way to interpret mouse movements"); + prop = RNA_def_collection_runtime(ot->srna, "stroke", &RNA_OperatorStrokeElement, "Stroke", ""); RNA_def_property_flag(prop, PROP_HIDDEN | PROP_SKIP_SAVE); /* NOTE: wait for input is enabled by default, so that all UI code can work properly without needing users to know about this */ - RNA_def_boolean(ot->srna, "wait_for_input", true, "Wait for Input", "Wait for first click instead of painting immediately"); + prop = RNA_def_boolean(ot->srna, "wait_for_input", true, "Wait for Input", "Wait for first click instead of painting immediately"); + RNA_def_property_flag(prop, PROP_SKIP_SAVE); } -- cgit v1.2.3 From 5b225c59bb5d0d0284279da16895925099d3879a Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 26 Nov 2017 13:40:26 +1100 Subject: Cleanup: move edge-rotate into own file --- source/blender/bmesh/CMakeLists.txt | 2 + source/blender/bmesh/operators/bmo_rotate_edge.c | 88 ++++++++++++++++++++++++ source/blender/bmesh/operators/bmo_utils.c | 54 --------------- 3 files changed, 90 insertions(+), 54 deletions(-) create mode 100644 source/blender/bmesh/operators/bmo_rotate_edge.c (limited to 'source') diff --git a/source/blender/bmesh/CMakeLists.txt b/source/blender/bmesh/CMakeLists.txt index 43e45eab98f..38a275483dd 100644 --- a/source/blender/bmesh/CMakeLists.txt +++ b/source/blender/bmesh/CMakeLists.txt @@ -40,6 +40,7 @@ set(INC_SYS ) set(SRC + # Naming convention for BMesh operators is: bmo_*action*_*details*.c operators/bmo_beautify.c operators/bmo_bevel.c operators/bmo_bisect_plane.c @@ -68,6 +69,7 @@ set(SRC operators/bmo_poke.c operators/bmo_primitive.c operators/bmo_removedoubles.c + operators/bmo_rotate_edge.c operators/bmo_similar.c operators/bmo_smooth_laplacian.c operators/bmo_split_edges.c diff --git a/source/blender/bmesh/operators/bmo_rotate_edge.c b/source/blender/bmesh/operators/bmo_rotate_edge.c new file mode 100644 index 00000000000..e4b15c6a0ef --- /dev/null +++ b/source/blender/bmesh/operators/bmo_rotate_edge.c @@ -0,0 +1,88 @@ +/* + * ***** BEGIN GPL LICENSE BLOCK ***** + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * ***** END GPL LICENSE BLOCK ***** + */ + +/** \file blender/bmesh/operators/bmo_rotate_edge.c + * \ingroup bmesh + * + * Rotate edges topology that share two faces. + */ + +#include "MEM_guardedalloc.h" + +#include "BLI_math.h" + +#include "bmesh.h" + +#include "intern/bmesh_operators_private.h" /* own include */ + +void bmo_rotate_edges_exec(BMesh *bm, BMOperator *op) +{ + BMOIter siter; + BMEdge *e, *e2; + const bool use_ccw = BMO_slot_bool_get(op->slots_in, "use_ccw"); + const bool is_single = BMO_slot_buffer_count(op->slots_in, "edges") == 1; + short check_flag = is_single ? + BM_EDGEROT_CHECK_EXISTS : + BM_EDGEROT_CHECK_EXISTS | BM_EDGEROT_CHECK_DEGENERATE; + +#define EDGE_OUT 1 +#define FACE_TAINT 1 + + BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { + /** + * this ends up being called twice, could add option to not to call check in + * #BM_edge_rotate to get some extra speed */ + if (BM_edge_rotate_check(e)) { + BMFace *fa, *fb; + if (BM_edge_face_pair(e, &fa, &fb)) { + + /* check we're untouched */ + if (BMO_face_flag_test(bm, fa, FACE_TAINT) == false && + BMO_face_flag_test(bm, fb, FACE_TAINT) == false) + { + + /* don't touch again (faces will be freed so run before rotating the edge) */ + BMO_face_flag_enable(bm, fa, FACE_TAINT); + BMO_face_flag_enable(bm, fb, FACE_TAINT); + + if (!(e2 = BM_edge_rotate(bm, e, use_ccw, check_flag))) { + + BMO_face_flag_disable(bm, fa, FACE_TAINT); + BMO_face_flag_disable(bm, fb, FACE_TAINT); +#if 0 + BMO_error_raise(bm, op, BMERR_INVALID_SELECTION, "Could not rotate edge"); + return; +#endif + + continue; + } + + BMO_edge_flag_enable(bm, e2, EDGE_OUT); + } + } + } + } + + BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "edges.out", BM_EDGE, EDGE_OUT); + +#undef EDGE_OUT +#undef FACE_TAINT + +} diff --git a/source/blender/bmesh/operators/bmo_utils.c b/source/blender/bmesh/operators/bmo_utils.c index aa1e4bc7523..e9ee1e6e552 100644 --- a/source/blender/bmesh/operators/bmo_utils.c +++ b/source/blender/bmesh/operators/bmo_utils.c @@ -121,60 +121,6 @@ void bmo_reverse_faces_exec(BMesh *bm, BMOperator *op) } } -void bmo_rotate_edges_exec(BMesh *bm, BMOperator *op) -{ - BMOIter siter; - BMEdge *e, *e2; - const bool use_ccw = BMO_slot_bool_get(op->slots_in, "use_ccw"); - const bool is_single = BMO_slot_buffer_count(op->slots_in, "edges") == 1; - short check_flag = is_single ? - BM_EDGEROT_CHECK_EXISTS : - BM_EDGEROT_CHECK_EXISTS | BM_EDGEROT_CHECK_DEGENERATE; - -#define EDGE_OUT 1 -#define FACE_TAINT 1 - - BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { - /** - * this ends up being called twice, could add option to not to call check in - * #BM_edge_rotate to get some extra speed */ - if (BM_edge_rotate_check(e)) { - BMFace *fa, *fb; - if (BM_edge_face_pair(e, &fa, &fb)) { - - /* check we're untouched */ - if (BMO_face_flag_test(bm, fa, FACE_TAINT) == false && - BMO_face_flag_test(bm, fb, FACE_TAINT) == false) - { - - /* don't touch again (faces will be freed so run before rotating the edge) */ - BMO_face_flag_enable(bm, fa, FACE_TAINT); - BMO_face_flag_enable(bm, fb, FACE_TAINT); - - if (!(e2 = BM_edge_rotate(bm, e, use_ccw, check_flag))) { - - BMO_face_flag_disable(bm, fa, FACE_TAINT); - BMO_face_flag_disable(bm, fb, FACE_TAINT); -#if 0 - BMO_error_raise(bm, op, BMERR_INVALID_SELECTION, "Could not rotate edge"); - return; -#endif - - continue; - } - - BMO_edge_flag_enable(bm, e2, EDGE_OUT); - } - } - } - } - - BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "edges.out", BM_EDGE, EDGE_OUT); - -#undef EDGE_OUT -#undef FACE_TAINT - -} #define SEL_FLAG 1 #define SEL_ORIG 2 -- cgit v1.2.3 From 329bf8e1bf0e9c74a7ef9426dcb237df9ac1287d Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 26 Nov 2017 17:48:00 +1100 Subject: BMesh: improve edge rotate when edges share faces Previously outcome depended on order of edges, now the longest boundary edges are rotated first, then the faces connected edges. This gives more predictable results, allowing regions containing a vertex fan to be rotated onto the next vertex. --- source/blender/bmesh/operators/bmo_rotate_edge.c | 234 ++++++++++++++++++++--- 1 file changed, 203 insertions(+), 31 deletions(-) (limited to 'source') diff --git a/source/blender/bmesh/operators/bmo_rotate_edge.c b/source/blender/bmesh/operators/bmo_rotate_edge.c index e4b15c6a0ef..d4cafdd0dde 100644 --- a/source/blender/bmesh/operators/bmo_rotate_edge.c +++ b/source/blender/bmesh/operators/bmo_rotate_edge.c @@ -27,62 +27,234 @@ #include "MEM_guardedalloc.h" #include "BLI_math.h" +#include "BLI_heap.h" #include "bmesh.h" #include "intern/bmesh_operators_private.h" /* own include */ -void bmo_rotate_edges_exec(BMesh *bm, BMOperator *op) +#define EDGE_OUT 1 +#define FACE_MARK 1 + +/** + * Rotate edges where every edge has it's own faces (we can rotate in any order). + */ +static void bm_rotate_edges_simple( + BMesh *bm, BMOperator *op, + const short check_flag, const bool use_ccw) { BMOIter siter; - BMEdge *e, *e2; - const bool use_ccw = BMO_slot_bool_get(op->slots_in, "use_ccw"); - const bool is_single = BMO_slot_buffer_count(op->slots_in, "edges") == 1; - short check_flag = is_single ? - BM_EDGEROT_CHECK_EXISTS : - BM_EDGEROT_CHECK_EXISTS | BM_EDGEROT_CHECK_DEGENERATE; - -#define EDGE_OUT 1 -#define FACE_TAINT 1 + BMEdge *e; BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { /** * this ends up being called twice, could add option to not to call check in * #BM_edge_rotate to get some extra speed */ if (BM_edge_rotate_check(e)) { - BMFace *fa, *fb; - if (BM_edge_face_pair(e, &fa, &fb)) { + BMEdge *e_rotate = BM_edge_rotate(bm, e, use_ccw, check_flag); + if (e_rotate != NULL) { + BMO_edge_flag_enable(bm, e_rotate, EDGE_OUT); + } + } + } +} + + +/** + * Edge length is just a way of ordering that's independent of order in the edges argument, + * we could use some other method since ideally all edges will be rotated, + * this just happens to be simple to calculate. + */ +static float bm_edge_calc_rotate_cost(const BMEdge *e) +{ + return -BM_edge_calc_length_squared(e); +} + +/** + * Check if this edge is a boundary: Are more than one of the connected faces edges rotating too? + */ +static float bm_edge_rotate_is_boundary(const BMEdge *e) +{ + /* Number of adjacent shared faces. */ + int count = 0; + BMLoop *l_radial_iter = e->l; + do { + /* Skip this edge. */ + BMLoop *l_iter = l_radial_iter->next; + do { + BMEdge *e_iter = l_iter->e; + const int e_iter_index = BM_elem_index_get(e_iter); + if ((e_iter_index != -1)) { + if (count == 1) { + return false; + } + count += 1; + break; + } + } while ((l_iter = l_iter->next) != l_radial_iter); + } while ((l_radial_iter = l_radial_iter->radial_next) != e->l); + return true; +} + +/** + * Rotate edges where edges share faces, + * edges which could not rotate need to be re-considered after neighbors are rotated. + */ +static void bm_rotate_edges_shared( + BMesh *bm, BMOperator *op, + short check_flag, const bool use_ccw, const int edges_len) +{ + Heap *heap = BLI_heap_new_ex(edges_len); + HeapNode **eheap_table = MEM_mallocN(sizeof(*eheap_table) * edges_len, __func__); + int edges_len_rotate = 0; + + { + BMIter iter; + BMEdge *e; + BM_ITER_MESH (e, &iter, bm, BM_EDGES_OF_MESH) { + BM_elem_index_set(e, -1); /* set_dirty! */ + } + bm->elem_index_dirty |= BM_EDGE; + } + + { + BMOIter siter; + BMEdge *e; + uint i; + BMO_ITER_INDEX (e, &siter, op->slots_in, "edges", BM_EDGE, i) { + BM_elem_index_set(e, BM_edge_is_manifold(e) ? i : -1); /* set_dirty! */ + eheap_table[i] = NULL; + } + } - /* check we're untouched */ - if (BMO_face_flag_test(bm, fa, FACE_TAINT) == false && - BMO_face_flag_test(bm, fb, FACE_TAINT) == false) - { + /* First operate on boundary edges, this is often all that's needed, + * regions that have no boundaries are handles after. */ + enum { + PASS_TYPE_BOUNDARY = 0, + PASS_TYPE_ALL = 1, + PASS_TYPE_DONE = 2, + }; + uint pass_type = PASS_TYPE_BOUNDARY; - /* don't touch again (faces will be freed so run before rotating the edge) */ - BMO_face_flag_enable(bm, fa, FACE_TAINT); - BMO_face_flag_enable(bm, fb, FACE_TAINT); - if (!(e2 = BM_edge_rotate(bm, e, use_ccw, check_flag))) { + while ((pass_type != PASS_TYPE_DONE) && (edges_len_rotate != edges_len)) { + BLI_assert(BLI_heap_is_empty(heap)); + { + BMOIter siter; + BMEdge *e; + uint i; + BMO_ITER_INDEX (e, &siter, op->slots_in, "edges", BM_EDGE, i) { + BLI_assert(eheap_table[i] == NULL); - BMO_face_flag_disable(bm, fa, FACE_TAINT); - BMO_face_flag_disable(bm, fb, FACE_TAINT); -#if 0 - BMO_error_raise(bm, op, BMERR_INVALID_SELECTION, "Could not rotate edge"); - return; -#endif + bool ok = (BM_elem_index_get(e) != -1) && BM_edge_rotate_check(e); - continue; + if (ok) { + if (pass_type == PASS_TYPE_BOUNDARY) { + ok = bm_edge_rotate_is_boundary(e); } + } + + if (ok) { + float cost = bm_edge_calc_rotate_cost(e); + eheap_table[i] = BLI_heap_insert(heap, cost, e); + } + } + } + + if (BLI_heap_is_empty(heap)) { + pass_type += 1; + continue; + } + + const int edges_len_rotate_prev = edges_len_rotate; + while (!BLI_heap_is_empty(heap)) { + BMEdge *e_best = BLI_heap_popmin(heap); + eheap_table[BM_elem_index_get(e_best)] = NULL; - BMO_edge_flag_enable(bm, e2, EDGE_OUT); + /* No problem if this fails, re-evaluate if faces connected to this edge are touched. */ + if (BM_edge_rotate_check(e_best)) { + BMEdge *e_rotate = BM_edge_rotate(bm, e_best, use_ccw, check_flag); + if (e_rotate != NULL) { + BMO_edge_flag_enable(bm, e_rotate, EDGE_OUT); + + /* invalidate so we don't try touch this again. */ + BM_elem_index_set(e_rotate, -1); /* set_dirty! */ + + edges_len_rotate += 1; + + /* Note: we could validate all edges which have not been rotated + * (not just previously degenerate edges). + * However there is no real need - they can be left until they're popped off the queue. */ + + /* We don't know the exact topology after rotating the edge, + * so loop over all faces attached to the new edge, typically this will only be two faces. */ + BMLoop *l_radial_iter = e_rotate->l; + do { + /* Skip this edge. */ + BMLoop *l_iter = l_radial_iter->next; + do { + BMEdge *e_iter = l_iter->e; + const int e_iter_index = BM_elem_index_get(e_iter); + if ((e_iter_index != -1) && (eheap_table[e_iter_index] == NULL)) { + if (BM_edge_rotate_check(e_iter)) { + /* Previously degenerate, now valid. */ + float cost = bm_edge_calc_rotate_cost(e_iter); + eheap_table[e_iter_index] = BLI_heap_insert(heap, cost, e_iter); + } + } + } while ((l_iter = l_iter->next) != l_radial_iter); + } while ((l_radial_iter = l_radial_iter->radial_next) != e_rotate->l); } } } + + /* If no actions were taken, move onto the next pass. */ + if (edges_len_rotate == edges_len_rotate_prev) { + pass_type += 1; + continue; + } } - BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "edges.out", BM_EDGE, EDGE_OUT); + BLI_heap_free(heap, NULL); + MEM_freeN(eheap_table); +} -#undef EDGE_OUT -#undef FACE_TAINT +void bmo_rotate_edges_exec(BMesh *bm, BMOperator *op) +{ + BMOIter siter; + BMEdge *e; + const int edges_len = BMO_slot_buffer_count(op->slots_in, "edges"); + const bool use_ccw = BMO_slot_bool_get(op->slots_in, "use_ccw"); + const bool is_single = (edges_len == 1); + short check_flag = is_single ? + BM_EDGEROT_CHECK_EXISTS : + BM_EDGEROT_CHECK_EXISTS | BM_EDGEROT_CHECK_DEGENERATE; + + bool is_simple = true; + if (is_single == false) { + BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { + BMFace *f_pair[2]; + if (BM_edge_face_pair(e, &f_pair[0], &f_pair[1])) { + for (uint i = 0; i < ARRAY_SIZE(f_pair); i += 1) { + if (BMO_face_flag_test(bm, f_pair[i], FACE_MARK)) { + is_simple = false; + break; + } + BMO_face_flag_enable(bm, f_pair[i], FACE_MARK); + } + if (is_simple == false) { + break; + } + } + } + } + if (is_simple) { + bm_rotate_edges_simple(bm, op, use_ccw, check_flag); + } + else { + bm_rotate_edges_shared(bm, op, check_flag, use_ccw, edges_len); + } + + BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "edges.out", BM_EDGE, EDGE_OUT); } -- cgit v1.2.3 From 23252eece61bada6659f9d7b3e923954ae2ec413 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 26 Nov 2017 18:34:21 +1100 Subject: Minor improvement to last commit Don't operate on multiple boundaries at once, instead keep collapsing from the first selected boundary. --- source/blender/bmesh/operators/bmo_rotate_edge.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'source') diff --git a/source/blender/bmesh/operators/bmo_rotate_edge.c b/source/blender/bmesh/operators/bmo_rotate_edge.c index d4cafdd0dde..dd6bf77dd3c 100644 --- a/source/blender/bmesh/operators/bmo_rotate_edge.c +++ b/source/blender/bmesh/operators/bmo_rotate_edge.c @@ -156,6 +156,18 @@ static void bm_rotate_edges_shared( if (ok) { float cost = bm_edge_calc_rotate_cost(e); + if (pass_type == PASS_TYPE_BOUNDARY) { + /* Trick to ensure once started, non boundaries are handled before other boundary edges. + * This means the first longest boundary defines the starting point which is rotated + * until all its connected edges are exhausted and the next boundary is popped off the heap. + * + * Without this we may rotate from different starting points and meet in the middle + * with obviously uneven topology. + * + * Move from negative to positive value, inverting so large values are still handled first. + */ + cost = cost != 0.0f ? -1.0f / cost : FLT_MAX; + } eheap_table[i] = BLI_heap_insert(heap, cost, e); } } -- cgit v1.2.3 From 311da4cd16c2a51c8a00cf67f96d7591c5bb1dfa Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 26 Nov 2017 20:13:18 +1100 Subject: Cleanup: rename edge -> edges --- source/blender/bmesh/CMakeLists.txt | 2 +- source/blender/bmesh/operators/bmo_rotate_edge.c | 272 ---------------------- source/blender/bmesh/operators/bmo_rotate_edges.c | 272 ++++++++++++++++++++++ 3 files changed, 273 insertions(+), 273 deletions(-) delete mode 100644 source/blender/bmesh/operators/bmo_rotate_edge.c create mode 100644 source/blender/bmesh/operators/bmo_rotate_edges.c (limited to 'source') diff --git a/source/blender/bmesh/CMakeLists.txt b/source/blender/bmesh/CMakeLists.txt index 38a275483dd..5245d24a075 100644 --- a/source/blender/bmesh/CMakeLists.txt +++ b/source/blender/bmesh/CMakeLists.txt @@ -69,7 +69,7 @@ set(SRC operators/bmo_poke.c operators/bmo_primitive.c operators/bmo_removedoubles.c - operators/bmo_rotate_edge.c + operators/bmo_rotate_edges.c operators/bmo_similar.c operators/bmo_smooth_laplacian.c operators/bmo_split_edges.c diff --git a/source/blender/bmesh/operators/bmo_rotate_edge.c b/source/blender/bmesh/operators/bmo_rotate_edge.c deleted file mode 100644 index dd6bf77dd3c..00000000000 --- a/source/blender/bmesh/operators/bmo_rotate_edge.c +++ /dev/null @@ -1,272 +0,0 @@ -/* - * ***** BEGIN GPL LICENSE BLOCK ***** - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation; either version 2 - * of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - * - * ***** END GPL LICENSE BLOCK ***** - */ - -/** \file blender/bmesh/operators/bmo_rotate_edge.c - * \ingroup bmesh - * - * Rotate edges topology that share two faces. - */ - -#include "MEM_guardedalloc.h" - -#include "BLI_math.h" -#include "BLI_heap.h" - -#include "bmesh.h" - -#include "intern/bmesh_operators_private.h" /* own include */ - -#define EDGE_OUT 1 -#define FACE_MARK 1 - -/** - * Rotate edges where every edge has it's own faces (we can rotate in any order). - */ -static void bm_rotate_edges_simple( - BMesh *bm, BMOperator *op, - const short check_flag, const bool use_ccw) -{ - BMOIter siter; - BMEdge *e; - - BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { - /** - * this ends up being called twice, could add option to not to call check in - * #BM_edge_rotate to get some extra speed */ - if (BM_edge_rotate_check(e)) { - BMEdge *e_rotate = BM_edge_rotate(bm, e, use_ccw, check_flag); - if (e_rotate != NULL) { - BMO_edge_flag_enable(bm, e_rotate, EDGE_OUT); - } - } - } -} - - -/** - * Edge length is just a way of ordering that's independent of order in the edges argument, - * we could use some other method since ideally all edges will be rotated, - * this just happens to be simple to calculate. - */ -static float bm_edge_calc_rotate_cost(const BMEdge *e) -{ - return -BM_edge_calc_length_squared(e); -} - -/** - * Check if this edge is a boundary: Are more than one of the connected faces edges rotating too? - */ -static float bm_edge_rotate_is_boundary(const BMEdge *e) -{ - /* Number of adjacent shared faces. */ - int count = 0; - BMLoop *l_radial_iter = e->l; - do { - /* Skip this edge. */ - BMLoop *l_iter = l_radial_iter->next; - do { - BMEdge *e_iter = l_iter->e; - const int e_iter_index = BM_elem_index_get(e_iter); - if ((e_iter_index != -1)) { - if (count == 1) { - return false; - } - count += 1; - break; - } - } while ((l_iter = l_iter->next) != l_radial_iter); - } while ((l_radial_iter = l_radial_iter->radial_next) != e->l); - return true; -} - -/** - * Rotate edges where edges share faces, - * edges which could not rotate need to be re-considered after neighbors are rotated. - */ -static void bm_rotate_edges_shared( - BMesh *bm, BMOperator *op, - short check_flag, const bool use_ccw, const int edges_len) -{ - Heap *heap = BLI_heap_new_ex(edges_len); - HeapNode **eheap_table = MEM_mallocN(sizeof(*eheap_table) * edges_len, __func__); - int edges_len_rotate = 0; - - { - BMIter iter; - BMEdge *e; - BM_ITER_MESH (e, &iter, bm, BM_EDGES_OF_MESH) { - BM_elem_index_set(e, -1); /* set_dirty! */ - } - bm->elem_index_dirty |= BM_EDGE; - } - - { - BMOIter siter; - BMEdge *e; - uint i; - BMO_ITER_INDEX (e, &siter, op->slots_in, "edges", BM_EDGE, i) { - BM_elem_index_set(e, BM_edge_is_manifold(e) ? i : -1); /* set_dirty! */ - eheap_table[i] = NULL; - } - } - - /* First operate on boundary edges, this is often all that's needed, - * regions that have no boundaries are handles after. */ - enum { - PASS_TYPE_BOUNDARY = 0, - PASS_TYPE_ALL = 1, - PASS_TYPE_DONE = 2, - }; - uint pass_type = PASS_TYPE_BOUNDARY; - - - while ((pass_type != PASS_TYPE_DONE) && (edges_len_rotate != edges_len)) { - BLI_assert(BLI_heap_is_empty(heap)); - { - BMOIter siter; - BMEdge *e; - uint i; - BMO_ITER_INDEX (e, &siter, op->slots_in, "edges", BM_EDGE, i) { - BLI_assert(eheap_table[i] == NULL); - - bool ok = (BM_elem_index_get(e) != -1) && BM_edge_rotate_check(e); - - if (ok) { - if (pass_type == PASS_TYPE_BOUNDARY) { - ok = bm_edge_rotate_is_boundary(e); - } - } - - if (ok) { - float cost = bm_edge_calc_rotate_cost(e); - if (pass_type == PASS_TYPE_BOUNDARY) { - /* Trick to ensure once started, non boundaries are handled before other boundary edges. - * This means the first longest boundary defines the starting point which is rotated - * until all its connected edges are exhausted and the next boundary is popped off the heap. - * - * Without this we may rotate from different starting points and meet in the middle - * with obviously uneven topology. - * - * Move from negative to positive value, inverting so large values are still handled first. - */ - cost = cost != 0.0f ? -1.0f / cost : FLT_MAX; - } - eheap_table[i] = BLI_heap_insert(heap, cost, e); - } - } - } - - if (BLI_heap_is_empty(heap)) { - pass_type += 1; - continue; - } - - const int edges_len_rotate_prev = edges_len_rotate; - while (!BLI_heap_is_empty(heap)) { - BMEdge *e_best = BLI_heap_popmin(heap); - eheap_table[BM_elem_index_get(e_best)] = NULL; - - /* No problem if this fails, re-evaluate if faces connected to this edge are touched. */ - if (BM_edge_rotate_check(e_best)) { - BMEdge *e_rotate = BM_edge_rotate(bm, e_best, use_ccw, check_flag); - if (e_rotate != NULL) { - BMO_edge_flag_enable(bm, e_rotate, EDGE_OUT); - - /* invalidate so we don't try touch this again. */ - BM_elem_index_set(e_rotate, -1); /* set_dirty! */ - - edges_len_rotate += 1; - - /* Note: we could validate all edges which have not been rotated - * (not just previously degenerate edges). - * However there is no real need - they can be left until they're popped off the queue. */ - - /* We don't know the exact topology after rotating the edge, - * so loop over all faces attached to the new edge, typically this will only be two faces. */ - BMLoop *l_radial_iter = e_rotate->l; - do { - /* Skip this edge. */ - BMLoop *l_iter = l_radial_iter->next; - do { - BMEdge *e_iter = l_iter->e; - const int e_iter_index = BM_elem_index_get(e_iter); - if ((e_iter_index != -1) && (eheap_table[e_iter_index] == NULL)) { - if (BM_edge_rotate_check(e_iter)) { - /* Previously degenerate, now valid. */ - float cost = bm_edge_calc_rotate_cost(e_iter); - eheap_table[e_iter_index] = BLI_heap_insert(heap, cost, e_iter); - } - } - } while ((l_iter = l_iter->next) != l_radial_iter); - } while ((l_radial_iter = l_radial_iter->radial_next) != e_rotate->l); - } - } - } - - /* If no actions were taken, move onto the next pass. */ - if (edges_len_rotate == edges_len_rotate_prev) { - pass_type += 1; - continue; - } - } - - BLI_heap_free(heap, NULL); - MEM_freeN(eheap_table); -} - -void bmo_rotate_edges_exec(BMesh *bm, BMOperator *op) -{ - BMOIter siter; - BMEdge *e; - const int edges_len = BMO_slot_buffer_count(op->slots_in, "edges"); - const bool use_ccw = BMO_slot_bool_get(op->slots_in, "use_ccw"); - const bool is_single = (edges_len == 1); - short check_flag = is_single ? - BM_EDGEROT_CHECK_EXISTS : - BM_EDGEROT_CHECK_EXISTS | BM_EDGEROT_CHECK_DEGENERATE; - - bool is_simple = true; - if (is_single == false) { - BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { - BMFace *f_pair[2]; - if (BM_edge_face_pair(e, &f_pair[0], &f_pair[1])) { - for (uint i = 0; i < ARRAY_SIZE(f_pair); i += 1) { - if (BMO_face_flag_test(bm, f_pair[i], FACE_MARK)) { - is_simple = false; - break; - } - BMO_face_flag_enable(bm, f_pair[i], FACE_MARK); - } - if (is_simple == false) { - break; - } - } - } - } - - if (is_simple) { - bm_rotate_edges_simple(bm, op, use_ccw, check_flag); - } - else { - bm_rotate_edges_shared(bm, op, check_flag, use_ccw, edges_len); - } - - BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "edges.out", BM_EDGE, EDGE_OUT); -} diff --git a/source/blender/bmesh/operators/bmo_rotate_edges.c b/source/blender/bmesh/operators/bmo_rotate_edges.c new file mode 100644 index 00000000000..dd6bf77dd3c --- /dev/null +++ b/source/blender/bmesh/operators/bmo_rotate_edges.c @@ -0,0 +1,272 @@ +/* + * ***** BEGIN GPL LICENSE BLOCK ***** + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * ***** END GPL LICENSE BLOCK ***** + */ + +/** \file blender/bmesh/operators/bmo_rotate_edge.c + * \ingroup bmesh + * + * Rotate edges topology that share two faces. + */ + +#include "MEM_guardedalloc.h" + +#include "BLI_math.h" +#include "BLI_heap.h" + +#include "bmesh.h" + +#include "intern/bmesh_operators_private.h" /* own include */ + +#define EDGE_OUT 1 +#define FACE_MARK 1 + +/** + * Rotate edges where every edge has it's own faces (we can rotate in any order). + */ +static void bm_rotate_edges_simple( + BMesh *bm, BMOperator *op, + const short check_flag, const bool use_ccw) +{ + BMOIter siter; + BMEdge *e; + + BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { + /** + * this ends up being called twice, could add option to not to call check in + * #BM_edge_rotate to get some extra speed */ + if (BM_edge_rotate_check(e)) { + BMEdge *e_rotate = BM_edge_rotate(bm, e, use_ccw, check_flag); + if (e_rotate != NULL) { + BMO_edge_flag_enable(bm, e_rotate, EDGE_OUT); + } + } + } +} + + +/** + * Edge length is just a way of ordering that's independent of order in the edges argument, + * we could use some other method since ideally all edges will be rotated, + * this just happens to be simple to calculate. + */ +static float bm_edge_calc_rotate_cost(const BMEdge *e) +{ + return -BM_edge_calc_length_squared(e); +} + +/** + * Check if this edge is a boundary: Are more than one of the connected faces edges rotating too? + */ +static float bm_edge_rotate_is_boundary(const BMEdge *e) +{ + /* Number of adjacent shared faces. */ + int count = 0; + BMLoop *l_radial_iter = e->l; + do { + /* Skip this edge. */ + BMLoop *l_iter = l_radial_iter->next; + do { + BMEdge *e_iter = l_iter->e; + const int e_iter_index = BM_elem_index_get(e_iter); + if ((e_iter_index != -1)) { + if (count == 1) { + return false; + } + count += 1; + break; + } + } while ((l_iter = l_iter->next) != l_radial_iter); + } while ((l_radial_iter = l_radial_iter->radial_next) != e->l); + return true; +} + +/** + * Rotate edges where edges share faces, + * edges which could not rotate need to be re-considered after neighbors are rotated. + */ +static void bm_rotate_edges_shared( + BMesh *bm, BMOperator *op, + short check_flag, const bool use_ccw, const int edges_len) +{ + Heap *heap = BLI_heap_new_ex(edges_len); + HeapNode **eheap_table = MEM_mallocN(sizeof(*eheap_table) * edges_len, __func__); + int edges_len_rotate = 0; + + { + BMIter iter; + BMEdge *e; + BM_ITER_MESH (e, &iter, bm, BM_EDGES_OF_MESH) { + BM_elem_index_set(e, -1); /* set_dirty! */ + } + bm->elem_index_dirty |= BM_EDGE; + } + + { + BMOIter siter; + BMEdge *e; + uint i; + BMO_ITER_INDEX (e, &siter, op->slots_in, "edges", BM_EDGE, i) { + BM_elem_index_set(e, BM_edge_is_manifold(e) ? i : -1); /* set_dirty! */ + eheap_table[i] = NULL; + } + } + + /* First operate on boundary edges, this is often all that's needed, + * regions that have no boundaries are handles after. */ + enum { + PASS_TYPE_BOUNDARY = 0, + PASS_TYPE_ALL = 1, + PASS_TYPE_DONE = 2, + }; + uint pass_type = PASS_TYPE_BOUNDARY; + + + while ((pass_type != PASS_TYPE_DONE) && (edges_len_rotate != edges_len)) { + BLI_assert(BLI_heap_is_empty(heap)); + { + BMOIter siter; + BMEdge *e; + uint i; + BMO_ITER_INDEX (e, &siter, op->slots_in, "edges", BM_EDGE, i) { + BLI_assert(eheap_table[i] == NULL); + + bool ok = (BM_elem_index_get(e) != -1) && BM_edge_rotate_check(e); + + if (ok) { + if (pass_type == PASS_TYPE_BOUNDARY) { + ok = bm_edge_rotate_is_boundary(e); + } + } + + if (ok) { + float cost = bm_edge_calc_rotate_cost(e); + if (pass_type == PASS_TYPE_BOUNDARY) { + /* Trick to ensure once started, non boundaries are handled before other boundary edges. + * This means the first longest boundary defines the starting point which is rotated + * until all its connected edges are exhausted and the next boundary is popped off the heap. + * + * Without this we may rotate from different starting points and meet in the middle + * with obviously uneven topology. + * + * Move from negative to positive value, inverting so large values are still handled first. + */ + cost = cost != 0.0f ? -1.0f / cost : FLT_MAX; + } + eheap_table[i] = BLI_heap_insert(heap, cost, e); + } + } + } + + if (BLI_heap_is_empty(heap)) { + pass_type += 1; + continue; + } + + const int edges_len_rotate_prev = edges_len_rotate; + while (!BLI_heap_is_empty(heap)) { + BMEdge *e_best = BLI_heap_popmin(heap); + eheap_table[BM_elem_index_get(e_best)] = NULL; + + /* No problem if this fails, re-evaluate if faces connected to this edge are touched. */ + if (BM_edge_rotate_check(e_best)) { + BMEdge *e_rotate = BM_edge_rotate(bm, e_best, use_ccw, check_flag); + if (e_rotate != NULL) { + BMO_edge_flag_enable(bm, e_rotate, EDGE_OUT); + + /* invalidate so we don't try touch this again. */ + BM_elem_index_set(e_rotate, -1); /* set_dirty! */ + + edges_len_rotate += 1; + + /* Note: we could validate all edges which have not been rotated + * (not just previously degenerate edges). + * However there is no real need - they can be left until they're popped off the queue. */ + + /* We don't know the exact topology after rotating the edge, + * so loop over all faces attached to the new edge, typically this will only be two faces. */ + BMLoop *l_radial_iter = e_rotate->l; + do { + /* Skip this edge. */ + BMLoop *l_iter = l_radial_iter->next; + do { + BMEdge *e_iter = l_iter->e; + const int e_iter_index = BM_elem_index_get(e_iter); + if ((e_iter_index != -1) && (eheap_table[e_iter_index] == NULL)) { + if (BM_edge_rotate_check(e_iter)) { + /* Previously degenerate, now valid. */ + float cost = bm_edge_calc_rotate_cost(e_iter); + eheap_table[e_iter_index] = BLI_heap_insert(heap, cost, e_iter); + } + } + } while ((l_iter = l_iter->next) != l_radial_iter); + } while ((l_radial_iter = l_radial_iter->radial_next) != e_rotate->l); + } + } + } + + /* If no actions were taken, move onto the next pass. */ + if (edges_len_rotate == edges_len_rotate_prev) { + pass_type += 1; + continue; + } + } + + BLI_heap_free(heap, NULL); + MEM_freeN(eheap_table); +} + +void bmo_rotate_edges_exec(BMesh *bm, BMOperator *op) +{ + BMOIter siter; + BMEdge *e; + const int edges_len = BMO_slot_buffer_count(op->slots_in, "edges"); + const bool use_ccw = BMO_slot_bool_get(op->slots_in, "use_ccw"); + const bool is_single = (edges_len == 1); + short check_flag = is_single ? + BM_EDGEROT_CHECK_EXISTS : + BM_EDGEROT_CHECK_EXISTS | BM_EDGEROT_CHECK_DEGENERATE; + + bool is_simple = true; + if (is_single == false) { + BMO_ITER (e, &siter, op->slots_in, "edges", BM_EDGE) { + BMFace *f_pair[2]; + if (BM_edge_face_pair(e, &f_pair[0], &f_pair[1])) { + for (uint i = 0; i < ARRAY_SIZE(f_pair); i += 1) { + if (BMO_face_flag_test(bm, f_pair[i], FACE_MARK)) { + is_simple = false; + break; + } + BMO_face_flag_enable(bm, f_pair[i], FACE_MARK); + } + if (is_simple == false) { + break; + } + } + } + } + + if (is_simple) { + bm_rotate_edges_simple(bm, op, use_ccw, check_flag); + } + else { + bm_rotate_edges_shared(bm, op, check_flag, use_ccw, edges_len); + } + + BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "edges.out", BM_EDGE, EDGE_OUT); +} -- cgit v1.2.3 From 099bda8875e7b814e5c677e423b3c6640551267c Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sun, 26 Nov 2017 11:36:50 +0100 Subject: Removing OMP: nuke last usages in bmesh_mesh.c Those three ones were actually giving no significant benefits, in fact even slowing things down in one case compared to no parallelization at all (in `BM_mesh_elem_table_ensure()`). Point being, once more, parallelizing *very* small tasks (like index or flag setting, etc.) is nearly never worth it. Also note that we could not easlily use per-item parallel looping in those three cases, since they are heavily relying on valid loop-generated index (or are doing non-threadable things like allocation from a mempool)... --- source/blender/bmesh/intern/bmesh_mesh.c | 197 ++++++++++++------------------- 1 file changed, 78 insertions(+), 119 deletions(-) (limited to 'source') diff --git a/source/blender/bmesh/intern/bmesh_mesh.c b/source/blender/bmesh/intern/bmesh_mesh.c index 0f97321d9b9..67db51446df 100644 --- a/source/blender/bmesh/intern/bmesh_mesh.c +++ b/source/blender/bmesh/intern/bmesh_mesh.c @@ -115,37 +115,24 @@ void BM_mesh_elem_toolflags_ensure(BMesh *bm) bm->etoolflagpool = BLI_mempool_create(sizeof(BMFlagLayer), bm->totedge, 512, BLI_MEMPOOL_NOP); bm->ftoolflagpool = BLI_mempool_create(sizeof(BMFlagLayer), bm->totface, 512, BLI_MEMPOOL_NOP); -#pragma omp parallel sections if (bm->totvert + bm->totedge + bm->totface >= BM_OMP_LIMIT) - { -#pragma omp section - { - BLI_mempool *toolflagpool = bm->vtoolflagpool; - BMIter iter; - BMVert_OFlag *ele; - BM_ITER_MESH (ele, &iter, bm, BM_VERTS_OF_MESH) { - ele->oflags = BLI_mempool_calloc(toolflagpool); - } - } -#pragma omp section - { - BLI_mempool *toolflagpool = bm->etoolflagpool; - BMIter iter; - BMEdge_OFlag *ele; - BM_ITER_MESH (ele, &iter, bm, BM_EDGES_OF_MESH) { - ele->oflags = BLI_mempool_calloc(toolflagpool); - } - } -#pragma omp section - { - BLI_mempool *toolflagpool = bm->ftoolflagpool; - BMIter iter; - BMFace_OFlag *ele; - BM_ITER_MESH (ele, &iter, bm, BM_FACES_OF_MESH) { - ele->oflags = BLI_mempool_calloc(toolflagpool); - } - } + BMIter iter; + BMVert_OFlag *v_olfag; + BLI_mempool *toolflagpool = bm->vtoolflagpool; + BM_ITER_MESH (v_olfag, &iter, bm, BM_VERTS_OF_MESH) { + v_olfag->oflags = BLI_mempool_calloc(toolflagpool); } + BMEdge_OFlag *e_olfag; + toolflagpool = bm->etoolflagpool; + BM_ITER_MESH (e_olfag, &iter, bm, BM_EDGES_OF_MESH) { + e_olfag->oflags = BLI_mempool_calloc(toolflagpool); + } + + BMFace_OFlag *f_olfag; + toolflagpool = bm->ftoolflagpool; + BM_ITER_MESH (f_olfag, &iter, bm, BM_FACES_OF_MESH) { + f_olfag->oflags = BLI_mempool_calloc(toolflagpool); + } bm->totflags = 1; } @@ -1158,94 +1145,78 @@ void BM_mesh_elem_index_ensure(BMesh *bm, const char htype) BM_ELEM_INDEX_VALIDATE(bm, "Should Never Fail!", __func__); #endif - if (htype_needed == 0) { + if (0 && htype_needed == 0) { goto finally; } - /* skip if we only need to operate on one element */ -#pragma omp parallel sections if ((!ELEM(htype_needed, BM_VERT, BM_EDGE, BM_FACE, BM_LOOP, BM_FACE | BM_LOOP)) && \ - (bm->totvert + bm->totedge + bm->totface >= BM_OMP_LIMIT)) - { -#pragma omp section + if (htype & BM_VERT) { + if (bm->elem_index_dirty & BM_VERT) { + BMIter iter; + BMElem *ele; - { - if (htype & BM_VERT) { - if (bm->elem_index_dirty & BM_VERT) { - BMIter iter; - BMElem *ele; - - int index; - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_VERTS_OF_MESH, index) { - BM_elem_index_set(ele, index); /* set_ok */ - } - BLI_assert(index == bm->totvert); - } - else { - // printf("%s: skipping vert index calc!\n", __func__); - } + int index; + BM_ITER_MESH_INDEX (ele, &iter, bm, BM_VERTS_OF_MESH, index) { + BM_elem_index_set(ele, index); /* set_ok */ } + BLI_assert(index == bm->totvert); + } + else { + // printf("%s: skipping vert index calc!\n", __func__); } + } -#pragma omp section - { - if (htype & BM_EDGE) { - if (bm->elem_index_dirty & BM_EDGE) { - BMIter iter; - BMElem *ele; - - int index; - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_EDGES_OF_MESH, index) { - BM_elem_index_set(ele, index); /* set_ok */ - } - BLI_assert(index == bm->totedge); - } - else { - // printf("%s: skipping edge index calc!\n", __func__); - } + if (htype & BM_EDGE) { + if (bm->elem_index_dirty & BM_EDGE) { + BMIter iter; + BMElem *ele; + + int index; + BM_ITER_MESH_INDEX (ele, &iter, bm, BM_EDGES_OF_MESH, index) { + BM_elem_index_set(ele, index); /* set_ok */ } + BLI_assert(index == bm->totedge); } + else { + // printf("%s: skipping edge index calc!\n", __func__); + } + } -#pragma omp section - { - if (htype & (BM_FACE | BM_LOOP)) { - if (bm->elem_index_dirty & (BM_FACE | BM_LOOP)) { - BMIter iter; - BMElem *ele; + if (htype & (BM_FACE | BM_LOOP)) { + if (bm->elem_index_dirty & (BM_FACE | BM_LOOP)) { + BMIter iter; + BMElem *ele; - const bool update_face = (htype & BM_FACE) && (bm->elem_index_dirty & BM_FACE); - const bool update_loop = (htype & BM_LOOP) && (bm->elem_index_dirty & BM_LOOP); + const bool update_face = (htype & BM_FACE) && (bm->elem_index_dirty & BM_FACE); + const bool update_loop = (htype & BM_LOOP) && (bm->elem_index_dirty & BM_LOOP); - int index; - int index_loop = 0; + int index; + int index_loop = 0; - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_FACES_OF_MESH, index) { - if (update_face) { - BM_elem_index_set(ele, index); /* set_ok */ - } + BM_ITER_MESH_INDEX (ele, &iter, bm, BM_FACES_OF_MESH, index) { + if (update_face) { + BM_elem_index_set(ele, index); /* set_ok */ + } - if (update_loop) { - BMLoop *l_iter, *l_first; + if (update_loop) { + BMLoop *l_iter, *l_first; - l_iter = l_first = BM_FACE_FIRST_LOOP((BMFace *)ele); - do { - BM_elem_index_set(l_iter, index_loop++); /* set_ok */ - } while ((l_iter = l_iter->next) != l_first); - } - } - - BLI_assert(index == bm->totface); - if (update_loop) { - BLI_assert(index_loop == bm->totloop); - } - } - else { - // printf("%s: skipping face/loop index calc!\n", __func__); + l_iter = l_first = BM_FACE_FIRST_LOOP((BMFace *)ele); + do { + BM_elem_index_set(l_iter, index_loop++); /* set_ok */ + } while ((l_iter = l_iter->next) != l_first); } } + + BLI_assert(index == bm->totface); + if (update_loop) { + BLI_assert(index_loop == bm->totloop); + } + } + else { + // printf("%s: skipping face/loop index calc!\n", __func__); } } - finally: bm->elem_index_dirty &= ~htype; } @@ -1417,28 +1388,16 @@ void BM_mesh_elem_table_ensure(BMesh *bm, const char htype) } } - /* skip if we only need to operate on one element */ -#pragma omp parallel sections if ((!ELEM(htype_needed, BM_VERT, BM_EDGE, BM_FACE)) && \ - (bm->totvert + bm->totedge + bm->totface >= BM_OMP_LIMIT)) - { -#pragma omp section - { - if (htype_needed & BM_VERT) { - BM_iter_as_array(bm, BM_VERTS_OF_MESH, NULL, (void **)bm->vtable, bm->totvert); - } - } -#pragma omp section - { - if (htype_needed & BM_EDGE) { - BM_iter_as_array(bm, BM_EDGES_OF_MESH, NULL, (void **)bm->etable, bm->totedge); - } - } -#pragma omp section - { - if (htype_needed & BM_FACE) { - BM_iter_as_array(bm, BM_FACES_OF_MESH, NULL, (void **)bm->ftable, bm->totface); - } - } + if (htype_needed & BM_VERT) { + BM_iter_as_array(bm, BM_VERTS_OF_MESH, NULL, (void **)bm->vtable, bm->totvert); + } + + if (htype_needed & BM_EDGE) { + BM_iter_as_array(bm, BM_EDGES_OF_MESH, NULL, (void **)bm->etable, bm->totedge); + } + + if (htype_needed & BM_FACE) { + BM_iter_as_array(bm, BM_FACES_OF_MESH, NULL, (void **)bm->ftable, bm->totface); } finally: -- cgit v1.2.3 From 2b6f3455584b3949045b517ce4759bcda70daf8a Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sun, 26 Nov 2017 13:06:39 +0100 Subject: Removing OMP: bmesh_interp.c Performances tests on this one are quite surprising actually... Parallelized loop itself is at least 10 times quicker with new BLI_task code than it was with OMP. And subdividing e.g. a heavy mesh with 3 levels of multires (whole process) takes 8 seconds with new code, while 10 seconds with OMP one. And cherry on top, BLI_task code only uses about 50% of CPU load, while OMP one was at nearly 100%! In fact, I suspect OMP code was not properly declaring outside vars, generating a lot of uneeded locks. Also, raised the minimum level of subdiv to enable parallelization, tests here showed that we only start to get significant gain with subdiv levels of 4, below single threaded one is quicker. --- source/blender/bmesh/intern/bmesh_interp.c | 115 ++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 36 deletions(-) (limited to 'source') diff --git a/source/blender/bmesh/intern/bmesh_interp.c b/source/blender/bmesh/intern/bmesh_interp.c index 00f8eb6df40..96b2eb17c4c 100644 --- a/source/blender/bmesh/intern/bmesh_interp.c +++ b/source/blender/bmesh/intern/bmesh_interp.c @@ -36,12 +36,13 @@ #include "DNA_meshdata_types.h" #include "BLI_alloca.h" +#include "BLI_linklist.h" #include "BLI_math.h" +#include "BLI_memarena.h" +#include "BLI_task.h" #include "BKE_customdata.h" #include "BKE_multires.h" -#include "BLI_memarena.h" -#include "BLI_linklist.h" #include "bmesh.h" #include "intern/bmesh_private.h" @@ -401,13 +402,78 @@ static void bm_loop_flip_disp( disp[1] = (mat[0][0] * b[1] - b[0] * mat[1][0]) / d; } + +typedef struct BMLoopInterpMultiresData { + BMLoop *l_dst; + BMLoop *l_src_first; + int cd_loop_mdisp_offset; + + MDisps *md_dst; + const float *f_src_center; + + float *axis_x, *axis_y; + float *v1, *v4; + float *e1, *e2; + + int res; + float d; +} BMLoopInterpMultiresData; + +static void loop_interp_multires_cb(void *userdata, int ix) +{ + BMLoopInterpMultiresData *data = userdata; + + BMLoop *l_first = data->l_src_first; + BMLoop *l_dst = data->l_dst; + const int cd_loop_mdisp_offset = data->cd_loop_mdisp_offset; + + MDisps *md_dst = data->md_dst; + const float *f_src_center = data->f_src_center; + + float *axis_x = data->axis_x; + float *axis_y = data->axis_y; + + float *v1 = data->v1; + float *v4 = data->v4; + float *e1 = data->e1; + float *e2 = data->e2; + + const int res = data->res; + const float d = data->d; + + float x = d * ix, y; + int iy; + for (y = 0.0f, iy = 0; iy < res; y += d, iy++) { + BMLoop *l_iter = l_first; + float co1[3], co2[3], co[3]; + + madd_v3_v3v3fl(co1, v1, e1, y); + madd_v3_v3v3fl(co2, v4, e2, y); + interp_v3_v3v3(co, co1, co2, x); + + do { + MDisps *md_src; + float src_axis_x[3], src_axis_y[3]; + float uv[2]; + + md_src = BM_ELEM_CD_GET_VOID_P(l_iter, cd_loop_mdisp_offset); + + if (mdisp_in_mdispquad(l_dst, l_iter, f_src_center, co, res, src_axis_x, src_axis_y, uv)) { + old_mdisps_bilinear(md_dst->disps[iy * res + ix], md_src->disps, res, uv[0], uv[1]); + bm_loop_flip_disp(src_axis_x, src_axis_y, axis_x, axis_y, md_dst->disps[iy * res + ix]); + + break; + } + } while ((l_iter = l_iter->next) != l_first); + } +} + void BM_loop_interp_multires_ex( BMesh *UNUSED(bm), BMLoop *l_dst, const BMFace *f_src, const float f_dst_center[3], const float f_src_center[3], const int cd_loop_mdisp_offset) { MDisps *md_dst; - float d, v1[3], v2[3], v3[3], v4[3] = {0.0f, 0.0f, 0.0f}, e1[3], e2[3]; - int ix, res; + float v1[3], v2[3], v3[3], v4[3] = {0.0f, 0.0f, 0.0f}, e1[3], e2[3]; float axis_x[3], axis_y[3]; /* ignore 2-edged faces */ @@ -433,38 +499,15 @@ void BM_loop_interp_multires_ex( mdisp_axis_from_quad(v1, v2, v3, v4, axis_x, axis_y); - res = (int)sqrt(md_dst->totdisp); - d = 1.0f / (float)(res - 1); -#pragma omp parallel for if (res > 3) - for (ix = 0; ix < res; ix++) { - float x = d * ix, y; - int iy; - for (y = 0.0f, iy = 0; iy < res; y += d, iy++) { - BMLoop *l_iter; - BMLoop *l_first; - float co1[3], co2[3], co[3]; - - madd_v3_v3v3fl(co1, v1, e1, y); - madd_v3_v3v3fl(co2, v4, e2, y); - interp_v3_v3v3(co, co1, co2, x); - - l_iter = l_first = BM_FACE_FIRST_LOOP(f_src); - do { - MDisps *md_src; - float src_axis_x[3], src_axis_y[3]; - float uv[2]; - - md_src = BM_ELEM_CD_GET_VOID_P(l_iter, cd_loop_mdisp_offset); - - if (mdisp_in_mdispquad(l_dst, l_iter, f_src_center, co, res, src_axis_x, src_axis_y, uv)) { - old_mdisps_bilinear(md_dst->disps[iy * res + ix], md_src->disps, res, uv[0], uv[1]); - bm_loop_flip_disp(src_axis_x, src_axis_y, axis_x, axis_y, md_dst->disps[iy * res + ix]); - - break; - } - } while ((l_iter = l_iter->next) != l_first); - } - } + const int res = (int)sqrt(md_dst->totdisp); + BMLoopInterpMultiresData data = { + .l_dst = l_dst, .l_src_first = BM_FACE_FIRST_LOOP(f_src), + .cd_loop_mdisp_offset = cd_loop_mdisp_offset, + .md_dst = md_dst, .f_src_center = f_src_center, + .axis_x = axis_x, .axis_y = axis_y, .v1 = v1, .v4 = v4, .e1 = e1, .e2 = e2, + .res = res, .d = 1.0f / (float)(res - 1) + }; + BLI_task_parallel_range(0, res, &data, loop_interp_multires_cb, res > 5); } /** -- cgit v1.2.3 From f1ce2799039fd554f6ef2b54a9f0fddd44444a85 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sun, 26 Nov 2017 15:51:50 +0100 Subject: Removing OMP: bmesh_operators.c Two more 'not really useful' cases (OMP only shows some noticeable speedup with above 1M elements, and since this is quick operation anyway compared to even ather basic operators, gain is in the 1% area of total processing time in best case). So not worth parallelizing here, we'll gain much more on tackling heavy operations. ;) And BMesh is free from OMP now! --- source/blender/bmesh/intern/bmesh_operators.c | 166 ++++++++++---------------- 1 file changed, 64 insertions(+), 102 deletions(-) (limited to 'source') diff --git a/source/blender/bmesh/intern/bmesh_operators.c b/source/blender/bmesh/intern/bmesh_operators.c index 3e12a43e457..0f672a9ff2a 100644 --- a/source/blender/bmesh/intern/bmesh_operators.c +++ b/source/blender/bmesh/intern/bmesh_operators.c @@ -1221,57 +1221,38 @@ static void bmo_flag_layer_alloc(BMesh *bm) bm->etoolflagpool = BLI_mempool_create(sizeof(BMFlagLayer) * bm->totflags, bm->totedge, 512, BLI_MEMPOOL_NOP); bm->ftoolflagpool = BLI_mempool_create(sizeof(BMFlagLayer) * bm->totflags, bm->totface, 512, BLI_MEMPOOL_NOP); -#pragma omp parallel sections if (bm->totvert + bm->totedge + bm->totface >= BM_OMP_LIMIT) - { -#pragma omp section - { - BMIter iter; - BMVert_OFlag *ele; - int i; - - BLI_mempool *newpool = bm->vtoolflagpool; - - /* now go through and memcpy all the flags. Loops don't get a flag layer at this time.. */ - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_VERTS_OF_MESH, i) { - void *oldflags = ele->oflags; - ele->oflags = BLI_mempool_calloc(newpool); - memcpy(ele->oflags, oldflags, old_totflags_size); - BM_elem_index_set(&ele->base, i); /* set_inline */ - BM_ELEM_API_FLAG_CLEAR((BMElemF *)ele); - } - } -#pragma omp section - { - BMIter iter; - BMEdge_OFlag *ele; - int i; - - BLI_mempool *newpool = bm->etoolflagpool; - - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_EDGES_OF_MESH, i) { - void *oldflags = ele->oflags; - ele->oflags = BLI_mempool_calloc(newpool); - memcpy(ele->oflags, oldflags, old_totflags_size); - BM_elem_index_set(&ele->base, i); /* set_inline */ - BM_ELEM_API_FLAG_CLEAR((BMElemF *)ele); - } - } -#pragma omp section - { - BMIter iter; - BMFace_OFlag *ele; - int i; - - BLI_mempool *newpool = bm->ftoolflagpool; - - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_FACES_OF_MESH, i) { - void *oldflags = ele->oflags; - ele->oflags = BLI_mempool_calloc(newpool); - memcpy(ele->oflags, oldflags, old_totflags_size); - BM_elem_index_set(&ele->base, i); /* set_inline */ - BM_ELEM_API_FLAG_CLEAR((BMElemF *)ele); - } - } + /* now go through and memcpy all the flags. Loops don't get a flag layer at this time.. */ + BMIter iter; + int i; + + BMVert_OFlag *v_oflag; + BLI_mempool *newpool = bm->vtoolflagpool; + BM_ITER_MESH_INDEX (v_oflag, &iter, bm, BM_VERTS_OF_MESH, i) { + void *oldflags = v_oflag->oflags; + v_oflag->oflags = BLI_mempool_calloc(newpool); + memcpy(v_oflag->oflags, oldflags, old_totflags_size); + BM_elem_index_set(&v_oflag->base, i); /* set_inline */ + BM_ELEM_API_FLAG_CLEAR((BMElemF *)v_oflag); + } + + BMEdge_OFlag *e_oflag; + newpool = bm->etoolflagpool; + BM_ITER_MESH_INDEX (e_oflag, &iter, bm, BM_EDGES_OF_MESH, i) { + void *oldflags = e_oflag->oflags; + e_oflag->oflags = BLI_mempool_calloc(newpool); + memcpy(e_oflag->oflags, oldflags, old_totflags_size); + BM_elem_index_set(&e_oflag->base, i); /* set_inline */ + BM_ELEM_API_FLAG_CLEAR((BMElemF *)e_oflag); + } + + BMFace_OFlag *f_oflag; + newpool = bm->ftoolflagpool; + BM_ITER_MESH_INDEX (f_oflag, &iter, bm, BM_FACES_OF_MESH, i) { + void *oldflags = f_oflag->oflags; + f_oflag->oflags = BLI_mempool_calloc(newpool); + memcpy(f_oflag->oflags, oldflags, old_totflags_size); + BM_elem_index_set(&f_oflag->base, i); /* set_inline */ + BM_ELEM_API_FLAG_CLEAR((BMElemF *)f_oflag); } BLI_mempool_destroy(voldpool); @@ -1300,57 +1281,38 @@ static void bmo_flag_layer_free(BMesh *bm) bm->etoolflagpool = BLI_mempool_create(new_totflags_size, bm->totedge, 512, BLI_MEMPOOL_NOP); bm->ftoolflagpool = BLI_mempool_create(new_totflags_size, bm->totface, 512, BLI_MEMPOOL_NOP); -#pragma omp parallel sections if (bm->totvert + bm->totedge + bm->totface >= BM_OMP_LIMIT) - { -#pragma omp section - { - BMIter iter; - BMVert_OFlag *ele; - int i; - - BLI_mempool *newpool = bm->vtoolflagpool; - - /* now go through and memcpy all the flag */ - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_VERTS_OF_MESH, i) { - void *oldflags = ele->oflags; - ele->oflags = BLI_mempool_alloc(newpool); - memcpy(ele->oflags, oldflags, new_totflags_size); - BM_elem_index_set(&ele->base, i); /* set_inline */ - BM_ELEM_API_FLAG_CLEAR((BMElemF *)ele); - } - } -#pragma omp section - { - BMIter iter; - BMEdge_OFlag *ele; - int i; - - BLI_mempool *newpool = bm->etoolflagpool; - - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_EDGES_OF_MESH, i) { - void *oldflags = ele->oflags; - ele->oflags = BLI_mempool_alloc(newpool); - memcpy(ele->oflags, oldflags, new_totflags_size); - BM_elem_index_set(&ele->base, i); /* set_inline */ - BM_ELEM_API_FLAG_CLEAR((BMElemF *)ele); - } - } -#pragma omp section - { - BMIter iter; - BMFace_OFlag *ele; - int i; - - BLI_mempool *newpool = bm->ftoolflagpool; - - BM_ITER_MESH_INDEX (ele, &iter, bm, BM_FACES_OF_MESH, i) { - void *oldflags = ele->oflags; - ele->oflags = BLI_mempool_alloc(newpool); - memcpy(ele->oflags, oldflags, new_totflags_size); - BM_elem_index_set(&ele->base, i); /* set_inline */ - BM_ELEM_API_FLAG_CLEAR((BMElemF *)ele); - } - } + /* now go through and memcpy all the flag */ + BMIter iter; + int i; + + BMVert_OFlag *v_oflag; + BLI_mempool *newpool = bm->vtoolflagpool; + BM_ITER_MESH_INDEX (v_oflag, &iter, bm, BM_VERTS_OF_MESH, i) { + void *oldflags = v_oflag->oflags; + v_oflag->oflags = BLI_mempool_alloc(newpool); + memcpy(v_oflag->oflags, oldflags, new_totflags_size); + BM_elem_index_set(&v_oflag->base, i); /* set_inline */ + BM_ELEM_API_FLAG_CLEAR((BMElemF *)v_oflag); + } + + BMEdge_OFlag *e_oflag; + newpool = bm->etoolflagpool; + BM_ITER_MESH_INDEX (e_oflag, &iter, bm, BM_EDGES_OF_MESH, i) { + void *oldflags = e_oflag->oflags; + e_oflag->oflags = BLI_mempool_alloc(newpool); + memcpy(e_oflag->oflags, oldflags, new_totflags_size); + BM_elem_index_set(&e_oflag->base, i); /* set_inline */ + BM_ELEM_API_FLAG_CLEAR((BMElemF *)e_oflag); + } + + BMFace_OFlag *f_oflag; + newpool = bm->ftoolflagpool; + BM_ITER_MESH_INDEX (f_oflag, &iter, bm, BM_FACES_OF_MESH, i) { + void *oldflags = f_oflag->oflags; + f_oflag->oflags = BLI_mempool_alloc(newpool); + memcpy(f_oflag->oflags, oldflags, new_totflags_size); + BM_elem_index_set(&f_oflag->base, i); /* set_inline */ + BM_ELEM_API_FLAG_CLEAR((BMElemF *)f_oflag); } BLI_mempool_destroy(voldpool); -- cgit v1.2.3 From 440a49a24cb82fc78edfb6b2539d9ebd0f805a4c Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sun, 26 Nov 2017 17:25:41 +0100 Subject: Removing OMP: autotrack BKE code. Pretty straightforward this time, we already have a single struct pointer containing all needed data (or nearly). And we gain about 10-15% speed on tracking! :) --- source/blender/blenkernel/intern/tracking_auto.c | 144 ++++++++++++----------- 1 file changed, 78 insertions(+), 66 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/tracking_auto.c b/source/blender/blenkernel/intern/tracking_auto.c index 30981ed8f23..50451b90581 100644 --- a/source/blender/blenkernel/intern/tracking_auto.c +++ b/source/blender/blenkernel/intern/tracking_auto.c @@ -30,6 +30,7 @@ */ #include +#include "atomic_ops.h" #include "MEM_guardedalloc.h" @@ -38,6 +39,7 @@ #include "BLI_utildefines.h" #include "BLI_listbase.h" +#include "BLI_task.h" #include "BLI_threads.h" #include "BLI_math.h" @@ -86,6 +88,8 @@ typedef struct AutoTrackContext { int sync_frame; bool first_sync; SpinLock spin_lock; + + bool step_ok; } AutoTrackContext; static void normalized_to_libmv_frame(const float normalized[2], @@ -379,81 +383,89 @@ AutoTrackContext *BKE_autotrack_context_new(MovieClip *clip, return context; } -bool BKE_autotrack_context_step(AutoTrackContext *context) +static void autotrack_context_step_cb(void *userdata, int track) { + AutoTrackContext *context = userdata; const int frame_delta = context->backwards ? -1 : 1; - bool ok = false; - int track; - -#pragma omp parallel for if (context->num_tracks > 1) - for (track = 0; track < context->num_tracks; ++track) { - AutoTrackOptions *options = &context->options[track]; - if (options->is_failed) { - continue; - } - libmv_Marker libmv_current_marker, - libmv_reference_marker, - libmv_tracked_marker; - libmv_TrackRegionResult libmv_result; - const int frame = BKE_movieclip_remap_scene_to_clip_frame( - context->clips[options->clip_index], - context->user.framenr); + + AutoTrackOptions *options = &context->options[track]; + if (options->is_failed) { + return; + } + libmv_Marker libmv_current_marker, + libmv_reference_marker, + libmv_tracked_marker; + libmv_TrackRegionResult libmv_result; + const int frame = BKE_movieclip_remap_scene_to_clip_frame( + context->clips[options->clip_index], + context->user.framenr); + BLI_spin_lock(&context->spin_lock); + const bool has_marker = libmv_autoTrackGetMarker(context->autotrack, + options->clip_index, + frame, + options->track_index, + &libmv_current_marker); + BLI_spin_unlock(&context->spin_lock); + /* Check whether we've got marker to sync with. */ + if (!has_marker) { + return; + } + /* Check whether marker is going outside of allowed frame margin. */ + if (!tracking_check_marker_margin(&libmv_current_marker, + options->track->margin, + context->frame_width, + context->frame_height)) + { + return; + } + libmv_tracked_marker = libmv_current_marker; + libmv_tracked_marker.frame = frame + frame_delta; + /* Update reference frame. */ + if (options->use_keyframe_match) { + libmv_tracked_marker.reference_frame = + libmv_current_marker.reference_frame; + libmv_autoTrackGetMarker(context->autotrack, + options->clip_index, + libmv_tracked_marker.reference_frame, + options->track_index, + &libmv_reference_marker); + } + else { + libmv_tracked_marker.reference_frame = frame; + libmv_reference_marker = libmv_current_marker; + } + /* Perform actual tracking. */ + if (libmv_autoTrackMarker(context->autotrack, + &options->track_region_options, + &libmv_tracked_marker, + &libmv_result)) + { BLI_spin_lock(&context->spin_lock); - const bool has_marker = libmv_autoTrackGetMarker(context->autotrack, - options->clip_index, - frame, - options->track_index, - &libmv_current_marker); + libmv_autoTrackAddMarker(context->autotrack, &libmv_tracked_marker); BLI_spin_unlock(&context->spin_lock); - /* Check whether we've got marker to sync with. */ - if (!has_marker) { - continue; - } - /* Check whether marker is going outside of allowed frame margin. */ - if (!tracking_check_marker_margin(&libmv_current_marker, - options->track->margin, - context->frame_width, - context->frame_height)) - { - continue; - } - libmv_tracked_marker = libmv_current_marker; - libmv_tracked_marker.frame = frame + frame_delta; - /* Update reference frame. */ - if (options->use_keyframe_match) { - libmv_tracked_marker.reference_frame = - libmv_current_marker.reference_frame; - libmv_autoTrackGetMarker(context->autotrack, - options->clip_index, - libmv_tracked_marker.reference_frame, - options->track_index, - &libmv_reference_marker); - } - else { - libmv_tracked_marker.reference_frame = frame; - libmv_reference_marker = libmv_current_marker; - } - /* Perform actual tracking. */ - if (libmv_autoTrackMarker(context->autotrack, - &options->track_region_options, - &libmv_tracked_marker, - &libmv_result)) - { - BLI_spin_lock(&context->spin_lock); - libmv_autoTrackAddMarker(context->autotrack, &libmv_tracked_marker); - BLI_spin_unlock(&context->spin_lock); - } - else { - options->is_failed = true; - options->failed_frame = frame + frame_delta; - } - ok = true; } + else { + options->is_failed = true; + options->failed_frame = frame + frame_delta; + } + + /* Note: Atomic is probably not actually needed here, I doubt we could get any other result than a true bool anyway. + * But for sake of consistency, and since it costs nothing... */ + atomic_fetch_and_or_uint8((uint8_t *)&context->step_ok, true); +} + +bool BKE_autotrack_context_step(AutoTrackContext *context) +{ + const int frame_delta = context->backwards ? -1 : 1; + context->step_ok = false; + + BLI_task_parallel_range(0, context->num_tracks, context, autotrack_context_step_cb, context->num_tracks > 1); + /* Advance the frame. */ BLI_spin_lock(&context->spin_lock); context->user.framenr += frame_delta; BLI_spin_unlock(&context->spin_lock); - return ok; + return context->step_ok; } void BKE_autotrack_context_sync(AutoTrackContext *context) -- cgit v1.2.3 From 06e64058dd687634b6bf873dbf21c6d3563ed6c9 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sun, 26 Nov 2017 19:06:26 +0100 Subject: Removing OMP: BKE's mask_rasterize.c Once again nothing much to say here, except that whole mask rendering process from VSE is about 25% quicker now. ;) --- source/blender/blenkernel/intern/mask_rasterize.c | 70 ++++++++++++++--------- 1 file changed, 42 insertions(+), 28 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/mask_rasterize.c b/source/blender/blenkernel/intern/mask_rasterize.c index 13ec970c65c..4c0cc131440 100644 --- a/source/blender/blenkernel/intern/mask_rasterize.c +++ b/source/blender/blenkernel/intern/mask_rasterize.c @@ -80,6 +80,7 @@ #include "BLI_math.h" #include "BLI_rect.h" +#include "BLI_task.h" #include "BLI_listbase.h" #include "BLI_linklist.h" @@ -1423,6 +1424,37 @@ float BKE_maskrasterize_handle_sample(MaskRasterHandle *mr_handle, const float x return value; } + +typedef struct MaskRasterizeBufferData { + MaskRasterHandle *mr_handle; + float x_inv, y_inv; + float x_px_ofs, y_px_ofs; + uint width; + + float *buffer; +} MaskRasterizeBufferData; + +static void maskrasterize_buffer_cb(void *userdata, int y) +{ + MaskRasterizeBufferData *data = userdata; + + MaskRasterHandle *mr_handle = data->mr_handle; + float *buffer = data->buffer; + + const uint width = data->width; + const float x_inv = data->x_inv; + const float x_px_ofs = data->x_px_ofs; + + uint i = (uint)y * width; + float xy[2]; + xy[1] = ((float)y * data->y_inv) + data->y_px_ofs; + for (uint x = 0; x < width; x++, i++) { + xy[0] = ((float)x * x_inv) + x_px_ofs; + + buffer[i] = BKE_maskrasterize_handle_sample(mr_handle, xy); + } +} + /** * \brief Rasterize a buffer from a single mask * @@ -1439,33 +1471,15 @@ void BKE_maskrasterize_buffer(MaskRasterHandle *mr_handle, { const float x_inv = 1.0f / (float)width; const float y_inv = 1.0f / (float)height; - const float x_px_ofs = x_inv * 0.5f; - const float y_px_ofs = y_inv * 0.5f; -#ifdef _MSC_VER - int y; /* msvc requires signed for some reason */ - - /* ignore sign mismatch */ -# pragma warning(push) -# pragma warning(disable:4018) -#else - unsigned int y; -#endif - -#pragma omp parallel for private(y) - for (y = 0; y < height; y++) { - unsigned int i = y * width; - unsigned int x; - float xy[2]; - xy[1] = ((float)y * y_inv) + y_px_ofs; - for (x = 0; x < width; x++, i++) { - xy[0] = ((float)x * x_inv) + x_px_ofs; - - buffer[i] = BKE_maskrasterize_handle_sample(mr_handle, xy); - } - } - -#ifdef _MSC_VER -# pragma warning(pop) -#endif + MaskRasterizeBufferData data = { + .mr_handle = mr_handle, + .x_inv = x_inv, + .y_inv = y_inv, + .x_px_ofs = x_inv * 0.5f, + .y_px_ofs = y_inv * 0.5f, + .width = width, + .buffer = buffer + }; + BLI_task_parallel_range(0, (int)height, &data, maskrasterize_buffer_cb, height * width > 10000); } -- cgit v1.2.3 From 440aa2bf70468d630c1dc70c719aad67496a47d5 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Sun, 26 Nov 2017 19:18:12 +0100 Subject: Cleanup: ImageEditor's mask drawing code was re-implementing `BKE_maskrasterize_buffer`! So this deduplicates and simplifies code, yeah. Also, as an odd bonus, new code seems slighly quicker than previous one (about 5 to 10% quicker). --- source/blender/blenkernel/intern/mask_rasterize.c | 9 +-- source/blender/editors/mask/mask_draw.c | 77 ++--------------------- 2 files changed, 5 insertions(+), 81 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/mask_rasterize.c b/source/blender/blenkernel/intern/mask_rasterize.c index 4c0cc131440..104bb0c07a6 100644 --- a/source/blender/blenkernel/intern/mask_rasterize.c +++ b/source/blender/blenkernel/intern/mask_rasterize.c @@ -1456,14 +1456,7 @@ static void maskrasterize_buffer_cb(void *userdata, int y) } /** - * \brief Rasterize a buffer from a single mask - * - * We could get some speedup by inlining #BKE_maskrasterize_handle_sample - * and calculating each layer then blending buffers, but this function is only - * used by the sequencer - so better have the caller thread. - * - * Since #BKE_maskrasterize_handle_sample is used threaded elsewhere, - * we can simply use openmp here for some speedup. + * \brief Rasterize a buffer from a single mask (threaded execution). */ void BKE_maskrasterize_buffer(MaskRasterHandle *mr_handle, const unsigned int width, const unsigned int height, diff --git a/source/blender/editors/mask/mask_draw.c b/source/blender/editors/mask/mask_draw.c index be7eb2bf9ed..407c19a3860 100644 --- a/source/blender/editors/mask/mask_draw.c +++ b/source/blender/editors/mask/mask_draw.c @@ -658,87 +658,18 @@ void ED_mask_draw(const bContext *C, draw_masklays(C, mask, draw_flag, draw_type, width, height, xscale * aspx, yscale * aspy); } -typedef struct ThreadedMaskRasterizeState { - MaskRasterHandle *handle; - float *buffer; - int width, height; -} ThreadedMaskRasterizeState; - -typedef struct ThreadedMaskRasterizeData { - int start_scanline; - int num_scanlines; -} ThreadedMaskRasterizeData; - -static void mask_rasterize_func(TaskPool * __restrict pool, void *taskdata, int UNUSED(threadid)) +static float *mask_rasterize(Mask *mask, const int width, const int height) { - ThreadedMaskRasterizeState *state = (ThreadedMaskRasterizeState *) BLI_task_pool_userdata(pool); - ThreadedMaskRasterizeData *data = (ThreadedMaskRasterizeData *) taskdata; - int scanline; - const float x_inv = 1.0f / (float)state->width; - const float y_inv = 1.0f / (float)state->height; - const float x_px_ofs = x_inv * 0.5f; - const float y_px_ofs = y_inv * 0.5f; - - for (scanline = 0; scanline < data->num_scanlines; scanline++) { - float xy[2]; - int x, y = data->start_scanline + scanline; - - xy[1] = ((float)y * y_inv) + y_px_ofs; - - for (x = 0; x < state->width; x++) { - int index = y * state->width + x; - - xy[0] = ((float)x * x_inv) + x_px_ofs; - - state->buffer[index] = BKE_maskrasterize_handle_sample(state->handle, xy); - } - } -} - -static float *threaded_mask_rasterize(Mask *mask, const int width, const int height) -{ - TaskScheduler *task_scheduler = BLI_task_scheduler_get(); - TaskPool *task_pool; MaskRasterHandle *handle; - ThreadedMaskRasterizeState state; - float *buffer; - int i, num_threads = BLI_task_scheduler_num_threads(task_scheduler), scanlines_per_thread; - - buffer = MEM_mallocN(sizeof(float) * height * width, "rasterized mask buffer"); + float *buffer = MEM_mallocN(sizeof(float) * height * width, "rasterized mask buffer"); /* Initialize rasterization handle. */ handle = BKE_maskrasterize_handle_new(); BKE_maskrasterize_handle_init(handle, mask, width, height, true, true, true); - state.handle = handle; - state.buffer = buffer; - state.width = width; - state.height = height; - - task_pool = BLI_task_pool_create(task_scheduler, &state); - - scanlines_per_thread = height / num_threads; - for (i = 0; i < num_threads; i++) { - ThreadedMaskRasterizeData *data = MEM_mallocN(sizeof(ThreadedMaskRasterizeData), - "threaded mask rasterize data"); - - data->start_scanline = i * scanlines_per_thread; - - if (i < num_threads - 1) { - data->num_scanlines = scanlines_per_thread; - } - else { - data->num_scanlines = height - data->start_scanline; - } - - BLI_task_pool_push(task_pool, mask_rasterize_func, data, true, TASK_PRIORITY_LOW); - } - - /* work and wait until tasks are done */ - BLI_task_pool_work_and_wait(task_pool); + BKE_maskrasterize_buffer(handle, width, height, buffer); /* Free memory. */ - BLI_task_pool_free(task_pool); BKE_maskrasterize_handle_free(handle); return buffer; @@ -802,7 +733,7 @@ void ED_mask_draw_region(Mask *mask, ARegion *ar, } if (draw_flag & MASK_DRAWFLAG_OVERLAY) { - float *buffer = threaded_mask_rasterize(mask, width, height); + float *buffer = mask_rasterize(mask, width, height); int format; if (overlay_mode == MASK_OVERLAY_ALPHACHANNEL) { -- cgit v1.2.3