Fixed index out of range bug

[neverhood311]

Line 152 sets faceIndex to the maximum index of the m_faceAreaCDF
vector. Line 158 adds 1 to faceIndex and tries to access that element,
resulting in a segfault. We need to subtract 2 from m_faceAreaCDF.size()
to get the right index.

[Tecla]

This is an okay fix, but I think there's a better one. The first entry in m_faceAreaCDF is always zero, and that's just a waste. I think it's better to not store that and have the same number of entries in m_faceAreaCDF as there are in m_faces.

[Tecla]

Try this change instead:

diff --git a/Rayito_Stage7_QT/RMesh.h b/Rayito_Stage7_QT/RMesh.h
index eb68e95..b12c8a5 100644
--- a/Rayito_Stage7_QT/RMesh.h
+++ b/Rayito_Stage7_QT/RMesh.h
@@ -107,7 +107,7 @@ public:
         // area based on a random number (this means you can use meshes as area
         // lights).
         m_faceAreaCDF.clear();
-        m_faceAreaCDF.reserve(m_faces.size() + 1);
+        m_faceAreaCDF.reserve(m_faces.size());
         m_totalArea = 0.0f;
         for (size_t faceIndex = 0; faceIndex < m_faces.size(); ++faceIndex)
         {
@@ -119,10 +119,9 @@ public:
                 Point p2 = m_vertices[m_faces[faceIndex].m_vertexIndices[tri + 2]];
                 faceArea += cross(p1 - p0, p2 - p0).length() * 0.5f;
             }
-            m_faceAreaCDF.push_back(m_totalArea);
             m_totalArea += faceArea;
+            m_faceAreaCDF.push_back(m_totalArea);
         }
-        m_faceAreaCDF.push_back(m_totalArea);
         
         // Build the BVH so ray intersections are nice and fast
         m_bvh.build();
@@ -140,6 +139,8 @@ public:
                                Vector& outNormal,
                                float& outPdf)
     {
+        if (m_faces.empty())
+            return false;
         // Select a face based on a random number (u3), proportional to face
         // surface area; a face with double the surface area of another is twice
         // as likely to be selected.
@@ -155,7 +156,7 @@ public:
         else
             faceIndex = std::distance(m_faceAreaCDF.begin(), iter) - 1;
         // Find the actual triangle on the face we are choosing
-        float faceArea = m_faceAreaCDF[faceIndex + 1] - m_faceAreaCDF[faceIndex];
+        float faceArea = faceIndex > 0 ? m_faceAreaCDF[faceIndex] - m_faceAreaCDF[faceIndex - 1] : m_faceAreaCDF[0];
         float triangleSelector = (u3 * m_totalArea - m_faceAreaCDF[faceIndex]) / faceArea;
         float triangleAreaSoFar = 0.0f;
         for (size_t tri = 0; tri < m_faces[faceIndex].m_vertexIndices.size() - 2; ++tri)
@@ -218,7 +219,12 @@ public:
     
     virtual float elementArea(unsigned int index) const
     {
-        return m_faceAreaCDF[index + 1] - m_faceAreaCDF[index];
+        if (m_faces.empty() || index >= m_faces.size())
+            return 0.0f;
+        else if (index == 0)
+            return m_faceAreaCDF[0];
+        else
+            return m_faceAreaCDF[index] - m_faceAreaCDF[index - 1];
     }
     
     // Methods for BVH intersection

[neverhood311]

Cool. I'll give this a try tomorrow. Would you like me to commit these changes or were you going to do it?

[Tecla]

Go ahead and make the changes in your branch and modify/update the pull request. That way you can get credit for testing it out to make sure I didn't put any new bugs in.

[neverhood311]

Ok, I made those changes, did some tests, and wasn't able to crash it. I manually set u3 to both 1.0f and 0.0f to make sure it handled those values properly. No problems.

[neverhood311]

Hey, just wondering whether you still wanted to merge this PR