Problems parsing geometries with a MTL file and no colour, or [w] coordinates.

[aleshaleksey]

Backstory: For reasons of needing more precision, I have forked this library and made some modifications to allow it to parse coordinates as something other than f32. I am not complaining about this library using f32, by the way, I understand the benefits of not using twice the number of bytes when you don't need it. However, I did find a couple of bugs (or maybe just annoying features?) in parse_floatn and in the two add_vertex functions. I have dealt with them on my fork, because for my use case this was not a tenable state of affairs, and thought that it may also help the development of the main repository if I passed on a brief summary of my findings an an "FYI". If this is something you already know about and it is not a problem for your purposes, all the better.

The first "bug": In short, if !v_color.is_empty() then if you have no vertex colours you will hit (v * 3 + 2 >= v_color.len()) and throw an LoadError::FaceColorOutOfBounds error. However, if you have a colour in the MTL file, and no vertex colours in the OBJ file, you will have a v_color length of either 3 or 4, so you will always hit this error. I do not know if this is considered a bug or a feature, and if it is a bug, whether the library design assumes that it should be fixed here, or by changing something elsewhere. On my side, I dealt with it in this function by introducing an extra check.

The second "bug" occurs if you have a geometry that contains the optional [w] coordinate (OBJ according to wikipedia) the parser with try to use parse_floatn to parse it as a colour value. However, because there is only one value there (and not three), it means that you end up with a short colour vector and then everything crashes and burns. This seems to be because the parse_floatn function adds the parsed values to the to the container and only then checks the number of values added. I dealt with this by having a temporary holding container that only dumped the values into the main container after the check was made (and only if the result was `true). There are clearly more efficient ways of dealing with it, if, in your expert opinion, it needs to be dealt with at all.

Either way, we've found this crate useful so far and I wish you all the best with future developments.

(Version used: 3.2.3, commit: 713dd4c)

[Twinklebear]

Thanks for reporting these issues @aleshaleksey ! The first issue sounds a bit odd to me, I wouldn't expect if you only had colors in the MTL file that you'd hit this, however; I could see this being an issue if there was a mix of objects with per-vertex colors and objects without them. Then you'd have !v_color.is_empty() and if the objects using using MTL colors only use higher vertex indices than those using per-vertex colors, it would crash. Maybe a workable option here would be to just not throw the error, and assume that if it was out of bounds maybe the vertex/face doesn't user per-vertex colors?

For the 2nd one, I see how that one would be an issue, tobj assumes positions are always vec3f which would be wrong if you have the w coordinate. I'm not quite sure on changing this though, it would need to rework a lot of the library to now handle both 3 & 4 component positions and track that throughout (and maybe there's some mix of those with w and without?).

It's good to hear the crate has been useful!

[aleshaleksey]

Firstly apologies for the slow reply.

We have used two types of object, the first was basically random things from the internet, the second is from our own obj writer which produces a simple and very uniform output, which I shall describe below:

The objects we are using have v tags for vertices (always with w, so 4 coordinates), vn tags for normals (always 3 coordinates) and f tags for triangle face indices (always 3 coordinates). The .obj file has no vertex colours, and no colours at all for that matter. So in that sense the object is both simple and regular. The .mtl file always looks like this (but values obviously differ):

newmtl mtl
Ka 0.2 0.6 0.2
Kd 0.2 0.6 0.2
Ks 0.0 0.0 0.0
d 1

So again, there is nothing complex here.

Now, On !v_color.is_empty(): This was first discovered with the random internet objects, but also occurred with our OBJ files because of the w bug thing. I have not investigated this further, because my duct-tape solution is "good enough" for my purpose, but I did have the thought that maybe the colour from the mtl file somehow gets added to the v_color vector? My other thought is that our objects have multiple s-type sub-objects, and tobj seems to merge them. Because I have not investigated the mechanism by which this is done, so I cannot exclude the possibility that somehow the colour from the mtl file might find its way into the v_color vector. Although this would surprise me. If for some files it is caused by mixing coloured and uncoloured vertices, then that would be a little bit awkward to solve, but I think at least some of the possible solutions would be simple (eg, as the simplest and a fairly inefficient solution, always writing a default/mtl colour tov_color if the vertex has none).

On the w/vec3f problem: I would agree that this would require a rework of the library. But I do not think it would necessarily be a major rework. If I am not mistaken, for v, there are are 4 possible lengths: 3 => Position only. 4 => Position + w. 6 => Position + colour. 7 => Position + w + colour. So if we assume that w is always discarded, The biggest change is to swap our the parse_floatn for something that only recognises lines with 3, 4, 6, or 7 floats, and parses them appropriately. This should add only a very small amount of complexity to the current implementation. (I am not sure what problems it would cause, however, but with my limited understanding of this crate, I don't see too many problems).

Something I could suggest, is adding a few more test geometries of different "flavours" for catching this kind of thing, but then things could very quickly stop being "tiny".

Either way, that's as far as I have thought about this problem. Again, I wish you all the best with this project.