Bug #1358
closedround_str in WebUtils.c dysfunctional for larger than 6 digits
0%
Description
This function
char *round_str(double d, int digits, char *buf) {
static constint exp[] = { 1, 10, 100, 1000, 10000, 100000, 1000000 };
long long i = static_cast<longlong>(d * exp[digits] + (d > 0 ? 0.49 : -0.49));
lltoa(i, buf);
char *num = buf;
if (num[0] == '-')
++num;
int len = std::strlen(num);
if (len <= digits) {
int shift = digits + 1 - len;
for (int i = digits + 1; i >= 0; --i) {
if (i >= shift)
num[i] = num[i - shift];
else
num[i] = '0';
}
len = digits + 1;
}
int dotPos = (std::max)(len - digits, 0);
for (int i = digits + 1; i >= 0; --i)
num[dotPos + i + 1] = num[dotPos + i];
num[dotPos] = '.';
return buf;
}
does not work if there are more than 6 digits requested. The broken lines are
static constint exp[] = { 1, 10, 100, 1000, 10000, 100000, 1000000 };
long long i = static_cast<longlong>(d * exp[digits] + (d > 0 ? 0.49 : -0.49));
For some reason, there is a hard coded array of powers of ten. If a programmer passes, say, 7 as digits, this won't work. It wasn't segfaulting or anything, it was just giving out totally garbage data. Ideally this would just be something like
long long i = static_cast<longlong>(d * pow(10, digits) + (d > 0 ? 0.49 : -0.49));
If the array was being used for speed purposes, at least do something like
static constint exp[] = { 1, 10, 100, 1000, 10000, 100000, 1000000 };
long long i = static_cast<longlong>(d * (digits <= 6 ? exp[digits] : pow(10, digits)) + (d > 0 ? 0.49 : -0.49));
Files
Updated by Wim Dumon over 12 years ago
- File floatrendering.patch floatrendering.patch added
Hello,
- round_str() is internal API. How did you manage to pass '7' as parameter to round_str()? We carefully only use it with numbers <= 6.
- we've had enough complaints on round_str()s behavior by now to do something about it.
I propose to apply the patch attached hereto. I have done a few comparisons (MSVS 2010), and concluded that:
1. old round_str() is about 2 times faster than boost.karma
- boost.karma is about 2 times faster than sprintf
- old round_str() does not properly handle special float values (nan, infinity)
- floats will always be rendered with proper precision when using boost.karma
remaining questions:
- do we always generate correct floats with the new implementation?
- does the performance of the extra test in round_str() matter?
- is the performance of boost.karma acceptable when compiler is used without optimization?
There's other people looking at the issue for rendering very large amounts of floats without conversing them to strings, primarily for WebGL purposes.
Can you test the patch applied hereto? I'll put it in git if you don't find problems with it.
Wim
Updated by Samuel - over 12 years ago
I'm using a WDoubleSpinBox. setValue(double value) calls textFromValue() which then calls round_str(double d, int digits, char *buf). I had been trying different numbers of decimals to display on my WDoubleSpinBox via setDecimals(int precision), which allows decimals higher than 6.
I wasn't able to test the patch, but it looks good to me.
Updated by Wim Dumon over 12 years ago
Samuel,
That's the root cause of the problem - round_str() cannot be used there. The correct patch for that problem is below, I'll make sure it gets into the public git.
WString WDoubleSpinBox::textFromValue() const
{
#ifndef WT_TARGET_JAVA
std::stringstream ss;
ss.precision(precision_);
ss << std::fixed << std::showpoint << value_;
std::string result = ss.str();
#else
char buf[30];
std::string result = Utils::round_str(value_, precision_, buf);
#endif // WT_TARGET_JAVA
Updated by Koen Deforche over 12 years ago
- Status changed from New to Resolved
- Assignee set to Koen Deforche
- Target version set to 3.2.3
Updated by Koen Deforche about 12 years ago
- Status changed from Resolved to Closed
Fixed in Wt 3.2.3 RC1.