<p>The problem with your code is, that it produces a lot of degenerated triangles. The not so obvious ones are at the top and bottom. The obvious ones are at the "wrapping line".</p>

<p>So what exactly is the problem? Each of your faces is a rectangle that consists of 2 triangles and 4 points. The 2 triangles share 2 points and each one has 1 individual point. But at the bottom/top 2 points are identical, so your rectangle degenerates to a triangle. As a result, one triangle degenerates to a line because of the duplicate points.</p>

<p>Now, why is this problematic for you? It is this line, that causes the trouble: </p>

<pre class="lang-cpp prettyprint-override"><code>vec3 normal = normalize(cross(d1, d2));
</code></pre>

<p>The cross product of a vector with itself is the zero vector. If you normalize a vector, you divide by its length, which is 0 in this case. So this is where your <code>NaN</code>s come from. Until now, I just spoke about the top and bottom, but why is it happening at the "wrapping line"? The problem is a result of your index calculation:</p>

<pre class="lang-cpp prettyprint-override"><code>    for(uint i=0; i &lt; u_resolution - 1; i++)
    {
        for(uint j=0; j &lt; v_resolution; j++)
        {
            // Grab the 4 points in a quad face in the sphere
            uint p00 = (i * (v_resolution) + j);
            uint p01 = (i * (v_resolution) + ((j + 1) % v_resolution));
            uint p10 = ((i + 1) * v_resolution + j);
            uint p11 = ((i + 1) * v_resolution + ((j + 1) % v_resolution));
   ...
</code></pre>

<p>The <code>j</code> loop is running until <code>v_resolution</code>.  So <code>j+1</code> gets problematic. I know your intention here is, that you need the triangles from the "other side", to calculate your normals correctly. That is why you performed the <code>% v_resolution</code> to get the corresponding points. But the problem is, you are not getting the points you want. With your code, you are including the last and first point of a "horizontal line", but they are identical in your case. The result is that you get a face that degenerates to a line, which means, that you will divide by 0 during normalization of both triangles surface normals.</p>

<p>A quick way to fix the <code>NaN</code>s at the "wrapping line" would be to make a temporary variable and store <code>j+1</code> in it. Then check if it is equal or higher than <code>v_resolution</code>. If <code>true</code>, increase it by one. Use this variable everywhere you used <code>j+1</code> during the index generation. However, this will still cause an issue at the wrapping line, since your start and end vertices are not the same and you only considered "both sides" for the last vertices of a horizontal line, but not for the first. This will lead to incorrect normals for the start vertices and you should see a jump in the shading. You might fix it, by just copying the normals of the last vertices to the first ones. </p>

<p>Also, you still need to address the problems at the top and bottom. This can be fixed by making a special loop only for top and bottom triangles. However, these are suboptimal fixes. A better approach would be to write code that does not produce duplicate points since it lowers the risk of creating degenerated triangles and is more memory efficient.</p>

CC BY-SA 4.0
2020-04-16T15:47:04.687Z
Ah yes I am an idiot, the vertical band can be fixed by dividing by `v_resolution` instead of `v_resolution-1` To prevent the last set of vertices from being duplicated. 

I am still thinking about the polar ones
Makogan
CC BY-SA 4.0
2020-04-16T20:57:30.160Z
Well, you need to treat them separately in their own loop. Additionally, try to avoid duplicating the polar points. Even without the degenerated triangles, the normals are incorrect, because each duplicate polar point calculates it's normal only from 2 adjacent triangles instead of all (`v_resolution`) triangles that are connected to the pole.
wychmaster
CC BY-SA 4.0
2020-04-16T21:09:30.830Z
But separate loops are fugly! But yes I think I'll hve to go with that
Makogan
CC BY-SA 4.0
2020-04-16T13:06:44.980Z
2020-04-16T15:36:11.303Z
2020-04-16T15:36:11.303Z
wychmaster
<p>I seem to have an off by one error when creating the indices for the faces of a sphere and I can't understand where.</p>

<p>The exact problem is that when I try to calculate the normals of each face a dark region of nans appears in the region where the vertices wrap around.</p>

<p><a href="https://i.sstatic.net/yh3pn.png" rel="nofollow noreferrer"><img src="https://i.sstatic.net/yh3pn.png" alt="enter image description here"></a></p>

<p>I generate the vertices and face information like this:</p>

<pre><code>pair&lt;vector&lt;vec3&gt;, vector&lt;uint&gt;&gt; GenerateSphere(
    float radius,
    uint u_resolution,
    uint v_resolution)
{
    vector&lt;vec3&gt; points;
    vector&lt;uint&gt; indices;
    for(uint i=0; i&lt; u_resolution; i++)
    {
        float u = (float(i) / float(u_resolution - 1)) * M_PI - M_PI / 2.f;
        for(uint j=0; j &lt; v_resolution; j++)
        {
            float v = float(j) * 2.f * M_PI / float(v_resolution - 1);

            points.push_back(vec3(cos(u) * cos(v), cos(u) * sin(v), sin(u)) * radius);
        }
    }

    for(uint i=0; i &lt; u_resolution - 1; i++)
    {
        for(uint j=0; j &lt; v_resolution; j++)
        {
            // Grab the 4 points in a quad face in the sphere
            uint p00 = (i * (v_resolution) + j);
            uint p01 = (i * (v_resolution) + ((j + 1) % v_resolution));
            uint p10 = ((i + 1) * v_resolution + j);
            uint p11 = ((i + 1) * v_resolution + ((j + 1) % v_resolution));

            assert((i + 1) &lt; u_resolution);
            assert(p00 &lt; points.size());
            assert(p01 &lt; points.size());
            assert(p10 &lt; points.size());
            assert(p11 &lt; points.size());
            assert(
                p00 != p01 &amp;&amp; p00 != p11 &amp;&amp; p00 != p10 &amp;&amp;
                p01 != p10 &amp;&amp; p01 != p11 &amp;&amp;
                p10 != p11
            );

            // Add the first triangle
            indices.push_back(p00);
            indices.push_back(p01);
            indices.push_back(p11);

            // Add the second triangle
            indices.push_back(p00);
            indices.push_back(p11);
            indices.push_back(p10);
        }
    }

    //for(auto&amp; v : points) cout &lt;&lt; to_string(v) &lt;&lt; endl;
    //cout &lt;&lt; endl;
    return {points, indices};
}
</code></pre>

<p>And immediately after calculate the normals like this:</p>

<pre><code>std::vector&lt;glm::vec3&gt; GenerateNormals(const std::vector&lt;glm::vec3&gt;&amp; vertices,
    const std::vector&lt;uint&gt;&amp; indices)
{
    assert(indices.size() % 3 ==0);

    map&lt;uint, vector&lt;vec3&gt;&gt; normal_map;
    for(uint i=0; i &lt; indices.size(); i += 3)
    {
        vec3 v1 = vertices[indices[i]];
        vec3 v2 = vertices[indices[i + 1]];
        vec3 v3 = vertices[indices[i + 2]];

        vec3 d1 = v2 - v1;
        vec3 d2 = v3 - v1;

        assert(indices[i] &lt; vertices.size());
        assert(indices[i + 1] &lt; vertices.size());
        assert(indices[i + 2] &lt; vertices.size());
        assert(indices[i] != indices[i+1] &amp;&amp; indices[i] != indices[i+2] &amp;&amp;
            indices[i+1] != indices[i+2]);

        if(abs(dot(d1, d2)) &lt; 0.0001)
        {
            d1 = v3 - v2;
            d2 = v1 - v2;
        }

        if(abs(dot(d1, d2)) &lt; 0.0001)
        {
            d1 = v1 - v3;
            d2 = v2 - v3;
        }

        vec3 normal = normalize(cross(d1, d2));

        normal_map[indices[i]].push_back(normal);
        normal_map[indices[i + 1]].push_back(normal);
        normal_map[indices[i + 2]].push_back(normal);
    }

    assert(normal_map.size() == vertices.size());
    std::vector&lt;glm::vec3&gt; normals(vertices.size());
    for(auto&amp; pair : normal_map)
    {
        uint index = pair.first;
        vector&lt;vec3&gt; v_normals = pair.second;

        vec3 normal(0);
        for(auto&amp; n : v_normals)
            normal += n;

        normal /= float(v_normals.size());
        normals[index] = normal;
    }
    assert(normals.size() == vertices.size());
    return normals;
}
</code></pre>

<p>Note: I know that I could just calculate the normals by taking the difference between the current vertex and the center of the sphere. These 2 functions I plan to use as templates to code other paramtetric surfaces, so that's not a solution for me I am trying to understand what I did wrong more than trying to fix it.</p>

<p>I have added lots of assertions through the code to try to catch the issue and I have also stepped through gdb but I can't seem to find the issue. </p>

<c++><geometry><parametrization>
Off by one error when creating face indices
Makogan
# Off by one error when creating face indices

(Asked by: username_0 on 2020-04-16)

I seem to have an off by one error when creating the indices for the faces of a sphere and I can't understand where.

The exact problem is that when I try to calculate the normals of each face a dark region of nans appears in the region where the vertices wrap around.

[![enter image description here](https://i.sstatic.net/yh3pn.png)](https://i.sstatic.net/yh3pn.png)

I generate the vertices and face information like this:

```
pair<vector<vec3>, vector<uint>> GenerateSphere(
    float radius,
    uint u_resolution,
    uint v_resolution)
{
    vector<vec3> points;
    vector<uint> indices;
    for(uint i=0; i< u_resolution; i++)
    {
        float u = (float(i) / float(u_resolution - 1)) * M_PI - M_PI / 2.f;
        for(uint j=0; j < v_resolution; j++)
        {
            float v = float(j) * 2.f * M_PI / float(v_resolution - 1);

            points.push_back(vec3(cos(u) * cos(v), cos(u) * sin(v), sin(u)) * radius);
        }
    }

    for(uint i=0; i < u_resolution - 1; i++)
    {
        for(uint j=0; j < v_resolution; j++)
        {
            // Grab the 4 points in a quad face in the sphere
            uint p00 = (i * (v_resolution) + j);
            uint p01 = (i * (v_resolution) + ((j + 1) % v_resolution));
            uint p10 = ((i + 1) * v_resolution + j);
            uint p11 = ((i + 1) * v_resolution + ((j + 1) % v_resolution));

            assert((i + 1) < u_resolution);
            assert(p00 < points.size());
            assert(p01 < points.size());
            assert(p10 < points.size());
            assert(p11 < points.size());
            assert(
                p00 != p01 && p00 != p11 && p00 != p10 &&
                p01 != p10 && p01 != p11 &&
                p10 != p11
            );

            // Add the first triangle
            indices.push_back(p00);
            indices.push_back(p01);
            indices.push_back(p11);

            // Add the second triangle
            indices.push_back(p00);
            indices.push_back(p11);
            indices.push_back(p10);
        }
    }

    //for(auto& v : points) cout << to_string(v) << endl;
    //cout << endl;
    return {points, indices};
}

```

And immediately after calculate the normals like this:

```
std::vector<glm::vec3> GenerateNormals(const std::vector<glm::vec3>& vertices,
    const std::vector<uint>& indices)
{
    assert(indices.size() % 3 ==0);

    map<uint, vector<vec3>> normal_map;
    for(uint i=0; i < indices.size(); i += 3)
    {
        vec3 v1 = vertices[indices[i]];
        vec3 v2 = vertices[indices[i + 1]];
        vec3 v3 = vertices[indices[i + 2]];

        vec3 d1 = v2 - v1;
        vec3 d2 = v3 - v1;

        assert(indices[i] < vertices.size());
        assert(indices[i + 1] < vertices.size());
        assert(indices[i + 2] < vertices.size());
        assert(indices[i] != indices[i+1] && indices[i] != indices[i+2] &&
            indices[i+1] != indices[i+2]);

        if(abs(dot(d1, d2)) < 0.0001)
        {
            d1 = v3 - v2;
            d2 = v1 - v2;
        }

        if(abs(dot(d1, d2)) < 0.0001)
        {
            d1 = v1 - v3;
            d2 = v2 - v3;
        }

        vec3 normal = normalize(cross(d1, d2));

        normal_map[indices[i]].push_back(normal);
        normal_map[indices[i + 1]].push_back(normal);
        normal_map[indices[i + 2]].push_back(normal);
    }

    assert(normal_map.size() == vertices.size());
    std::vector<glm::vec3> normals(vertices.size());
    for(auto& pair : normal_map)
    {
        uint index = pair.first;
        vector<vec3> v_normals = pair.second;

        vec3 normal(0);
        for(auto& n : v_normals)
            normal += n;

        normal /= float(v_normals.size());
        normals[index] = normal;
    }
    assert(normals.size() == vertices.size());
    return normals;
}

```

Note: I know that I could just calculate the normals by taking the difference between the current vertex and the center of the sphere. These 2 functions I plan to use as templates to code other paramtetric surfaces, so that's not a solution for me I am trying to understand what I did wrong more than trying to fix it.

I have added lots of assertions through the code to try to catch the issue and I have also stepped through gdb but I can't seem to find the issue.

**Tags:** c++, geometry, parametrization


---

## 1 answers

### Answer by username_1 on 2020-04-16. Score: 3 | Accepted answer

The problem with your code is, that it produces a lot of degenerated triangles. The not so obvious ones are at the top and bottom. The obvious ones are at the "wrapping line".

So what exactly is the problem? Each of your faces is a rectangle that consists of 2 triangles and 4 points. The 2 triangles share 2 points and each one has 1 individual point. But at the bottom/top 2 points are identical, so your rectangle degenerates to a triangle. As a result, one triangle degenerates to a line because of the duplicate points.

Now, why is this problematic for you? It is this line, that causes the trouble:

```cpp
vec3 normal = normalize(cross(d1, d2));

```

The cross product of a vector with itself is the zero vector. If you normalize a vector, you divide by its length, which is 0 in this case. So this is where your `NaN`s come from. Until now, I just spoke about the top and bottom, but why is it happening at the "wrapping line"? The problem is a result of your index calculation:

```cpp
    for(uint i=0; i < u_resolution - 1; i++)
    {
        for(uint j=0; j < v_resolution; j++)
        {
            // Grab the 4 points in a quad face in the sphere
            uint p00 = (i * (v_resolution) + j);
            uint p01 = (i * (v_resolution) + ((j + 1) % v_resolution));
            uint p10 = ((i + 1) * v_resolution + j);
            uint p11 = ((i + 1) * v_resolution + ((j + 1) % v_resolution));
   ...

```

The `j` loop is running until `v_resolution`. So `j+1` gets problematic. I know your intention here is, that you need the triangles from the "other side", to calculate your normals correctly. That is why you performed the `% v_resolution` to get the corresponding points. But the problem is, you are not getting the points you want. With your code, you are including the last and first point of a "horizontal line", but they are identical in your case. The result is that you get a face that degenerates to a line, which means, that you will divide by 0 during normalization of both triangles surface normals.

A quick way to fix the `NaN`s at the "wrapping line" would be to make a temporary variable and store `j+1` in it. Then check if it is equal or higher than `v_resolution`. If `true`, increase it by one. Use this variable everywhere you used `j+1` during the index generation. However, this will still cause an issue at the wrapping line, since your start and end vertices are not the same and you only considered "both sides" for the last vertices of a horizontal line, but not for the first. This will lead to incorrect normals for the start vertices and you should see a jump in the shading. You might fix it, by just copying the normals of the last vertices to the first ones.

Also, you still need to address the problems at the top and bottom. This can be fixed by making a special loop only for top and bottom triangles. However, these are suboptimal fixes. A better approach would be to write code that does not produce duplicate points since it lowers the risk of creating degenerated triangles and is more memory efficient.

> **Comments:**
> **username_0**: Ah yes I am an idiot, the vertical band can be fixed by dividing by `v_resolution` instead of `v_resolution-1` To prevent the last set of vertices from being duplicated.   I am still thinking about the polar ones  
> **username_1**: Well, you need to treat them separately in their own loop. Additionally, try to avoid duplicating the polar points. Even without the degenerated triangles, the normals are incorrect, because each duplicate polar point calculates it's normal only from 2 adjacent triangles instead of all (`v_resolution`) triangles that are connected to the pole.  
> **username_0**: But separate loops are fugly! But yes I think I'll hve to go with that  

---

