[fixed] fast_atof not understanding comma decimal separator

[hendu]

fast_atof.h, line 318:

Code: Select all

        if ('.' == *in)

This should be changed to also check for the comma, ','. Otherwise mtl files such as this are not read right:

# Material file for pipewall.obj

newmtl gray
Ns 0
d 1
illum 2
Kd 0,000000 0,000000 0,000000
Ks 0,000000 0,000000 0,000000
Ka 0,200000 0,200000 0,200000

newmtl brown
Ns 0
d 1
illum 2
Kd 0,320000 0,160000 0,000000
Ks 0,000000 0,000000 0,000000
Ka 0,000000 0,000000 0,000000

[CuteAlien]

Hm, which tool produces such .obj files? I'm not too keen on changing fast_atof for this as it works similar to atof in that regard (it doesn't work the same when it comes to leading white-spaces as I just noticed ...).

On first view I would say that obj file has been written wrong.

[hendu]

Misfit Model 3d.

I read through a couple .OBJ specs, nowhere does it say what the decimal separator is supposed to be. This is a locale-varying thing, in my locale comma is the decimal separator, and here atof parses the comma and not the dot.

Editors being locale-aware, printing a float out uses the locale's separator by default.

Code: Select all

#include <stdlib.h>
#include <stdio.h>
#include <locale.h>
 
int main() {
        /* Instead of forcing LANG=C, use user's locale */
        setlocale(LC_ALL, "");
 
        printf("1,2 to float: %f\n", atof("1,2"));
        printf("1.2 to float: %f\n", atof("1.2"));
        return 0;
}
 

[CuteAlien]

Well, locale makes sense for atof, but that would just mean that with the wrong local active nearly every .obj (or .mtl) would not load. And yeah - .obj format only mentions floating-point, but that's still the first .mtl I've seen which _didn't_ use a point. So I guess the correct solution would rather be not to use atof or fast_atof at all in the .obj loader but use an own function for that. Still not really nice, a workaround for a single editor which messes up export (and yes - I think that is messing up even if it's not defined - the dot is used in all examples in the specification).

Adding a check for all comma and point always to fast_atof is maybe a also a solution, just feels even worse to me (1 additional check for all characters before the point - not really what you want in a version called "fast"). I must admit I don't even know if those are the only 2 possiblities in floating points - have to check (could be some languages use even other signs).

[hybrid]

File formats should always use the C locale

[CuteAlien]

Hm, sorry, I tried using it, but not a tool I can learn using enough in 5-10 minutes to get some test-models which I can export. And Misfits doesn't have any testmodels on their homepage (somewhat surprising for a modeling tool...).

I think it's a bug on their side, so maybe report it to them. As workaround you can maybe set the local to another value for this tool. I have to set for example LC_ALL to en_US.UTF-8 for codeblocks to avoid getting an ugly german/english mix (nothing worse than german error-messages).

[hendu]

It's a dead app; the author quit ~a year ago. Yes, I've been using LANG=C to work around it.

edit: Here you go, an untextured sphere .obj: http://kiwi6.com/file/il709bqem0

If you load it in the current meshviewer, it's not quite round due to everything after the comma being ignored, in effect rounding it to ints.

[CuteAlien]

Thanks. Still thinking what to do with that - can't decide. Making it possible to set the character used for decimal point in fast_atof.h would be another solution. Would even be nice, but in this case the problem is that you probably would want to set more than once character (as you likely still want the point as it's used everywhere else) and that again would slow down fast_atof. I hate this problem...

[CuteAlien]

OK, I tested it and it doesn't make a noticeable speed difference. So in svn trunk you can now set the characters used for the decimal point. For example:

Code: Select all

 
irr::core::LOCALE_DECIMAL_POINTS = ".,";
 

Unfortunately core::string is using it's own float -> string conversion which depends on locale settings. I guess we have to adapt that as well some day, but I'm not sure right now why fast_atof is not used there and don't dare modifying that for 1.8.