Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(573)

Issue 11093031: Add gfx::ToRoundedInt safe conversion method from float to int. (Closed)

Created:
8 years, 2 months ago by danakj
Modified:
8 years, 2 months ago
Reviewers:
sky
CC:
chromium-reviews, cc-bugs_chromium.org, backer, piman, tfarina
Base URL:
http://git.chromium.org/chromium/src.git@gfx-scale
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : #

Total comments: 6

Patch Set 3 : #

Total comments: 4

Patch Set 4 : git co t #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+134 lines, -56 lines) Patch
M ui/gfx/point_conversions.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/gfx/rect_conversions.cc View 1 chunk +1 line, -1 line 0 comments Download
D ui/gfx/safe_floor_ceil.h View 1 chunk +0 lines, -16 lines 0 comments Download
D ui/gfx/safe_floor_ceil.cc View 1 chunk +0 lines, -29 lines 0 comments Download
A + ui/gfx/safe_integer_conversions.h View 1 2 3 1 chunk +6 lines, -3 lines 0 comments Download
A + ui/gfx/safe_integer_conversions.cc View 1 2 3 4 5 3 chunks +13 lines, -3 lines 0 comments Download
A ui/gfx/safe_integer_conversions_unittest.cc View 1 2 3 4 5 6 1 chunk +109 lines, -0 lines 0 comments Download
M ui/gfx/size_conversions.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/ui.gyp View 1 chunk +2 lines, -2 lines 0 comments Download
M ui/ui_unittests.gypi View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
danakj
8 years, 2 months ago (2012-10-09 23:05:46 UTC) #1
danakj
updated to not use non-existant-pre-c99 std::round()
8 years, 2 months ago (2012-10-09 23:13:46 UTC) #2
tfarina
http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_conversions.cc File ui/gfx/safe_integer_conversions.cc (right): http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_conversions.cc#newcode5 ui/gfx/safe_integer_conversions.cc:5: #include <cmath> aren't you missing the include: #include "ui/gfx/safe_integer_conversions.h" ...
8 years, 2 months ago (2012-10-10 00:26:02 UTC) #3
danakj
http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_conversions.cc File ui/gfx/safe_integer_conversions.cc (right): http://codereview.chromium.org/11093031/diff/1002/ui/gfx/safe_integer_conversions.cc#newcode5 ui/gfx/safe_integer_conversions.cc:5: #include <cmath> On 2012/10/10 00:26:02, tfarina wrote: > aren't ...
8 years, 2 months ago (2012-10-10 00:28:04 UTC) #4
tfarina
http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_conversions.h File ui/gfx/safe_integer_conversions.h (right): http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_conversions.h#newcode10 ui/gfx/safe_integer_conversions.h:10: UI_EXPORT int ClampToInt(float value); is it worth adding unittests ...
8 years, 2 months ago (2012-10-10 00:29:44 UTC) #5
danakj
http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_conversions.h File ui/gfx/safe_integer_conversions.h (right): http://codereview.chromium.org/11093031/diff/8001/ui/gfx/safe_integer_conversions.h#newcode10 ui/gfx/safe_integer_conversions.h:10: UI_EXPORT int ClampToInt(float value); On 2012/10/10 00:29:44, tfarina wrote: ...
8 years, 2 months ago (2012-10-10 00:31:35 UTC) #6
sky
LGTM - again add test coverage.
8 years, 2 months ago (2012-10-10 04:21:38 UTC) #7
danakj
thanks! tests added.
8 years, 2 months ago (2012-10-10 16:05:12 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11093031/7003
8 years, 2 months ago (2012-10-10 16:05:23 UTC) #9
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build. Your ...
8 years, 2 months ago (2012-10-10 16:37:23 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11093031/20
8 years, 2 months ago (2012-10-10 16:37:30 UTC) #11
commit-bot: I haz the power
8 years, 2 months ago (2012-10-10 18:49:38 UTC) #12
Change committed as 161137

Powered by Google App Engine
This is Rietveld 408576698