Opened 8 years ago

Last modified 11 months ago

#1325 new defect

[PATCH] Replace ecvt() in FCollada with something else

Reported by: historic_bruno Owned by:
Priority: Nice to Have Milestone: Work In Progress
Component: Core engine Keywords: patch
Cc: Patch: Phab:D2399

Description (last modified by nf)

FCollada uses the deprecated POSIX function ecvt to convert floats to strings, but it's not available on FreeBSD and it would be nice to have a portable replacement. The result needs to be compatible with the COLLADA schema which uses XML's xs:double type for floats.

Possible solutions:

It's a good idea to avoid ecvt implementations that depend on dtoa, because it's also not available on FreeBSD and there are numerous buggy versions of that code around which apparently break with compiler optimizations.

Attachments (2)

comparison-format (14.6 KB ) - added by Markus 7 years ago.
compare some output formattings
canonical-xsdouble.patch (5.1 KB ) - added by Markus 7 years ago.
FUStringBuilder.hpp exists twice

Download all attachments as: .zip

Change History (13)

comment:1 by historic_bruno, 8 years ago

We could do what OpenCOLLADA does, using an implementation of dtoa. The main benefit is we know it works for OpenCOLLADA.

The problem with sprintf and similar, in my opinion, is they're just not exacting enough to meet the spec.

Version 0, edited 8 years ago by historic_bruno (next)

comment:2 by Markus, 7 years ago

I just copied the FloatToString and the corresponding append function to a small test program. For me it does not support negative exponents and the format does not conform to the "canonical representation" of the document linked above.

What are these functions actually used for? Only development?

Achieving a "canonical representation" should be possible with some simple math and formating... I will try this WE.

comment:3 by historic_bruno, 7 years ago

I don't believe we use these functions for anything at the moment, I assume they are for exporting COLLADA documents or something, but if we do in the future, it would be preferable to not have them broken :)

FWIW, it looks like the FreeBSD port of 0 A.D. copy-pasted some code from a forum, but I don't know if they actually confirmed the license is compatible with FreeBSD, so that's naughty and I also don't know that the code works beyond compiling.

Last edited 7 years ago by historic_bruno (previous) (diff)

by Markus, 7 years ago

Attachment: comparison-format added

compare some output formattings

by Markus, 7 years ago

Attachment: canonical-xsdouble.patch added

FUStringBuilder.hpp exists twice

comment:4 by Markus, 7 years ago

Keywords: patch review added
Summary: Replace ecvt() in FCollada with something else[PATCH] Replace ecvt() in FCollada with something else

comment:5 by historic_bruno, 7 years ago

Milestone: BacklogAlpha 14

comment:6 by historic_bruno, 7 years ago

Interesting, so it looks like the ecvt implementation only keeps 6 digits and anything on the order of 1e-8 or smaller becomes 0. That's not canonical xs:double but it's valid? I guess we could check if Collada importers can also handle negative exponents without trouble.

comment:7 by historic_bruno, 7 years ago

Milestone: Alpha 14Alpha 15

comment:8 by Jorma Rebane, 7 years ago

Keywords: review removed

Taking a look at this patch, I think we're really missing the point, which is to print valid numbers (that make sense) into a Collada file, no?

In this case, any value that is ridiculously small (for example, smaller than +/- 0.00001) should be considered as 0.0; For that matter, ridiculous large float values should also be ignored and set to NAN - because they're obviously invalid vertex coordinates.

The goal is not to produce a float print function that works for every universal solution - the goal is to get valid and reasonable output. In this case NAN, +/- 0.00001 to +/i 10000000.0.

In this case, we can also get away with sprintf(buffer, "%d", ...);

comment:9 by leper, 7 years ago

Milestone: Alpha 15Backlog

comment:10 by nf, 11 months ago

Description: modified (diff)

Hi folks,

I have successfully built fcollada using

It uses musl libc (MIT licensed) implementation of ecvt(). Let me know what you think.

comment:11 by elexis, 11 months ago

Milestone: BacklogWork In Progress
Patch: Phab:D2399
Note: See TracTickets for help on using tickets.