Fix shared data access in raytracer, tidy up render loop

Render loop lost track of which blocks had been rendered due to a
synchronization issue. Specifically, std::vector<bool> is special-cased
as a bitfield and so is pretty much guaranteed not to be atomic unless
synchronized externally.

Also:

- Clean up types of a few variables
- Clean up openmp sharing type of variables (may result in better
  optimization)
- Replace shared rendered block count with an openmp reduction (results
  in fewer locks)

Fixes: lp:1608289
* https://bugs.launchpad.net/kicad/+bug/1608289

3d-viewer/3d_rendering/3d_render_raytracing/c3d_render_raytracing.cpp

@@ -353,33 +353,38 @@ void C3D_RENDER_RAYTRACING::rt_render_tracing( GLubyte *ptrPBO ,
 {
     m_isPreview = false;
 
-    unsigned int nrBlocks = m_blockPositions.size();    // Cache the number of blocks
-    unsigned startTime = GetRunningMicroSecs();         // Get time that started render this block
-    bool breakLoop = false;                             // It will be used to break the loop
+    const size_t nrBlocks = m_blockPositions.size();
+    const unsigned startTime = GetRunningMicroSecs();
+    bool breakLoop = false;
+    int numBlocksRendered = 0;
 
-    #pragma omp parallel for schedule(dynamic)
-    for( signed int iBlock = 0; iBlock < (int)nrBlocks; iBlock++ )
+    #pragma omp parallel for schedule(dynamic) shared(breakLoop) \
+        firstprivate(ptrPBO, nrBlocks, startTime) reduction(+:numBlocksRendered) default(none)
+    for( size_t iBlock = 0; iBlock < nrBlocks; iBlock++ )
     {
 
         #pragma omp flush(breakLoop)
-        if( !breakLoop ) // That is used to break the other threads
+        if( !breakLoop )
         {
-            // Check if this block was already processed
-            if( !m_blockPositionsWasProcessed[iBlock] )
+            bool process_block;
+
+            // std::vector<bool> stuffs eight bools to each byte, so access to
+            // them can never be natively atomic.
+            #pragma omp critical(checkProcessBlock)
             {
+                process_block = !m_blockPositionsWasProcessed[iBlock];
                 m_blockPositionsWasProcessed[iBlock] = true;
+            }
 
-                // Render this block
+            if( process_block )
+            {
                 rt_render_trace_block( ptrPBO, iBlock );
-
-                #pragma omp atomic
-                m_nrBlocksRenderProgress++;
+                numBlocksRendered++;
 
 
                 // Check if it spend already some time render and request to exit
                 // to display the progress
                 #ifdef _OPENMP
-                // This makes possible that only one thread (id 0) can check the time
                 if( omp_get_thread_num() == 0 )
                 #endif
                     if( (GetRunningMicroSecs() - startTime) > 150000 )
@@ -391,6 +396,8 @@ void C3D_RENDER_RAYTRACING::rt_render_tracing( GLubyte *ptrPBO ,
         }
     }
 
+    m_nrBlocksRenderProgress += numBlocksRendered;
+
     if( aStatusTextReporter )
         aStatusTextReporter->Report( wxString::Format( _( "Rendering: %.0f %%" ),
                                                        (float)(m_nrBlocksRenderProgress * 100) /

3d-viewer/3d_rendering/3d_render_raytracing/c3d_render_raytracing.h

@@ -40,6 +40,7 @@
 #include <plugins/3dapi/c3dmodel.h>
 
 #include <map>
+#include <cstddef>
 
 /// Vector of materials
 typedef std::vector< CBLINN_PHONG_MATERIAL > MODEL_MATERIALS;
@@ -110,7 +111,7 @@ private:
     unsigned long int m_stats_start_rendering_time;
 
     /// Save the number of blocks progress of the render
-    unsigned int m_nrBlocksRenderProgress;
+    size_t m_nrBlocksRenderProgress;
 
     CPOSTSHADER_SSAO m_postshader_ssao;