-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rounding issues with Twitter Bootstrap #352
Comments
+1 |
2 similar comments
+1 |
+1 |
+1 |
I don't think there is anything I can do about this. I can't increase the precision of php's numbers. But, while investigating I noticed that a handful of the numbers in bootstrap's |
That's correct. It looks like PHP's rounding function isn't accurate enough. Could we perhaps add in a naughty way of doing things like |
what about bcmath? http://php.net/manual/en/book.bc.php |
@unforeseen The differences are of much smaller magnitude, wouldn't subtracting 0.0001 just make things a lot worse? Also, in the case of broken numbers, the examples I looked at didn't involve the round function, it was just the result of the built in math operators. @Asare-w I suppose something like this is a possibility but I don't think many people have this extension installed. |
Like I say, it's a quick hack. |
bcmath is bundled with php since 4.0.4 (http://www.php.net/manual/en/bc.requirements.php), and by default php is configured with --enable-bcmath. so bcmath should by widley supported |
You could also do a simple check for the existance of GMP or BCMath and use those when available with fallback support. For the sake of rounding accuracy, this is a pretty big deal. |
If it's not, you could polyfill it but it's a bit of a faff.
|
ah, I didn't know that @Asare-w |
I can take care of it if someone can vaguely point me to the right area so I don't have to trace every bit of the app? Or is it any time there is a piece of math? |
The numeric operators are handled here: https://github.com/leafo/lessphp/blob/master/lessc.inc.php#L1525 |
Thanks. I'll give it a go later tonight / tomorrow. |
Just because I just tried this just a moment ago, I thought I'd let you know. I tried BCMath, but it still returns results which mess up the layout (which I'm then assuming are incorrect). I'm not exactly sure what the reason is, but either php just always rounds certain numbers up, or it's just not as simple as I thought it was (which I dearly hope is the case). |
Thanks for the update. I switched over to http://twitter.github.com/recess/ and it works fine. So it's definitely a php rounding error. Maybe http://www.php.net/manual/en/book.gmp.php might work. I'll still give a few things a go later. |
Can you try changing the 'precision' setting in php.ini from the 14 (default) to 16? Or use:
|
Hah. Ignore this and goto my next comment. So I found this issue interesting so I looked into it further. Node/Javascript uses 64bit Double Precision Floating Point variables for its numbers. While bcmath is accurate, it isn't going to return the data as would Node because Node numbers are rounded to the nearest (1+(x / 2^52)) * 2^y. Just for fun I attempted to recreate how Node calculates its numbers in pure php: WARNING: This is a very inefficient solution. This will increase compile time by several magnitudes. That being said, I compiled bootstrap and the results are as good as they can get. Except for the fact I can't figure out why Node sometimes shows 16 vs 17 significant digits. Not sending a pull request because I can't imagine this actually being used in production. |
That's some nice debugging! I ended up having to ditch lessphp entirely and instead call node lessc from php. |
It would be great if this solution could somehow be included in the main lessphp project. |
@marten-seemann agreed - it could be config option |
Well, turns out PHP uses 64bit also for its floats. Don't know why I didn't check that first. @robocoder is right and ini_set would return the appropriate values. The same problem occurs in my hack occurs in this solution too, node sometimes returns 16 ~ 17 significant chars. |
I'm using lessphp to import bootstrap.less. This is the original (part of) the css file, taken from the example:
While other processors (like node js less) produce the correct result, with lessphp I get:
Which is slightly different, but enough to make my layout wrong. See these two images:
Original
With lessphp
The text was updated successfully, but these errors were encountered: