NormAngleAccum::angle is unused

[m-7761]

https://github.com/zturtleman/mm3d/blob/f21f69ae33cbee6f3a93de83ac8f0765e059934c/src/libmm3d/model.cc#L4152

This angle is calculated but not factored into the normals calculation:

// Accumulate for smooth normal, weighted by face angle

I share since I don't know if this is by design or oversight.

I'm currently attempting to refactor this normals stuff since it suffers from a lot of the same insanity as the rest of the library.

I can't tell if the vertex normals need to be stored in the triangles or not (are they different or not) but hopefully with some more study I will reach a definite idea. It causes problems that the vertex normals aren't stored identically for frame animations. I don't know if they really need to be stored at all. At least md2 and md3 formats seem to want vertex normals. That means really they should be busting up their data probably... which I think maybe they do. In which case they should be using the triangle's normals instead.

I think the vertex normals are just intermediary. I think probably it would be better to not store them except maybe as an optimization if that makes sense... but the library as is is completely suboptimal, so it's just noise.

[m-7761]

Follow-up: It looks like m_vertexNormals isn't needed, since it differs from m_finalNormals only by the smoothing factor.

The MM3D and MS3D formats write m_vertexNormals but it seems like writing m_finalNormals would be improvement in the latter's case, and the former doesn't use the information for reading, so same. (If removing smoothing mattered it could be calculated without prior to writing.)

I think a better way is to store a std::string or something (small string optimization) in Vertex to hold a list of triangles. calculateNormals currently builds a 2D array of std::vector for this that is not good. That list (or two if angles are stored) can be kept up-to-date.

Doing a full refresh of normals is probably not a good long term idea, so that would make it easier to do spot repairs instead.

Note, currently refreshing normals for frame animations also refreshes every frame even though you're probably only looking at one. That's how I ended up down this rabbit hole.

I worked on a system for that yesterday but I'm afraid it's going to be a wash. I don't think it makes sense to keep normal data for animation frames, especially with interpolation. Either way it seems like using too much memory. It would be simpler to do the same for skeleton animation. That would smooth correctly. (The redirection step could be removed completely if plugins calculated their own normals prior to writing.)

(EDITED: Actually I think that will be required because the getFrameAnimVertexNormal API needs to be removed. It's pretty useless anyway. MM3D doesn't write it. Arguably it shouldn't write normals at all in that case.)

[m-7761]

The MD3 filter doesn't produce ideal normals over the course of the animation owing to how Mesh locks them in.

To remove "getFrameAnimVertexNormal" I cooked up the following solution that refreshes the mesh normals. It should normally be more efficient than doing a nasty/inaccurate look up I think, and at least averages them.

To make this work the write filter has to use setCurrentAnimationFrame manually, so I've made filtermgr.cc to disable undo (like it does for reading) and save/restore the animation state. In this new system if you need normals you must generate them, which setting the animation does automatically.

These exporters will need to be updated to take advantage of interpolation. It's not required unless the input doesn't define all of its frames.

//2020: Have to change how this works in a pinch.
//bool Md3Filter::getVertexNormal(Model *model, int groupId, int vertexId, float *normal)
void Md3Filter::resetVertexNormals(Model *model, Mesh &mesh)
{
    /*INSANITY
    int_list tris = model->getGroupTriangles(groupId);
    int_list::iterator tri;
    for(tri = tris.begin(); tri!=tris.end(); tri++)
    {
        for(int n = 0; n<3; n++)
        {
            int vert = model->getTriangleVertex(*tri,n);
            if(vert==vertexId)
            {
                //https://github.com/zturtleman/mm3d/issues/109
                //model->getNormal(*tri,n,normal);
                model->getAnimNormal(*tri,n,normal);
                return true;
            }
        }
    }
    return false;*/
    for(auto&ea:mesh.vertices)
    {
        for(int i=0;i<3;i++) ea.norm[i] = 0; //HACK: averaging to be a little better.
    }
    for(auto&ea:mesh.faces)
    {
        for(int i=0;i<3;i++)
        {
            float n[3];
            model->getAnimNormal(ea.modelTri,i,n);

            auto &v = mesh.vertices[ea.v[i]];
            for(int i=0;i<3;i++) v.norm[i]+=n[i]; //HACK: averaging to be a little better.
        }
    }
}

[m-7761]

EDITED: Working on MD2 the way it computes normals is nasty (see code below) but at least its sqrt can be factored out. It should work in integer space. There must be efficient examples for doing this...

Anyway, the MD2 exporter uses the same vertices as MM3D so the only way I could manage was to manually compute vertex normals by averaging the faces' smoothed normals in the exporter. It doesn't use Mesh.

Now I believe there is no need to store them. And I think the changes to how they are calculated should be more accurate, but I think the exporters could do a better job too.

static unsigned bestNormal(float *norm)
{
    //2020: There has to a better way to do this!
    //float bestDistance = 10000.0f;
    float bestDistance = 10000*10000;
    unsigned bestIndex = 0;

    for(unsigned t = 0; t<MAX_QUAKE_NORMALS; t++)
    {
        float x = norm[0]-s_quakeNormals[t][0];
        float y = norm[1]-s_quakeNormals[t][1];
        float z = norm[2]-s_quakeNormals[t][2];

        //float distance = sqrt(x*x+y*y+z*z);
        float distance = x*x+y*y+z*z;

        if(distance<bestDistance)
        {
            bestIndex = t;
            bestDistance = distance;
        }
    }

    return bestIndex;
}

[m-7761]

Update: BACK TO THE ORIGINAL POST (sorry) I've reimplemented this with angle weights on my end. If the angle calculation is correct it seems like it just needs to be multiplied when accumulating A B C later on after comparing the max smoothing angle.

In that case it seems like a case of absent-mindedness?

Anyway, I've removed the nested vector part of calculateNormals and added face connectivity information to Vertex. I think the algorithm might be better turned inside out but I've so far just done the bare minimum to reproduce it without the vector.

The angles are stored in Triangle for each of the 3 sides alongside their face-normals. They aren't really needed after the calculation but in theory they could be reused. In thoery recalculating normals for the entire model isn't ideal, but in practice animations often change all of the normals/angles together so it's hard to say what's for the best. Right now there isn't the infrastructure for doing spotfixes on normals.

In case it's helpful below is the alternative angle code I developed. The delta3 part is just subtraction.

        double *v0 = m_vertices[tri->m_vertexIndices[0]]->m_absSource;
        double *v1 = m_vertices[tri->m_vertexIndices[1]]->m_absSource;
        double *v2 = m_vertices[tri->m_vertexIndices[2]]->m_absSource;
        /*REFERENCE
        calculate_normal(aacc.norm,v0,v1,v2);
        for(int i=0;i<3;i++)
        {
            tri->m_flatSource[i] = aacc.norm[i];
            acl_normmap[tri->m_vertexIndices[i]].push_back(aacc);
        }*/
        calculate_normal(tri->m_flatSource,v0,v1,v2);

        //New angle code (assuming degenerate values don't matter)
        double a[3][3],dp[3] = {};
        normalize3(delta3(a[0],v1,v0));
        normalize3(delta3(a[1],v2,v1));
        normalize3(delta3(a[2],v0,v2));
        for(int i=3;i-->0;)
        {
            dp[0]+=a[0][i]*-a[2][i];
            dp[1]+=a[1][i]*-a[0][i];
            dp[2]+=a[2][i]*-a[1][i];
        }
        for(int i=3;i-->0;) 
        tri->m_angleSource[i] = acos(dp[i]);