[Bf-blender-cvs] [342a5cb] compositor-2016: Fix T47637: Multiple multires objects in Sculpt mode make blender crash.
Bastien Montagne
noreply at git.blender.org
Wed Jun 8 21:52:32 CEST 2016
Commit: 342a5cb8261400735cb5f9098334b785dd4c4da8
Author: Bastien Montagne
Date: Thu Jun 2 15:57:58 2016 +0200
Branches: compositor-2016
https://developer.blender.org/rB342a5cb8261400735cb5f9098334b785dd4c4da8
Fix T47637: Multiple multires objects in Sculpt mode make blender crash.
That was a nice and funny hunt, albeit rather time consumming!
To summarize, so far code was using a static global gpu_buffer for pbvh vbo drawing
of 'grid' types (multires mostly?).
There were two issues here:
1) Global gpu buffer was assigned to GPU_PBVH_Buffers->index_buf, but then nearly no
check was done when freeing that buffer, to ensure we were not freeing the global one
(not totally sure this one was actually causing any issue, but was bad and unsafe anyway).
Was solved by adding a flag to GPU_PBVH_Buffers to indicate when we are using some
'common' buffer here, which freeing is handled separately.
2) Main issue: if several multires objects in sculpt mode with different grid size
were present simultaneously, the global gpu buffer had to be resized for each object draw
(i.e., freed and re-allocated), but then the pbvh nodes from other objects storing freed reference
to that global buffer had no way to know that it had been freed, which was causing the segfault & crash.
Was solved by getting rid of that global buffer, and instead allocating one 'grid_commmon_gpu_buffer' per pbvh.
Told ya baby, globals are *PURE EVIL*!
===================================================================
M source/blender/blenkernel/intern/pbvh.c
M source/blender/blenkernel/intern/pbvh_intern.h
M source/blender/gpu/GPU_buffers.h
M source/blender/gpu/intern/gpu_buffers.c
M source/blender/gpu/intern/gpu_init_exit.c
===================================================================
diff --git a/source/blender/blenkernel/intern/pbvh.c b/source/blender/blenkernel/intern/pbvh.c
index d73f087..58ec75d 100644
--- a/source/blender/blenkernel/intern/pbvh.c
+++ b/source/blender/blenkernel/intern/pbvh.c
@@ -637,6 +637,7 @@ void BKE_pbvh_free(PBVH *bvh)
BLI_gset_free(node->bm_other_verts, NULL);
}
}
+ GPU_free_pbvh_buffer_multires(&bvh->grid_common_gpu_buffer);
if (bvh->deformed) {
if (bvh->verts) {
@@ -1100,7 +1101,7 @@ static void pbvh_update_draw_buffers(PBVH *bvh, PBVHNode **nodes, int totnode)
node->totprim,
bvh->grid_hidden,
bvh->gridkey.grid_size,
- &bvh->gridkey);
+ &bvh->gridkey, &bvh->grid_common_gpu_buffer);
break;
case PBVH_FACES:
node->draw_buffers =
diff --git a/source/blender/blenkernel/intern/pbvh_intern.h b/source/blender/blenkernel/intern/pbvh_intern.h
index bae323d..4d2307c 100644
--- a/source/blender/blenkernel/intern/pbvh_intern.h
+++ b/source/blender/blenkernel/intern/pbvh_intern.h
@@ -145,6 +145,11 @@ struct PBVH {
const DMFlagMat *grid_flag_mats;
int totgrid;
BLI_bitmap **grid_hidden;
+ /* index_buf of GPU_PBVH_Buffers can be the same for all 'fully drawn' nodes (same size).
+ * Previously was stored in a static var in gpu_buffer.c, but this breaks in case we handle several different
+ * objects in sculpt mode with different sizes at the same time, so now storing that common gpu buffer
+ * in an opaque pointer per pbvh. See T47637. */
+ struct GridCommonGPUBuffer *grid_common_gpu_buffer;
/* Only used during BVH build and update,
* don't need to remain valid after */
diff --git a/source/blender/gpu/GPU_buffers.h b/source/blender/gpu/GPU_buffers.h
index ee7abe0..aefaf1a 100644
--- a/source/blender/gpu/GPU_buffers.h
+++ b/source/blender/gpu/GPU_buffers.h
@@ -49,6 +49,7 @@ struct DerivedMesh;
struct GSet;
struct GPUVertPointLink;
struct GPUDrawObject;
+struct GridCommonGPUBuffer;
struct PBVH;
struct MVert;
@@ -160,9 +161,6 @@ void GPU_buffer_free(GPUBuffer *buffer);
void GPU_drawobject_free(struct DerivedMesh *dm);
-/* free special global multires grid buffer */
-void GPU_buffer_multires_free(bool force);
-
/* flag that controls data type to fill buffer with, a modifier will prepare. */
typedef enum {
GPU_BUFFER_VERTEX = 0,
@@ -231,8 +229,9 @@ GPU_PBVH_Buffers *GPU_build_mesh_pbvh_buffers(
const int *face_indices,
const int face_indices_len);
-GPU_PBVH_Buffers *GPU_build_grid_pbvh_buffers(int *grid_indices, int totgrid,
- unsigned int **grid_hidden, int gridsize, const struct CCGKey *key);
+GPU_PBVH_Buffers *GPU_build_grid_pbvh_buffers(
+ int *grid_indices, int totgrid,unsigned int **grid_hidden, int gridsize, const struct CCGKey *key,
+ struct GridCommonGPUBuffer **grid_common_gpu_buffer);
GPU_PBVH_Buffers *GPU_build_bmesh_pbvh_buffers(bool smooth_shading);
@@ -267,5 +266,6 @@ void GPU_init_draw_pbvh_BB(void);
bool GPU_pbvh_buffers_diffuse_changed(GPU_PBVH_Buffers *buffers, struct GSet *bm_faces, bool show_diffuse_color);
void GPU_free_pbvh_buffers(GPU_PBVH_Buffers *buffers);
+void GPU_free_pbvh_buffer_multires(struct GridCommonGPUBuffer **grid_common_gpu_buffer);
#endif
diff --git a/source/blender/gpu/intern/gpu_buffers.c b/source/blender/gpu/intern/gpu_buffers.c
index f80ce3c..2c6f204 100644
--- a/source/blender/gpu/intern/gpu_buffers.c
+++ b/source/blender/gpu/intern/gpu_buffers.c
@@ -107,10 +107,12 @@ static GPUAttrib attribData[MAX_GPU_ATTRIB_DATA] = { { -1, 0, 0 } };
static ThreadMutex buffer_mutex = BLI_MUTEX_INITIALIZER;
/* multires global buffer, can be used for many grids having the same grid size */
-static GPUBuffer *mres_glob_buffer = NULL;
-static int mres_prev_gridsize = -1;
-static GLenum mres_prev_index_type = 0;
-static unsigned mres_prev_totquad = 0;
+typedef struct GridCommonGPUBuffer {
+ GPUBuffer *mres_buffer;
+ int mres_prev_gridsize;
+ GLenum mres_prev_index_type;
+ unsigned mres_prev_totquad;
+} GridCommonGPUBuffer;
void GPU_buffer_material_finalize(GPUDrawObject *gdo, GPUBufferMaterial *matinfo, int totmat)
{
@@ -407,33 +409,6 @@ void GPU_buffer_free(GPUBuffer *buffer)
BLI_mutex_unlock(&buffer_mutex);
}
-void GPU_buffer_multires_free(bool force)
-{
- if (!mres_glob_buffer) {
- /* Early output, no need to lock in this case, */
- return;
- }
-
- if (force && BLI_thread_is_main()) {
- if (mres_glob_buffer) {
- if (mres_glob_buffer->id)
- glDeleteBuffers(1, &mres_glob_buffer->id);
- MEM_freeN(mres_glob_buffer);
- }
- }
- else {
- BLI_mutex_lock(&buffer_mutex);
- gpu_buffer_free_intern(mres_glob_buffer);
- BLI_mutex_unlock(&buffer_mutex);
- }
-
- mres_glob_buffer = NULL;
- mres_prev_gridsize = -1;
- mres_prev_index_type = 0;
- mres_prev_totquad = 0;
-}
-
-
void GPU_drawobject_free(DerivedMesh *dm)
{
GPUDrawObject *gdo;
@@ -1009,6 +984,7 @@ struct GPU_PBVH_Buffers {
const int *grid_indices;
int totgrid;
bool has_hidden;
+ bool is_index_buf_global; /* Means index_buf uses global bvh's grid_common_gpu_buffer, **DO NOT** free it! */
bool use_bmesh;
@@ -1226,8 +1202,10 @@ GPU_PBVH_Buffers *GPU_build_mesh_pbvh_buffers(
/* An element index buffer is used for smooth shading, but flat
* shading requires separate vertex normals so an index buffer is
* can't be used there. */
- if (buffers->smooth)
+ if (buffers->smooth) {
buffers->index_buf = GPU_buffer_alloc(sizeof(unsigned short) * tottri * 3);
+ buffers->is_index_buf_global = false;
+ }
if (buffers->index_buf) {
/* Fill the triangle buffer */
@@ -1248,8 +1226,11 @@ GPU_PBVH_Buffers *GPU_build_mesh_pbvh_buffers(
GPU_buffer_unlock(buffers->index_buf, GPU_BINDING_INDEX);
}
else {
- GPU_buffer_free(buffers->index_buf);
+ if (!buffers->is_index_buf_global) {
+ GPU_buffer_free(buffers->index_buf);
+ }
buffers->index_buf = NULL;
+ buffers->is_index_buf_global = false;
}
}
@@ -1416,22 +1397,33 @@ void GPU_update_grid_pbvh_buffers(GPU_PBVH_Buffers *buffers, CCGElem **grids,
} (void)0
/* end FILL_QUAD_BUFFER */
-static GPUBuffer *gpu_get_grid_buffer(int gridsize, GLenum *index_type, unsigned *totquad)
+static GPUBuffer *gpu_get_grid_buffer(
+ int gridsize, GLenum *index_type, unsigned *totquad, GridCommonGPUBuffer **grid_common_gpu_buffer)
{
/* used in the FILL_QUAD_BUFFER macro */
BLI_bitmap * const *grid_hidden = NULL;
const int *grid_indices = NULL;
int totgrid = 1;
+ GridCommonGPUBuffer *gridbuff = *grid_common_gpu_buffer;
+
+ if (gridbuff == NULL) {
+ *grid_common_gpu_buffer = gridbuff = MEM_mallocN(sizeof(GridCommonGPUBuffer), __func__);
+ gridbuff->mres_buffer = NULL;
+ gridbuff->mres_prev_gridsize = -1;
+ gridbuff->mres_prev_index_type = 0;
+ gridbuff->mres_prev_totquad = 0;
+ }
+
/* VBO is already built */
- if (mres_glob_buffer && mres_prev_gridsize == gridsize) {
- *index_type = mres_prev_index_type;
- *totquad = mres_prev_totquad;
- return mres_glob_buffer;
+ if (gridbuff->mres_buffer && gridbuff->mres_prev_gridsize == gridsize) {
+ *index_type = gridbuff->mres_prev_index_type;
+ *totquad = gridbuff->mres_prev_totquad;
+ return gridbuff->mres_buffer;
}
/* we can't reuse old, delete the existing buffer */
- else if (mres_glob_buffer) {
- GPU_buffer_free(mres_glob_buffer);
+ else if (gridbuff->mres_buffer) {
+ GPU_buffer_free(gridbuff->mres_buffer);
}
/* Build new VBO */
@@ -1439,17 +1431,17 @@ static GPUBuffer *gpu_get_grid_buffer(int gridsize, GLenum *index_type, unsigned
if (gridsize * gridsize < USHRT_MAX) {
*index_type = GL_UNSIGNED_SHORT;
- FILL_QUAD_BUFFER(unsigned short, *totquad, mres_glob_buffer);
+ FILL_QUAD_BUFFER(unsigned short, *totquad, gridbuff->mres_buffer);
}
else {
*index_type = GL_UNSIGNED_INT;
- FILL_QUAD_BUFFER(unsigned int, *totquad, mres_glob_buffer);
+ FILL_QUAD_BUFFER(unsigned int, *totquad, gridbuff->mres_buffer);
}
- mres_prev_gridsize = gridsize;
- mres_prev_index_type = *index_type;
- mres_prev_totquad = *totquad;
- return mres_glob_buffer;
+ gridbuff->mres_prev_gridsize = gridsize;
+ gridbuff->mres_prev_index_type = *index_type;
+ gridbuff->mres_prev_totquad = *totquad;
+ return gridbuff->mres_buffer;
}
#define FILL_FAST_BUFFER(type_) \
@@ -1476,8 +1468,9 @@ static GPUBuffer *gpu_get_grid_buffer(int gridsize, GLenum *index_type, unsigned
} \
} (void)0
-GPU_PBVH_Buffers *GPU_build_grid_pbvh_buffers(int *grid_indices, int totgrid,
- BLI_bitmap **grid_hidden, int gridsize, const CCGKey *key)
+GPU_PBVH_Buffers *GPU_build_grid_pbvh_buffers(
+ int *grid_indices, int totgrid, BLI_bitmap **grid_hidden, int gridsize, const CCGKey *key,
+ GridCommonGPUBuffer **grid_common_gpu_buffer)
{
GPU_PBVH_Buffers *buffers;
int totquad;
@@ -1506,8 +1499,10 @@ GPU_PBVH_Buffers *GPU_build_grid_pbvh_buffers(int *grid_indices, int totgrid,
}
if (totquad == fully_visible_totquad) {
- buffers->index_buf = gpu_get_grid_buffer(gridsize, &buffers->index_type, &buffers->tot_quad);
+ buffers->index_buf = gpu_get_grid_buffer(
+ gridsize, &buffers->index_type, &buffers->tot_quad, grid_common_gpu_buffer);
buffers->has_hidden = false;
+ buffers->is_index_buf_global = true;
}
else {
buffers->tot_quad = totquad;
@@ -1522,6 +1517,7 @@ GPU_PBVH_Buffers *GPU_build_grid_pbvh_buffers(int *grid_indices, int totgrid,
}
buffers->has_hidden = true;
+ buffers->is_index_buf_global = false;
}
/* Build coord/normal VBO */
@@ -1746,8 +1742,9 @@ void GPU_update_bmesh_pbvh_buffers(GPU_PBVH_Buffers *buffers,
const int use_short = (maxvert < USHRT_MAX);
/* Initialize triangle index buffer */
- if (buffers->index_buf)
+ if (buffers->index_buf && !buffers->is_index_buf_global)
GPU_buffer_free(buffers->index_buf);
+ buffers->is_index_buf_global = false;
buffers->index_buf = GPU_buffer_alloc((use_short ?
sizeof(unsigned short) :
sizeof(unsigned int)) * 3 * tottri);
@@ -1792,12 +17
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list