zturtleman / mm3d

Maverick Model 3D is a 3D model editor and animator for games.
https://clover.moe/mm3d
GNU General Public License v2.0
114 stars 22 forks source link

getNormal and getFlatNormal are inconsistent #116

Open m-7761 opened 4 years ago

m-7761 commented 4 years ago
        bool getNormalUnanimated(unsigned triangleNum, unsigned vertexIndex, double *normal)const;
        bool getNormal(unsigned triangleNum, unsigned vertexIndex, double *normal)const;
        bool getFlatNormalUnanimated(unsigned triangleNum, double *normal)const;
        bool getFlatNormal(unsigned triangleNum, double *normal)const;

Just to make a note for this. I've changed the signature for these to use double and have made them to be usable to retrieve animated normals. Today I noticed "getVertexCoordsUnanimated" and so switched the semantics to work the same way.

The problem (reason for this issue) is getFlatNormal was calculating its normal. I've changed it to try to copy the normal if the valid flag is present.

Whereas getNormal doesn't calculate and neither does it validate.

So, I think either they should be validating, or neither should consult the validity state and neither should they calculate.

(Note that I don't know if the way of wholesale invalidating/calculating normals will be the way of the future as it is now. I expect only affected normals will be repaired, but I think that doing that as soon as they change won't be desirable either, so most likely validity will be tracked for each face and a counter may be required to know when every face is valid too. Ideally any operation/algorithm that invalidates normals should be cleaning up after itself instead of leaving the model in an invalid state.)

Right now I can't make a decision, so I'm just leaving this here so it won't be forgotten.