commit ff819559a704106ada6e09ffe54ff24cf45f53da
parent 5713601a714672252a7e9733d98f7580e2a0c0f9
Author: Vincent Forest <vincent.forest@meso-star.com>
Date: Wed, 18 Mar 2015 12:04:02 +0100
Pursue to add thread safety to the s3d API
Diffstat:
5 files changed, 80 insertions(+), 26 deletions(-)
diff --git a/src/s3d.h b/src/s3d.h
@@ -222,6 +222,12 @@ s3d_shape_enable
(struct s3d_shape* shape,
const char enable);
+/* Define whether the shape is attached or not */
+S3D_API res_T
+s3d_shape_is_attached
+ (struct s3d_shape* shape,
+ char* is_attached);
+
/* Remove the shape from the scene on which it is attached. No error is
* reported if the shape is not attached to a scene. After its detachment, the
* scene release its reference on the shape */
diff --git a/src/s3d_scene.c b/src/s3d_scene.c
@@ -103,7 +103,6 @@ scene_setup(struct s3d_scene* scn)
}
exit:
- rtcCommit(scn->rtc_scn);
return res;
error:
LIST_FOR_EACH(node, &scn->shapes) {
@@ -223,10 +222,14 @@ s3d_scene_attach_shape(struct s3d_scene* scn, struct s3d_shape* shape)
if(!scn || !shape)
return RES_BAD_ARG;
+ /* Prevent the scene_<clear|pull|remove_shape> oprations */
mutex_lock(scn->lock);
+ /* Prevent the shape_<attach|detach|setup> operations */
+ mutex_rw_wlock(shape->lock);
if(!is_list_empty(&shape->scene_attachment)) {
mutex_unlock(scn->lock);
+ mutex_rw_unlock(shape->lock);
return RES_BAD_ARG;
}
@@ -237,7 +240,10 @@ s3d_scene_attach_shape(struct s3d_scene* scn, struct s3d_shape* shape)
scn->is_outdated = 1;
mutex_unlock(scn->lock);
+ mutex_rw_unlock(shape->lock);
+ /* The shape should be own by the caller. Consequently, the shape cannot be
+ * deleted before the end of the attach process */
S3D(shape_ref_get(shape));
return RES_OK;
}
@@ -249,6 +255,7 @@ s3d_scene_clear(struct s3d_scene* scn)
if(!scn)
return RES_BAD_ARG;
+ /* Prevent the scene_<attach_shape|remove_shape|pull> operations */
mutex_lock(scn->lock);
LIST_FOR_EACH_SAFE(node, tmp, &scn->shapes) {
struct s3d_shape* shape = CONTAINER_OF
@@ -263,25 +270,28 @@ res_T
s3d_scene_pull(struct s3d_scene* scn)
{
res_T res = RES_OK;
- int status;
- if(!scn) {
- res = RES_BAD_ARG;
- goto error;
- }
+ enum scene_status status;
- if(!scn->is_outdated)
- goto exit;
+ if(!scn)
+ return RES_BAD_ARG;
- if(ATOMIC_CAS(&scn->status, SCENE_PULL, SCENE_READY) == SCENE_RAY_TRACE) {
+ status = ATOMIC_CAS(&scn->status, SCENE_PULL, SCENE_READY);
+ if(status == SCENE_RAY_TRACE || status == SCENE_COMMIT) {
+ /* The scene cannot be pulled while it is ray traced or updated */
res = RES_BAD_ARG; /* TODO return a RES_BAD_OP instead */
goto error;
} else {
+ /* Prevent the scene_<attach/remove_shape|clear|pull> operations */
mutex_lock(scn->lock);
if(scn->is_outdated) {
res = scene_setup(scn);
+ rtcCommit(scn->rtc_scn);
scn->is_outdated = res != RES_OK;
}
mutex_unlock(scn->lock);
+
+ /* The status is necessaraly SCENE_PULL */
+ ASSERT(scn->status == SCENE_PULL);
ATOMIC_SET(&scn->status, SCENE_READY);
if(res != RES_OK)
goto error;
@@ -308,8 +318,10 @@ s3d_scene_trace_ray
if(!f3_is_normalized(dir) || range[0] < 0.f || range[0] > range[1])
return RES_BAD_ARG;
- if(ATOMIC_CAS(&scn->status, SCENE_RAY_TRACE, SCENE_READY) == SCENE_PULL)
- return RES_BAD_ARG; /* return a RES_BAD_OP instead */
+ if(ATOMIC_CAS(&scn->status, SCENE_RAY_TRACE, SCENE_READY) == SCENE_PULL) {
+ /* A scene cannot be ray-traced while it is pulled */
+ return RES_BAD_ARG; /* TODO return a RES_BAD_OP instead */
+ }
f3_set(ray.org, org);
f3_set(ray.dir, dir);
@@ -322,10 +334,10 @@ s3d_scene_trace_ray
ray.time = 0.f;
rtcIntersect(scn->rtc_scn, ray);
- ATOMIC_SET(&scn->status, SCENE_READY);
+ ATOMIC_CAS(&scn->status, SCENE_READY, SCENE_RAY_TRACE);
if(ray.geomID == RTC_INVALID_GEOMETRY_ID) {
- hit->shape = NULL;
+ *hit = S3D_HIT_NULL;
} else {
f3_set(hit->normal, ray.Ng);
hit->uv[0] = ray.u;
@@ -337,6 +349,7 @@ s3d_scene_trace_ray
ASSERT(hit->shape != NULL && ray.geomID == hit->shape->rtc_geom);
}
+
exit:
return res;
error:
diff --git a/src/s3d_scene_c.h b/src/s3d_scene_c.h
@@ -45,6 +45,7 @@
enum scene_status {
SCENE_PULL, /* The scene is pulling its updates */
+ SCENE_COMMIT, /* Some scene shapes are updating */
SCENE_RAY_TRACE, /* The scene is ray-traced */
SCENE_READY /* The scene can accept any operation */
};
@@ -54,7 +55,7 @@ struct s3d_scene {
struct darray_geom2shape geom2shape;
RTCScene rtc_scn;
char is_outdated; /* Flag defining if the scene description was updated */
- int status;
+ enum scene_status status;
struct mutex* lock;
diff --git a/src/s3d_shape.c b/src/s3d_shape.c
@@ -251,8 +251,9 @@ shape_release(ref_T* ref)
shape = CONTAINER_OF(ref, struct s3d_shape, ref);
dev = shape->dev;
- /* It is unacessary to protect the read of the scene_attachment, scn and
- * rtc_geom fields since the shape should not be shared when it is released */
+ /* It is unacessary to prevent the concurrent read/write of the
+ * scene_attachment, scn and rtc_geom fields since the shape should not be
+ * shared when it is released */
/* The shape should not be attached */
ASSERT(is_list_empty(&shape->scene_attachment));
@@ -290,6 +291,7 @@ s3d_shape_create_mesh
list_init(&shape->scene_attachment);
mesh_init(dev->allocator, &shape->data.mesh);
shape->type = SHAPE_MESH;
+ shape->status = SHAPE_STATUS_READY;
shape->rtc_geom = RTC_INVALID_GEOMETRY_ID;
shape->scn = NULL;
S3D(device_ref_get(dev));
@@ -329,14 +331,25 @@ s3d_shape_ref_put(struct s3d_shape* shape)
}
res_T
-s3d_shape_detach(struct s3d_shape* shape)
+s3d_shape_is_attached(struct s3d_shape* shape, char* is_attached)
{
- char is_not_attached;
- if(!shape) return RES_BAD_ARG;
+ if(!shape || !is_attached)
+ return RES_BAD_ARG;
+ /* Prevent the shape from its concurrent attachment/detachment. Anyway,
+ * several threads can concurrently query for its attachement status */
mutex_rw_rlock(shape->lock);
- is_not_attached = is_list_empty(&shape->scene_attachment);
+ *is_attached = !is_list_empty(&shape->scene_attachment);
+ ASSERT(!*is_attached || shape->scn);
mutex_rw_unlock(shape->lock);
- if(is_not_attached)
+ return RES_OK;
+}
+
+res_T
+s3d_shape_detach(struct s3d_shape* shape)
+{
+ char is_attached;
+ if(!shape) return RES_BAD_ARG;
+ if(S3D(shape_is_attached(shape, &is_attached)), !is_attached)
return RES_OK;
scene_remove_shape(shape->scn, shape);
return RES_OK;
@@ -359,10 +372,22 @@ s3d_shape_mesh_setup_indexed_vertices
char has_position = 0;
res_T res = RES_OK;
- if(!shape || shape->type != SHAPE_MESH || !ntris || !nverts || !attribs) {
- res = RES_BAD_ARG;
- goto error;
+ if(!shape || shape->type != SHAPE_MESH || !ntris || !nverts || !attribs)
+ return RES_BAD_ARG;
+
+ /* Prevent the shape from its concurrent attachment/detachment & update */
+ mutex_rw_wlock(shape->lock);
+
+ if(!is_list_empty(&shape->scene_attachment)) {
+ if(ATOMIC_CAS(&shape->scn->status, SCENE_COMMIT, SCENE_READY)==SCENE_PULL) {
+ /* Is there any use case where a scene is pulling shapes while its
+ * associated data were updated ? Currently it seems that it reveals only
+ * an unexpected comportment */
+ res = RES_BAD_ARG; /* FIXME return a RES_BAD_OP instead */
+ goto error;
+ }
}
+
/* Check indices description */
if(get_indices == S3D_KEEP) {
const unsigned nids_prev = darray_u32_size_get(&shape->data.mesh.indices);
@@ -411,12 +436,14 @@ s3d_shape_mesh_setup_indexed_vertices
mesh_setup_attribs(&shape->data.mesh, nverts, attribs + iattr, data);
}
}
- mutex_rw_rlock(shape->lock);
+
if(shape->scn) /* Notify the shape update */
shape->scn->is_outdated = 1;
- mutex_rw_unlock(shape->lock);
exit:
+ if(!is_list_empty(&shape->scene_attachment)) /* Restore scene status */
+ ATOMIC_CAS(&shape->scn->status, SCENE_READY, SCENE_COMMIT);
+ mutex_rw_unlock(shape->lock);
return res;
error:
goto exit;
diff --git a/src/s3d_shape_c.h b/src/s3d_shape_c.h
@@ -48,6 +48,12 @@ enum mesh_buffer {
MESH_VERTEX_BUFFER = BIT(1)
};
+enum shape_status {
+ SHAPE_STATUS_COMMIT, /* Shape data are committed */
+ SHAPE_STATUS_PULL, /* Shape data are pulled */
+ SHAPE_STATUS_READY /* The shape is ready for any operation */
+};
+
enum shape_type {
SHAPE_MESH,
SHAPE_SCENE,
@@ -68,6 +74,7 @@ struct mesh { /* Triangular mesh */
struct s3d_shape {
struct list_node scene_attachment;
enum shape_type type;
+ enum shape_status status;
unsigned rtc_geom; /* Embree geometry id */
struct s3d_scene* scn; /* The scene on which the shape is attached */
struct mutex_rw* lock;
