Page MenuHome

Sculpt mesh abstraction API
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Sat, Jul 27, 12:21 AM.

Details

Summary

These functions make possible porting the tools from the sculpt branch, making them compatible with PBVH_FACES and #PBVH_BMESH## without duplicating the code. They can also help to simplify some existing code.

I ported some operators in the sculpt branch and they are working fine with dyntopo enabled. I think those are the most problematic operators, but maybe I'll need to add more functions to this API in the future. This should be enough to start making the final patches for most of the tools.

Diff Detail

Repository
rB Blender

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.Tue, Jul 30, 1:20 AM

This patch relies on the multires memory optimization being removed, otherwise there will be uninitialized variables and crashes. We can probably do this, but just nothing we can't really merge this without that.

Overall approach seems fine, some comments on the implementation.

source/blender/editors/sculpt_paint/sculpt.c
97

All these functions should be static since they are not exposed in any header file.

222

Creating a gset and the associated memory allocations here is too slow. Since the typical number of neighbors is low, you can just store them in an array and loop over the array every time to check for duplicates.

To avoid the memory allocation, SculptVertexNeighbourIter can contain a fixed size array of e.g. 256. int *neighbours would initially point to that. You can then use MEM_malloc to double the capacity when that number is exceeded, which would be almost never.

The code to do that can also be shared between the bmesh and faces case, if you make a common sculpt_vertex_neighbours_add function for both to call.

This revision now requires changes to proceed.Tue, Jul 30, 1:20 AM
  • Replace gset with static arrays
  • Make sculpt mesh API functions static
Pablo Dobarro (pablodp606) marked 2 inline comments as done.Tue, Jul 30, 6:29 PM

Ok, if this is fine we can wait to remove the multires optimization for merging, but this should already allow me to start making the patches for most of the new tools

Brecht Van Lommel (brecht) requested changes to this revision.Wed, Jul 31, 1:32 AM

Here's how I think it should work (did not test the code though):

1#define SCULPT_VERTEX_FIXED_CAPACITY 256
2
3typedef struct SculptVertexNeighbourIter {
4 int *neighbours;
5 int size;
6 int capacity;
7
8 int neighbours_fixed[SCULPT_VERTEX_FIXED_CAPACITY];
9
10 int index;
11 int i;
12} SculptVertexNeighbourIter;
13
14static void sculpt_vertex_neighbour_add(SculptVertexNeighbourIter *iter, int neighbour_index)
15{
16 for (int i = 0; i < iter->size; i++) {
17 if (iter->neighbours[i] == neighbour_index) {
18 return;
19 }
20 }
21
22 if (iter->count >= iter->capacity) {
23 iter->capacity += SCULPT_VERTEX_FIXED_CAPACITY;
24
25 if (iter->neighbours == iter->neighbours_fixed) {
26 iter->neighbours = MEM_mallocN(iter->capacity * sizeof(int), "neighbour array");
27 memcpy(iter->neighbours, iter->neighbours_fixed, sizeof(int) * iter->count);
28 }
29 else {
30 iter->neighbours = MEM_reallocN_id(iter->neighbours,
31 iter->capacity * sizeof(int),
32 "neighbour array");
33 }
34 }
35
36 iter->neighbours[iter->size] = neighbour_index;
37 iter->size++;
38}
39
40static void sculpt_vertex_neighbours_get_bmesh(SculptSession *ss,
41 int index,
42 SculptVertexNeighbourIter *iter)
43{
44 BMVert *v = BM_vert_at_index(ss->bm, index);
45 BMIter liter;
46 BMLoop *l;
47 iter->size = 0;
48 iter->capacity = SCULPT_VERTEX_FIXED_CAPACITY;
49 iter->neighbours = iter->neighbours_fixed;
50
51...
52
53#define sculpt_vertex_neighbours_iter_end(neighbour_iterator) \
54 } \
55 if (neighbour_iterator.neighbours != neighbour_iterator.neighbours_fixed) { \
56 MEM_freeN(neighbour_iterator.neighbours); \
57 } \

source/blender/editors/sculpt_paint/sculpt.c
218

This will not reallocate when iter->count reaches 2 * SCULPT_VERTEX_NEIGHBOUR_MAX.

243

Doing one allocation per vertex is still relatively slow, these things tend to work poorly especially with multithreading.

This revision now requires changes to proceed.Wed, Jul 31, 1:32 AM
  • Avoid allocating memory on neighbour iterator
This revision is now accepted and ready to land.Wed, Jul 31, 4:43 PM

Pablo, as we talked, I think the patch is fine to commit as well as long as you:

  • Document in the code that the API is not to be used with multi-res
  • Mention this also in the commit message
  • Do not commit to master any code that uses this API that do not poll for multi-res usage
This revision was automatically updated to reflect the committed changes.