Add gfx::ToRoundedInt safe conversion method from float to int.

Add gfx::ToRoundedInt safe conversion method from float to int.

BUG=147395
R=sky@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=161137

[danakj, 2012-10-09 23:13:46]

updated to not use non-existant-pre-c99 std::round()

[tfarina, 2012-10-10 00:26:02]

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
File ui/gfx/safe_integer_conversions.cc (right):

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.cc:5: #include <cmath>
aren't you missing the include:

#include "ui/gfx/safe_integer_conversions.h"

?

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.cc:28: int ToRoundedInt(float value) {
Looks like this is what SymmetricRound() is trying to do.

After you land this, I'll get rid of SymmetricRound() and convert the calls to
gfx::ToRoundedInt().

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.cc:38: 
rm extra blank line.

[danakj, 2012-10-10 00:28:04]

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
File ui/gfx/safe_integer_conversions.cc (right):

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.cc:5: #include <cmath>
On 2012/10/10 00:26:02, tfarina wrote:
> aren't you missing the include:
> 
> #include "ui/gfx/safe_integer_conversions.h"
> 
> ?

looks like it.

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.cc:28: int ToRoundedInt(float value) {
On 2012/10/10 00:26:02, tfarina wrote:
> Looks like this is what SymmetricRound() is trying to do.
> 
> After you land this, I'll get rid of SymmetricRound() and convert the calls to
> gfx::ToRoundedInt().

cool :)

http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.cc:38: 
On 2012/10/10 00:26:02, tfarina wrote:
> rm extra blank line.

Done.

[tfarina, 2012-10-10 00:29:44]

http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_convers...
File ui/gfx/safe_integer_conversions.h (right):

http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.h:10: UI_EXPORT int ClampToInt(float value);
is it worth adding unittests for these functions?

http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.h:10: UI_EXPORT int ClampToInt(float value);
missing the include to ui/base/ui_export.h

[danakj, 2012-10-10 00:31:35]

http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_convers...
File ui/gfx/safe_integer_conversions.h (right):

http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.h:10: UI_EXPORT int ClampToInt(float value);
On 2012/10/10 00:29:44, tfarina wrote:
> missing the include to ui/base/ui_export.h

Done.

http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_convers...
ui/gfx/safe_integer_conversions.h:10: UI_EXPORT int ClampToInt(float value);
On 2012/10/10 00:29:44, tfarina wrote:
> is it worth adding unittests for these functions? 

I think the likelihood these functions will regress is marginal.

[sky, 2012-10-10 04:21:38]

LGTM - again add test coverage.

[danakj, 2012-10-10 16:05:12]

thanks! tests added.

[commit-bot: I haz the power, 2012-10-10 16:05:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11093031/7003

[commit-bot: I haz the power, 2012-10-10 16:37:23]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-10 16:37:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11093031/20

[commit-bot: I haz the power, 2012-10-10 18:49:38]

Change committed as 161137