Comments and style adjustments, and a couple fixes.

16 files changed, 62 insertions, 27 deletions

diff --git a/libdimension/cameras.c b/libdimension/cameras.c
index f346bbc..29d1df4 100644
--- a/libdimension/cameras.c
+++ b/libdimension/cameras.c
@@ -36,16 +36,19 @@ static dmnsn_line dmnsn_perspective_camera_ray_fn(const dmnsn_camera *camera,
 dmnsn_camera *
 dmnsn_new_perspective_camera(dmnsn_matrix trans)
 {
+  dmnsn_matrix *ptr;
   dmnsn_camera *camera = dmnsn_new_camera();
   if (camera) {
     camera->ray_fn = &dmnsn_perspective_camera_ray_fn;
 
-    camera->ptr = malloc(sizeof(dmnsn_matrix));
-    if (!camera->ptr) {
+    /* Allocate room for the transformation matrix */
+    ptr = malloc(sizeof(dmnsn_matrix));
+    if (!ptr) {
       dmnsn_delete_camera(camera);
       return NULL;
     }
-    *((dmnsn_matrix*)camera->ptr) = trans;
+    *ptr = trans;
+    camera->ptr = ptr;
   }
   return camera;
 }
diff --git a/libdimension/canvas.c b/libdimension/canvas.c
index 4151a0b..2e4000b 100644
--- a/libdimension/canvas.c
+++ b/libdimension/canvas.c
@@ -61,7 +61,7 @@ dmnsn_delete_canvas(dmnsn_canvas *canvas)
     for (i = 0; i < dmnsn_array_size(canvas->optimizers); ++i) {
       dmnsn_array_get(canvas->optimizers, i, &optimizer);
       if (optimizer.free_fn) {
-        optimizer.free_fn(optimizer.ptr);
+        (*optimizer.free_fn)(optimizer.ptr);
       }
     }
 
@@ -76,6 +76,7 @@ int
 dmnsn_optimize_canvas(dmnsn_canvas *canvas, dmnsn_canvas_optimizer optimizer)
 {
   if (canvas->too_late) {
+    /* Don't set an optimizer if dmnsn_set_pixel() has been called */
     return 1;
   } else {
     dmnsn_array_push(canvas->optimizers, &optimizer);
@@ -100,6 +101,6 @@ dmnsn_set_pixel(dmnsn_canvas *canvas, unsigned int x, unsigned int y,
   /* Call the optimizers */
   for (i = 0; i < dmnsn_array_size(canvas->optimizers); ++i) {
     dmnsn_array_get(canvas->optimizers, i, &optimizer);
-    optimizer.optimizer_fn(canvas, optimizer, x, y);
+    (*optimizer.optimizer_fn)(canvas, optimizer, x, y);
   }
 }
diff --git a/libdimension/color.c b/libdimension/color.c
index 4aede1f..433e437 100644
--- a/libdimension/color.c
+++ b/libdimension/color.c
@@ -268,7 +268,7 @@ dmnsn_color_add(dmnsn_color color1, dmnsn_color color2)
   Lab.b = Lab1.b + Lab2.b;
 
   ret = dmnsn_color_from_Lab(Lab, dmnsn_whitepoint);
-  /* Waited average of transparencies by intensity */
+  /* Weighted average of transparencies by intensity */
   ret.filter = (Lab1.L*color1.filter + Lab2.L*color2.filter)/Lab.L;
   ret.trans  = (Lab1.L*color1.trans  + Lab2.L*color2.trans)/Lab.L;
 
diff --git a/libdimension/dimension/cameras.h b/libdimension/dimension/cameras.h
index a75e9b9..d161f0e 100644
--- a/libdimension/dimension/cameras.h
+++ b/libdimension/dimension/cameras.h
@@ -31,6 +31,7 @@
 dmnsn_camera *dmnsn_new_perspective_camera(dmnsn_matrix trans);
 void dmnsn_delete_perspective_camera(dmnsn_camera *camera);
 
+/* Get or set the transformation matrix */
 dmnsn_matrix dmnsn_get_perspective_camera_trans(const dmnsn_camera *camera);
 void dmnsn_set_perspective_camera_trans(dmnsn_camera *camera, dmnsn_matrix T);
 
diff --git a/libdimension/dimension/canvas.h b/libdimension/dimension/canvas.h
index aaf728b..1818ccc 100644
--- a/libdimension/dimension/canvas.h
+++ b/libdimension/dimension/canvas.h
@@ -40,6 +40,7 @@ typedef struct {
   dmnsn_color *pixels;
 } dmnsn_canvas;
 
+/* Forward-declare dmnsn_canvas_optimizer */
 typedef struct dmnsn_canvas_optimizer dmnsn_canvas_optimizer;
 
 /* Canvas optimizer callback types */
diff --git a/libdimension/dimension/error.h b/libdimension/dimension/error.h
index cd7dd81..c5bf2a1 100644
--- a/libdimension/dimension/error.h
+++ b/libdimension/dimension/error.h
@@ -34,10 +34,17 @@ typedef enum {
   DMNSN_SEVERITY_HIGH    /* Always die */
 } dmnsn_severity;
 
+#ifdef __GNUC__
+  #define DMNSN_FUNC __PRETTY_FUNCTION__
+#elif (__STDC_VERSION__ >= 199901L)
+  #define DMNSN_FUNC __func__
+#else
+  #define DMNSN_FUNC __FILE__
+#endif
+
 /* Use this macro to report an error */
 #define dmnsn_error(severity, str)                                             \
-  dmnsn_report_error((dmnsn_severity)(severity), __PRETTY_FUNCTION__, __LINE__,\
-                     (str))
+  dmnsn_report_error((dmnsn_severity)(severity), DMNSN_FUNC, __LINE__, (str))
 
 /* Called by dmnsn_error() - don't call directly */
 void dmnsn_report_error(dmnsn_severity severity,
diff --git a/libdimension/dimension/progress.h b/libdimension/dimension/progress.h
index 57a38b6..0de1f03 100644
--- a/libdimension/dimension/progress.h
+++ b/libdimension/dimension/progress.h
@@ -52,14 +52,16 @@ typedef struct {
   pthread_mutex_t *mutex;
 } dmnsn_progress;
 
+/* Allocate a new progress object */
 dmnsn_progress *dmnsn_new_progress();
 /* For failed returns from *_async() functions */
 void dmnsn_delete_progress(dmnsn_progress *progress);
 
-/* This joins the worker thread and returns it's integer return value in
-   addition to deleting `progress' */
+/* Join the worker thread and returns it's integer return value in addition to
+   deleting `progress' */
 int dmnsn_finish_progress(dmnsn_progress *progress);
 
+/* Get the progress of the background task, out of 1.0 */
 double dmnsn_get_progress(const dmnsn_progress *progress);
 /* Wait for the progress to be >= prog, in a better way than spinlocking */
 void dmnsn_wait_progress(const dmnsn_progress *progress, double prog);
diff --git a/libdimension/dimension/raytrace.h b/libdimension/dimension/raytrace.h
index 2be79a8..a855c24 100644
--- a/libdimension/dimension/raytrace.h
+++ b/libdimension/dimension/raytrace.h
@@ -25,6 +25,7 @@
 #ifndef DIMENSION_RAYTRACE_H
 #define DIMENSION_RAYTRACE_H
 
+/* Render a scene by raytracing */
 int dmnsn_raytrace_scene(dmnsn_scene *scene);
 dmnsn_progress *dmnsn_raytrace_scene_async(dmnsn_scene *scene);
 
diff --git a/libdimension/dimension/scene.h b/libdimension/dimension/scene.h
index 67db588..11195d4 100644
--- a/libdimension/dimension/scene.h
+++ b/libdimension/dimension/scene.h
@@ -32,6 +32,7 @@ typedef struct {
   dmnsn_canvas *canvas;
 } dmnsn_scene;
 
+/* Create a scene, initializing only the ->objects field */
 dmnsn_scene *dmnsn_new_scene();
 void dmnsn_delete_scene(dmnsn_scene *scene);
 
diff --git a/libdimension/error.c b/libdimension/error.c
index df3ade3..81278f1 100644
--- a/libdimension/error.c
+++ b/libdimension/error.c
@@ -49,7 +49,7 @@ dmnsn_get_resilience()
   if (pthread_mutex_lock(&dmnsn_resilience_mutex) != 0) {
     /* Couldn't lock the mutex, so warn and continue. */
     fprintf(stderr, "Dimension WARNING: %s, line %u: %s\n",
-            __PRETTY_FUNCTION__, __LINE__,
+            DMNSN_FUNC, __LINE__,
             "Couldn't lock resilience mutex.");
   }
   resilience = dmnsn_resilience; /* Copy the static variable to a local */
@@ -57,7 +57,7 @@ dmnsn_get_resilience()
     /* Couldn't unlock the mutex, so warn and continue.  If the mutex was locked
        earlier, the next dmnsn_get/set_resilience is likely to hang. */
     fprintf(stderr, "Dimension WARNING: %s, line %u: %s\n",
-            __PRETTY_FUNCTION__, __LINE__,
+            DMNSN_FUNC, __LINE__,
             "Couldn't unlock resilience mutex.");
   }
   return resilience;
@@ -69,8 +69,7 @@ dmnsn_set_resilience(dmnsn_severity resilience)
 {
   if (resilience > DMNSN_SEVERITY_HIGH) {
     /* Tried to set an illegal resilience, bail out */
-    fprintf(stderr, "Dimension ERROR: %s, line %u: %s\n",
-            __PRETTY_FUNCTION__, __LINE__,
+    fprintf(stderr, "Dimension ERROR: %s, line %u: %s\n", DMNSN_FUNC, __LINE__,
             "Resilience has wrong value.");
     exit(EXIT_FAILURE);
   }
@@ -78,7 +77,7 @@ dmnsn_set_resilience(dmnsn_severity resilience)
   if (pthread_mutex_lock(&dmnsn_resilience_mutex) != 0) {
     /* Couldn't lock the mutex, so warn and continue. */
     fprintf(stderr, "Dimension WARNING: %s, line %u: %s\n",
-            __PRETTY_FUNCTION__, __LINE__,
+            DMNSN_FUNC, __LINE__,
             "Couldn't lock resilience mutex.");
   }
   dmnsn_resilience = resilience;
@@ -86,7 +85,7 @@ dmnsn_set_resilience(dmnsn_severity resilience)
     /* Couldn't unlock the mutex, so warn and continue.  If the mutex was locked
        earlier, the next dmnsn_get/set_resilience is likely to hang. */
     fprintf(stderr, "Dimension WARNING: %s, line %u: %s\n",
-            __PRETTY_FUNCTION__, __LINE__,
+            DMNSN_FUNC, __LINE__,
             "Couldn't unlock resilience mutex.");
   }
 }
diff --git a/libdimension/geometry.c b/libdimension/geometry.c
index 030cd68..8ebc3d9 100644
--- a/libdimension/geometry.c
+++ b/libdimension/geometry.c
@@ -65,7 +65,7 @@ dmnsn_rotation_matrix(dmnsn_vector theta)
   }
   axis = dmnsn_vector_normalize(theta);
 
-  /* Shorthand to make dmnsn_matrix_construct call legible */
+  /* Shorthand to make dmnsn_matrix_construct() call legible */
 
   s = sin(angle);
   t = 1.0 - cos(angle);
@@ -123,7 +123,13 @@ dmnsn_matrix_inverse(dmnsn_matrix A)
   double Pdet = A.n[0][0]*A.n[1][1] - A.n[0][1]*A.n[1][0];
 
   if (Pdet == 0.0) {
-    /* If we can't invert P, try a more generic algorithm */
+    /* If we can't invert P, try a more generic algorithm; this is very
+       unlikely, but not impossible, eg.
+         ( 1 1 0 0 )
+         ( 1 1 1 0 )
+         ( 0 1 1 0 )
+         ( 0 0 0 1 )
+     */
     return dmnsn_matrix_inverse_generic(A);
   }
 
@@ -146,6 +152,7 @@ dmnsn_matrix_inverse(dmnsn_matrix A)
   PiQ  = dmnsn_matrix2_mul(Pi, Q);
   RPiQ = dmnsn_matrix2_mul(R, PiQ);
 
+  /* Calculate the partitioned inverse */
   SS = dmnsn_matrix2_inverse(dmnsn_matrix2_sub(S, RPiQ));
   RR = dmnsn_matrix2_negate(dmnsn_matrix2_mul(SS, RPi));
   QQ = dmnsn_matrix2_negate(dmnsn_matrix2_mul(PiQ, SS));
diff --git a/libdimension/gl.c b/libdimension/gl.c
index 20c9af3..006db4b 100644
--- a/libdimension/gl.c
+++ b/libdimension/gl.c
@@ -46,12 +46,15 @@ dmnsn_gl_optimize_canvas(dmnsn_canvas *canvas)
   optimizer.optimizer_fn = &dmnsn_gl_optimizer_fn;
   optimizer.free_fn = &free;
 
+  /* Allocate a buffer to hold RGB values */
   optimizer.ptr = malloc(4*canvas->x*canvas->y*sizeof(GLushort));
   if (!optimizer.ptr) {
     return 1;
   }
 
+  /* Set a new optimizer */
   if (dmnsn_optimize_canvas(canvas, optimizer) != 0) {
+    /* Set failed; dmnsn_set_pixel() has probably been called already */
     free(optimizer.ptr);
     return 1;
   }
@@ -78,11 +81,11 @@ dmnsn_gl_write_canvas(const dmnsn_canvas *canvas)
     dmnsn_array_get(canvas->optimizers, i, &optimizer);
     if (optimizer.optimizer_fn == &dmnsn_gl_optimizer_fn) {
       glDrawPixels(width, height, GL_RGBA, GL_UNSIGNED_SHORT, optimizer.ptr);
-      return 0;
+      return glGetError() == GL_NO_ERROR ? 0 : 1;
     }
   }
 
-  /* We couldn't, so to this non-optimally */
+  /* We couldn't, so transform the canvas to RGB now */
   pixels = malloc(4*width*height*sizeof(GLushort));
   if (!pixels) {
     return 1;
@@ -129,7 +132,7 @@ dmnsn_gl_write_canvas(const dmnsn_canvas *canvas)
   glDrawPixels(width, height, GL_RGBA, GL_UNSIGNED_SHORT, pixels);
 
   free(pixels);
-  return 0;
+  return glGetError() == GL_NO_ERROR ? 0 : 1;
 }
 
 /* Read a canvas from a GL framebuffer.  Returns NULL on failure. */
@@ -157,6 +160,12 @@ dmnsn_gl_read_canvas(unsigned int x0, unsigned int y0,
 
   glReadPixels(x0, y0, width, height, GL_RGBA, GL_UNSIGNED_SHORT, pixels);
 
+  if (glGetError() != GL_NO_ERROR) {
+    free(pixels);
+    dmnsn_delete_canvas(canvas);
+    return NULL;
+  }
+
   for (y = 0; y < height; ++y) {
     for (x = 0; x < width; ++x) {
       pixel = pixels + 4*(y*width + x);
diff --git a/libdimension/inlines.c b/libdimension/inlines.c
index e7d4c74..0a11969 100644
--- a/libdimension/inlines.c
+++ b/libdimension/inlines.c
@@ -18,6 +18,8 @@
  * <http://www.gnu.org/licenses/>.                                       *
  *************************************************************************/
 
+/* Set DMNSN_INLINE to produce definitions of inline functions, emitted here,
+   if needed */
 #ifdef __cplusplus
   /* C++ inline semantics */
   #define DMNSN_INLINE inline
diff --git a/libdimension/progress.c b/libdimension/progress.c
index f366832..24f3a5e 100644
--- a/libdimension/progress.c
+++ b/libdimension/progress.c
@@ -63,6 +63,8 @@ dmnsn_new_progress()
       return NULL;
     }
 
+    /* Initialize the rwlock, condition variable, and mutex */
+
     if (pthread_rwlock_init(progress->rwlock, NULL) != 0) {
       free(progress->rwlock);
       free(progress->mutex);
@@ -71,7 +73,6 @@ dmnsn_new_progress()
       free(progress);
       return NULL;
     }
-
     if (pthread_cond_init(progress->cond, NULL) != 0) {
       if (pthread_rwlock_destroy(progress->rwlock) != 0) {
         dmnsn_error(DMNSN_SEVERITY_LOW,
@@ -135,9 +136,8 @@ int dmnsn_finish_progress(dmnsn_progress *progress)
   int retval = 1;
 
   if (progress) {
-    if (pthread_cond_broadcast(progress->cond) != 0) {
-      dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't signal condition variable.");
-    }
+    /* Wake up all waiters */
+    dmnsn_done_progress(progress);
 
     if (pthread_join(progress->thread, &ptr) != 0) {
       /* Medium severity because an unjoined thread likely means that the thread
diff --git a/libdimension/raytrace.c b/libdimension/raytrace.c
index 1984d24..7bb6381 100644
--- a/libdimension/raytrace.c
+++ b/libdimension/raytrace.c
@@ -196,7 +196,7 @@ dmnsn_raytrace_scene_impl(dmnsn_progress *progress, dmnsn_scene *scene,
   dmnsn_line ray, ray_trans;
   dmnsn_array *intersections;
   dmnsn_color color;
-  dmnsn_CIE_Lab Lab = { 0.0, 0.0, 0.0 }; // Shut up uninitialized use warning
+  dmnsn_CIE_Lab Lab = { 0.0, 0.0, 0.0 }; /* Shut up uninitialized use warning */
 
   width  = scene->canvas->x;
   height = scene->canvas->y;
diff --git a/libdimension/scene.c b/libdimension/scene.c
index 9405f03..d167e16 100644
--- a/libdimension/scene.c
+++ b/libdimension/scene.c
@@ -26,8 +26,9 @@ dmnsn_scene *
 dmnsn_new_scene()
 {
   dmnsn_scene *scene = malloc(sizeof(dmnsn_scene));
-  if (scene)
+  if (scene) {
     scene->objects = dmnsn_new_array(sizeof(dmnsn_object*));
+  }
   return scene;
 }