[Bf-blender-cvs] [2a70c8e] opensubdiv-modifier: Fix accidental crashes when changing subdiv level
Sergey Sharybin
noreply at git.blender.org
Mon May 12 20:09:04 CEST 2014
Commit: 2a70c8ebe0b9ff53107875ce5b2b0c3719f81396
Author: Sergey Sharybin
Date: Sun May 11 19:27:54 2014 +0200
https://developer.blender.org/rB2a70c8ebe0b9ff53107875ce5b2b0c3719f81396
Fix accidental crashes when changing subdiv level
Issue was caused by freeing GLMesh from non-main thread.
Since this might invoke OpenGL calls, better to free the
mesh from main thread instead.
===================================================================
M intern/opensubdiv/opensubdiv_capi.cc
M intern/opensubdiv/opensubdiv_capi.h
M intern/opensubdiv/opensubdiv_gpu_capi.cc
M source/blender/blenkernel/intern/CCGSubSurf.c
===================================================================
diff --git a/intern/opensubdiv/opensubdiv_capi.cc b/intern/opensubdiv/opensubdiv_capi.cc
index 3e7787c..1ba85bd 100644
--- a/intern/opensubdiv/opensubdiv_capi.cc
+++ b/intern/opensubdiv/opensubdiv_capi.cc
@@ -69,7 +69,7 @@
#include "MEM_guardedalloc.h"
-/* **************** Types declaration **************** */
+// **************** Types declaration ****************
struct OpenSubdiv_ComputeControllerDescr;
@@ -258,8 +258,7 @@ struct OpenSubdiv_GLMesh *openSubdiv_createOsdGLMeshFromEvaluator(
int num_vertex_elements = 3;
int num_varying_elements = 0;
- /* Trick to avoid passing multi-argument template to a macro. */
- OsdGLMeshInterface *mesh;
+ OsdGLMeshInterface *mesh = NULL;
switch (controller_type) {
#define CHECK_CONTROLLER_TYPE(type, class, controller_class) \
@@ -313,10 +312,15 @@ struct OpenSubdiv_GLMesh *openSubdiv_createOsdGLMeshFromEvaluator(
#undef CHECK_CONTROLLER_TYPE
}
+ if (mesh == NULL) {
+ return NULL;
+ }
+
OpenSubdiv_GLMesh *gl_mesh =
(OpenSubdiv_GLMesh *) OBJECT_GUARDED_NEW(OpenSubdiv_GLMesh);
gl_mesh->controller_type = controller_type;
gl_mesh->descriptor = (OpenSubdiv_GLMeshDescr *) mesh;
+ gl_mesh->level = level;
return gl_mesh;
}
diff --git a/intern/opensubdiv/opensubdiv_capi.h b/intern/opensubdiv/opensubdiv_capi.h
index e5a68b7..3491273 100644
--- a/intern/opensubdiv/opensubdiv_capi.h
+++ b/intern/opensubdiv/opensubdiv_capi.h
@@ -39,6 +39,7 @@ struct OpenSubdiv_GLMeshDescr;
typedef struct OpenSubdiv_GLMesh {
int controller_type;
OpenSubdiv_GLMeshDescr *descriptor;
+ int level;
} OpenSubdiv_GLMesh;
#endif
@@ -78,4 +79,4 @@ void openSubdiv_cleanup(void);
}
#endif
-#endif /* __OPENSUBDIV_CAPI_H__ */
+#endif // __OPENSUBDIV_CAPI_H__
diff --git a/intern/opensubdiv/opensubdiv_gpu_capi.cc b/intern/opensubdiv/opensubdiv_gpu_capi.cc
index 0df6614..680944e 100644
--- a/intern/opensubdiv/opensubdiv_gpu_capi.cc
+++ b/intern/opensubdiv/opensubdiv_gpu_capi.cc
@@ -39,7 +39,7 @@
#include <opensubdiv/osd/cudaGLVertexBuffer.h>
#include <opensubdiv/osd/glDrawRegistry.h>
-/* **************** Types declaration **************** */
+// **************** Types declaration ****************
using OpenSubdiv::OsdDrawContext;
using OpenSubdiv::OsdGLDrawRegistry;
@@ -54,9 +54,8 @@ protected:
virtual SourceConfigType *_CreateDrawSourceConfig(DescType const &desc);
};
-/* TODO(sergey): Fine for tests, but ideally need to be stored
- * in some sort of object draw context.
- */
+// TODO(sergey): Fine for tests, but ideally need to be stored
+// in some sort of object draw context.
static GLuint g_transformUB = 0,
g_transformBinding = 0,
g_tessellationUB = 0,
@@ -203,7 +202,7 @@ EffectDrawRegistry::_CreateDrawSourceConfig(DescType const &desc)
sconfig->fragmentShader.AddDefine("PRIM_TRI");
}
- /* TODO(sergey): Currently unsupported, but good to have for the reference. */
+ // TODO(sergey): Currently unsupported, but good to have for the reference.
#if 0
if (screenSpaceTess) {
sconfig->commonShader.AddDefine("OSD_ENABLE_SCREENSPACE_TESSELLATION");
@@ -271,7 +270,8 @@ EffectDrawRegistry::_CreateDrawConfig(DescType const &desc,
}
static GLuint bindProgram(OsdGLMeshInterface *mesh,
- OsdDrawContext::PatchArray const &patch)
+ OsdDrawContext::PatchArray const &patch,
+ int level)
{
EffectDrawRegistry::ConfigType *config =
g_effectRegistry.GetDrawConfig(patch.GetDescriptor());
@@ -298,7 +298,7 @@ static GLuint bindProgram(OsdGLMeshInterface *mesh,
float TessLevel;
} tessellationData;
- tessellationData.TessLevel = static_cast<float>(1 << /*g_tessLevel*/1);
+ tessellationData.TessLevel = static_cast<float>(1 << level);
if (! g_tessellationUB) {
glGenBuffers(1, &g_tessellationUB);
@@ -427,7 +427,7 @@ void openSubdiv_osdGLMeshDisplay(OpenSubdiv_GLMesh *gl_mesh)
glPatchParameteri(GL_PATCH_VERTICES, desc.GetNumControlVertices());
}
- GLuint program = bindProgram(mesh, patch);
+ GLuint program = bindProgram(mesh, patch, gl_mesh->level);
GLuint diffuseColor = glGetUniformLocation(program, "diffuseColor");
glProgramUniform4f(program, diffuseColor, 0.4f, 0.4f, 0.8f, 1);
diff --git a/source/blender/blenkernel/intern/CCGSubSurf.c b/source/blender/blenkernel/intern/CCGSubSurf.c
index 1e06a67..7d6d783 100644
--- a/source/blender/blenkernel/intern/CCGSubSurf.c
+++ b/source/blender/blenkernel/intern/CCGSubSurf.c
@@ -462,6 +462,7 @@ struct CCGSubSurf {
#ifdef WITH_OPENSUBDIV
struct OpenSubdiv_EvaluatorDescr *osd_evaluator;
struct OpenSubdiv_GLMesh *osd_mesh;
+ bool osd_mesh_invalid;
unsigned int osd_vao;
bool skip_grids;
short osd_compute;
@@ -922,6 +923,7 @@ CCGSubSurf *ccgSubSurf_new(CCGMeshIFC *ifc, int subdivLevels, CCGAllocatorIFC *a
#ifdef WITH_OPENSUBDIV
ss->osd_evaluator = NULL;
ss->osd_mesh = NULL;
+ ss->osd_mesh_invalid = false;
ss->osd_vao = 0;
ss->skip_grids = false;
ss->osd_compute = 0;
@@ -940,6 +942,7 @@ void ccgSubSurf_free(CCGSubSurf *ss)
openSubdiv_deleteEvaluatorDescr(ss->osd_evaluator);
}
if (ss->osd_mesh != NULL) {
+ /* TODO(sergey): Make sure free happens form the main thread! */
openSubdiv_deleteOsdGLMesh(ss->osd_mesh);
}
if (ss->osd_vao != 0) {
@@ -2327,6 +2330,12 @@ void ccgSubSurf_prepareGLMesh(CCGSubSurf *ss)
glGenVertexArrays(1, &ss->osd_vao);
}
+ if (ss->osd_mesh_invalid) {
+ openSubdiv_deleteOsdGLMesh(ss->osd_mesh);
+ ss->osd_mesh = NULL;
+ ss->osd_mesh_invalid = false;
+ }
+
if (ss->osd_mesh == NULL) {
ss->osd_mesh = openSubdiv_createOsdGLMeshFromEvaluator(
ss->osd_evaluator,
@@ -2558,10 +2567,11 @@ static bool opensubdiv_ensureEvaluator(CCGSubSurf *ss)
/* We would also need to re-create gl mesh from sratch
* if the topology changes.
+ * Here we only tag for free, actual free should happen
+ * from the main thread.
*/
if (ss->osd_mesh) {
- openSubdiv_deleteOsdGLMesh(ss->osd_mesh);
- ss->osd_mesh = NULL;
+ ss->osd_mesh_invalid = true;
}
}
}
More information about the Bf-blender-cvs
mailing list