search

ousnius / nifly

C++ NIF library for the Gamebryo/NetImmerse File Format
GNU General Public License v3.0
54 stars 21 forks source link

Copy particleNorms before resizing? #13

Closed renngar closed 3 years ago

renngar commented 3 years ago

This is more a question than an definite bug report or PR.

In OptimizeFor, shouldn't the normals, if they exist, be copied to the particleNorms before particleNorms is resized? Or is the intention that if the number of normals and vertices differ then all of the particleNorms are populated with <1, 0, 0>?

You can see what I mean on this branch, https://github.com/renngar/nifly/commit/db550f23c7fd787fe45d739f11412bc75aaf93bd.

My expectation was that if there were more vertices than normals, the particleNorms would be extended using <1, 0, 0> and if there were fewer vertices, then normals would be copied to particleNorms, which would be truncated to match the length of vertices. I don't know if my assumption is correct or not, so I thought I would ask.

ousnius commented 3 years ago

@renngar particleNorms has to have the same size as particleVerts (and vertices). In the case of there being a difference, your edit would keep the existing normals where possible and only added ones would default to (1,0,0) instead of all of them like in the original code.

In my opinion, the original approach is more correct, as the sizes should not differ and if they do, you cannot expect the normals to make sense so they should all be initialized.