Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2615713007:
Change LayoutUnit to use saturated_cast (Closed)
DescriptionChange LayoutUnit to use saturated_cast
Change the LayoutUnit implementation to use base::saturated_cast instead of WTF
::clampTo as the former is much more efficient. Also removes clampToLayoutUnit.
R=pdr@chromium.org
Review-Url: https://codereview.chromium.org/2615713007
Cr-Commit-Position: refs/heads/master@{#443367}
Committed: https://chromium.googlesource.com/chromium/src/+/031e22e5ca256199390180d5ffbb9cf182c36664
Patch Set 1 #Patch Set 2 : Fix includes #
Messages
Total messages: 27 (17 generated)
The CQ bit was checked by eae@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by eae@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== sat_cast BUG= ========== to ========== Change LayoutUnit to use saturated_cast Change the LayoutUnit implementation to use base::saturated_cast instead of WTF ::clampTo as the former is much more efficient. Also removes clampToLayoutUnit. R=jschuh@chromium.org ==========
eae@chromium.org changed reviewers: + jschuh@chromium.org
Description was changed from ========== Change LayoutUnit to use saturated_cast Change the LayoutUnit implementation to use base::saturated_cast instead of WTF ::clampTo as the former is much more efficient. Also removes clampToLayoutUnit. R=jschuh@chromium.org ========== to ========== Change LayoutUnit to use saturated_cast Change the LayoutUnit implementation to use base::saturated_cast instead of WTF ::clampTo as the former is much more efficient. Also removes clampToLayoutUnit. R=pdr@chromium.org ==========
eae@chromium.org changed reviewers: + pdr@chromium.org - jschuh@chromium.org
LGTM Can you check https://chromeperf.appspot.com/group_report?rev=[change #] when this lands? It'll be interesting if we can see it on a profile.
On 2017/01/11 21:35:37, pdr. wrote: > LGTM > > Can you check https://chromeperf.appspot.com/group_report?rev=[change #] when > this lands? It'll be interesting if we can see it on a profile. Good idea, will do!
The CQ bit was checked by eae@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by eae@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484248884179840,
"parent_rev": "30c4e21c362f23e2a40c5bba0483e59c3fc672ff", "commit_rev":
"031e22e5ca256199390180d5ffbb9cf182c36664"}
Message was sent while issue was closed.
Description was changed from ========== Change LayoutUnit to use saturated_cast Change the LayoutUnit implementation to use base::saturated_cast instead of WTF ::clampTo as the former is much more efficient. Also removes clampToLayoutUnit. R=pdr@chromium.org ========== to ========== Change LayoutUnit to use saturated_cast Change the LayoutUnit implementation to use base::saturated_cast instead of WTF ::clampTo as the former is much more efficient. Also removes clampToLayoutUnit. R=pdr@chromium.org Review-Url: https://codereview.chromium.org/2615713007 Cr-Commit-Position: refs/heads/master@{#443367} Committed: https://chromium.googlesource.com/chromium/src/+/031e22e5ca256199390180d5ffbb... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/031e22e5ca256199390180d5ffbb...
Message was sent while issue was closed.
On 2017/01/12 21:34:30, commit-bot: I haz the power wrote: > Committed patchset #2 (id:20001) as > https://chromium.googlesource.com/chromium/src/+/031e22e5ca256199390180d5ffbb... BTW this is not neutral as it implies changes in behaviour in some cases. For example when the passed value overflows clampTo() clamps it to numeric_limits::min numeric_limits::max. Nevertheless saturated_cast as stated by the doc translates NaN to 0.
Message was sent while issue was closed.
On 2017/02/20 09:52:41, svillar wrote: > On 2017/01/12 21:34:30, commit-bot: I haz the power wrote: > > Committed patchset #2 (id:20001) as > > > https://chromium.googlesource.com/chromium/src/+/031e22e5ca256199390180d5ffbb... > > BTW this is not neutral as it implies changes in behaviour in some cases. For > example when the passed value overflows clampTo() clamps it to > numeric_limits::min numeric_limits::max. Nevertheless saturated_cast as stated > by the doc translates NaN to 0. Indeed, thanks for pointing that out Svillar! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
