changeset 27779:1e6389ae4060

Add comment explaining code choice (bug #57353). * graphics.cc (patch::properties::update_vertex_normals): Add a comment to explain why a std::vector<std::vector<RowVector>> (num_v) is used.
author Markus Mützel <markus.muetzel@gmx.de>
date Fri, 06 Dec 2019 19:32:03 +0100
parents 2f8559459314
children 0dad96574d12
files libinterp/corefcn/graphics.cc
diffstat 1 files changed, 12 insertions(+), 5 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/graphics.cc	Mon Dec 02 23:43:27 2019 -0600
+++ b/libinterp/corefcn/graphics.cc	Fri Dec 06 19:32:03 2019 +0100
@@ -10104,8 +10104,8 @@
       Matrix v = get_vertices ().matrix_value ();
       Matrix f = get_faces ().matrix_value ();
 
-      octave_idx_type num_v = v.rows ();  // number of vertices
-      octave_idx_type num_f = f.rows ();  // number of faces
+      octave_idx_type num_v = v.rows ();      // number of vertices
+      octave_idx_type num_f = f.rows ();      // number of faces
       octave_idx_type max_nc = f.columns ();  // max. number of polygon corners
 
       // In which cases can we skip updating the normals?
@@ -10122,7 +10122,14 @@
         }
 
       // Second step: assign normals to the respective vertices
-      // list of normals for vertices
+
+      // The following code collects the face normals for all faces adjacent to
+      // each vertex.  For this, a std::vector of length NUM_V (which might be
+      // very large) is used so that memory is allocated from the heap rather
+      // than the stack.  Each element of this vector corresponds to one vertex
+      // of the patch.  The element itself is a variable length std::vector.
+      // This second vector contains the face normals (of type RowVector) of
+      // the adjacent faces.
       std::vector<std::vector<RowVector>> vec_vn (num_v);
       for (octave_idx_type i = 0; i < num_f; i++)
         {
@@ -10154,8 +10161,8 @@
               // direction of the normal.  How to determine the inner and outer
               // faces of all parts of the patch and point the normals outwards?
               // (Necessary for correct lighting with "backfacelighting" set to
-              // "lit" or "unlit".) Matlab does not seem to do it correctly
-              // either.  So bother here?
+              // "lit" or "unlit".)  Matlab does not seem to do it correctly
+              // either.  So should we bother?
 
               vn0 = *it;