Skip to content

Commit

Permalink
Replace Saturated*() calls with Clamp*() templates
Browse files Browse the repository at this point in the history
This replaces the calls but leaves the saturated_arithmetic headers
where they are. Those will be moved in the next CL.
This is a reland of 191d114

TBR=eae@chromium.org

Bug: 672489
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I8e88f30fe3d22db3cac5bfcf13e83fc8dbaff870
Reviewed-on: https://chromium-review.googlesource.com/576137
Commit-Queue: Justin Schuh <jschuh@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Justin Schuh <jschuh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488966}
  • Loading branch information
jschuh authored and Commit Bot committed Jul 24, 2017
1 parent fef0806 commit 4a572da
Show file tree
Hide file tree
Showing 16 changed files with 54 additions and 194 deletions.
5 changes: 5 additions & 0 deletions base/numerics/clamped_math.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,11 @@ class ClampedNumeric {
value_);
}

// This method extracts the raw integer value without saturating it to the
// destination type as the conversion operator does. This is useful when
// e.g. assigning to an auto type or passing as a deduced template parameter.
constexpr T RawValue() const { return value_; }

private:
T value_;

Expand Down
40 changes: 0 additions & 40 deletions base/numerics/saturated_arithmetic.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,46 +22,6 @@

namespace base {

ALWAYS_INLINE int32_t SaturatedAddition(int32_t a, int32_t b) {
uint32_t ua = a;
uint32_t ub = b;
uint32_t result = ua + ub;

// Can only overflow if the signed bit of the two values match. If the
// signed bit of the result and one of the values differ it overflowed.
// The branch compiles to a CMOVNS instruction on x86.
if (~(ua ^ ub) & (result ^ ua) & (1 << 31))
return std::numeric_limits<int>::max() + (ua >> 31);

return result;
}

ALWAYS_INLINE int32_t SaturatedSubtraction(int32_t a, int32_t b) {
uint32_t ua = a;
uint32_t ub = b;
uint32_t result = ua - ub;

// Can only overflow if the signed bit of the two input values differ. If
// the signed bit of the result and the first value differ it overflowed.
// The branch compiles to a CMOVNS instruction on x86.
if ((ua ^ ub) & (result ^ ua) & (1 << 31))
return std::numeric_limits<int>::max() + (ua >> 31);

return result;
}

ALWAYS_INLINE int32_t SaturatedNegative(int32_t a) {
if (UNLIKELY(a == std::numeric_limits<int>::min()))
return std::numeric_limits<int>::max();
return -a;
}

ALWAYS_INLINE int32_t SaturatedAbsolute(int32_t a) {
if (a >= 0)
return a;
return SaturatedNegative(a);
}

ALWAYS_INLINE int GetMaxSaturatedSetResultForTesting(int fractional_shift) {
// For C version the set function maxes out to max int, this differs from
// the ARM asm version, see saturated_arithmetic_arm.h for the equivalent asm
Expand Down
30 changes: 0 additions & 30 deletions base/numerics/saturated_arithmetic_arm.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,6 @@

namespace base {

inline int32_t SaturatedAddition(int32_t a, int32_t b) {
int32_t result;

asm("qadd %[output],%[first],%[second]"
: [output] "=r"(result)
: [first] "r"(a), [second] "r"(b));

return result;
}

inline int32_t SaturatedSubtraction(int32_t a, int32_t b) {
int32_t result;

asm("qsub %[output],%[first],%[second]"
: [output] "=r"(result)
: [first] "r"(a), [second] "r"(b));

return result;
}

inline int32_t SaturatedNegative(int32_t a) {
return SaturatedSubtraction(0, a);
}

inline int32_t SaturatedAbsolute(int32_t a) {
if (a >= 0)
return a;
return SaturatedNegative(a);
}

inline int GetMaxSaturatedSetResultForTesting(int fractional_shift) {
// For ARM Asm version the set function maxes out to the biggest
// possible integer part with the fractional part zero'd out.
Expand Down
72 changes: 0 additions & 72 deletions base/numerics/saturated_arithmetic_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,78 +12,6 @@

namespace base {

TEST(SaturatedArithmeticTest, Addition) {
int int_max = std::numeric_limits<int>::max();
int int_min = std::numeric_limits<int>::min();

EXPECT_EQ(0, SaturatedAddition(0, 0));
EXPECT_EQ(1, SaturatedAddition(0, 1));
EXPECT_EQ(100, SaturatedAddition(0, 100));
EXPECT_EQ(150, SaturatedAddition(100, 50));

EXPECT_EQ(-1, SaturatedAddition(0, -1));
EXPECT_EQ(0, SaturatedAddition(1, -1));
EXPECT_EQ(50, SaturatedAddition(100, -50));
EXPECT_EQ(-50, SaturatedAddition(50, -100));

EXPECT_EQ(int_max - 1, SaturatedAddition(int_max - 1, 0));
EXPECT_EQ(int_max, SaturatedAddition(int_max - 1, 1));
EXPECT_EQ(int_max, SaturatedAddition(int_max - 1, 2));
EXPECT_EQ(int_max - 1, SaturatedAddition(0, int_max - 1));
EXPECT_EQ(int_max, SaturatedAddition(1, int_max - 1));
EXPECT_EQ(int_max, SaturatedAddition(2, int_max - 1));
EXPECT_EQ(int_max, SaturatedAddition(int_max - 1, int_max - 1));
EXPECT_EQ(int_max, SaturatedAddition(int_max, int_max));

EXPECT_EQ(int_min, SaturatedAddition(int_min, 0));
EXPECT_EQ(int_min + 1, SaturatedAddition(int_min + 1, 0));
EXPECT_EQ(int_min + 2, SaturatedAddition(int_min + 1, 1));
EXPECT_EQ(int_min + 3, SaturatedAddition(int_min + 1, 2));
EXPECT_EQ(int_min, SaturatedAddition(int_min + 1, -1));
EXPECT_EQ(int_min, SaturatedAddition(int_min + 1, -2));
EXPECT_EQ(int_min + 1, SaturatedAddition(0, int_min + 1));
EXPECT_EQ(int_min, SaturatedAddition(-1, int_min + 1));
EXPECT_EQ(int_min, SaturatedAddition(-2, int_min + 1));

EXPECT_EQ(int_max / 2 + 10000, SaturatedAddition(int_max / 2, 10000));
EXPECT_EQ(int_max, SaturatedAddition(int_max / 2 + 1, int_max / 2 + 1));
EXPECT_EQ(-1, SaturatedAddition(int_min, int_max));
}

TEST(SaturatedArithmeticTest, Subtraction) {
int int_max = std::numeric_limits<int>::max();
int int_min = std::numeric_limits<int>::min();

EXPECT_EQ(0, SaturatedSubtraction(0, 0));
EXPECT_EQ(-1, SaturatedSubtraction(0, 1));
EXPECT_EQ(-100, SaturatedSubtraction(0, 100));
EXPECT_EQ(50, SaturatedSubtraction(100, 50));

EXPECT_EQ(1, SaturatedSubtraction(0, -1));
EXPECT_EQ(2, SaturatedSubtraction(1, -1));
EXPECT_EQ(150, SaturatedSubtraction(100, -50));
EXPECT_EQ(150, SaturatedSubtraction(50, -100));

EXPECT_EQ(int_max, SaturatedSubtraction(int_max, 0));
EXPECT_EQ(int_max - 1, SaturatedSubtraction(int_max, 1));
EXPECT_EQ(int_max - 1, SaturatedSubtraction(int_max - 1, 0));
EXPECT_EQ(int_max, SaturatedSubtraction(int_max - 1, -1));
EXPECT_EQ(int_max, SaturatedSubtraction(int_max - 1, -2));
EXPECT_EQ(-int_max + 1, SaturatedSubtraction(0, int_max - 1));
EXPECT_EQ(-int_max, SaturatedSubtraction(-1, int_max - 1));
EXPECT_EQ(-int_max - 1, SaturatedSubtraction(-2, int_max - 1));
EXPECT_EQ(-int_max - 1, SaturatedSubtraction(-3, int_max - 1));

EXPECT_EQ(int_min, SaturatedSubtraction(int_min, 0));
EXPECT_EQ(int_min + 1, SaturatedSubtraction(int_min + 1, 0));
EXPECT_EQ(int_min, SaturatedSubtraction(int_min + 1, 1));
EXPECT_EQ(int_min, SaturatedSubtraction(int_min + 1, 2));

EXPECT_EQ(0, SaturatedSubtraction(int_min, int_min));
EXPECT_EQ(0, SaturatedSubtraction(int_max, int_max));
EXPECT_EQ(int_max, SaturatedSubtraction(int_max, int_min));
}

TEST(SaturatedArithmeticTest, SetSigned) {
int int_max = std::numeric_limits<int>::max();
int int_min = std::numeric_limits<int>::min();
Expand Down
6 changes: 3 additions & 3 deletions cc/base/rtree.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include <vector>

#include "base/logging.h"
#include "base/numerics/saturated_arithmetic.h"
#include "base/numerics/clamped_math.h"
#include "ui/gfx/geometry/rect.h"

namespace cc {
Expand Down Expand Up @@ -258,8 +258,8 @@ auto RTree<T>::BuildRecursive(std::vector<Branch<T>>* branches, int level)
++node->num_children;
++current_branch;
}
branch.bounds.SetRect(x, y, base::SaturatedSubtraction(right, x),
base::SaturatedSubtraction(bottom, y));
branch.bounds.SetRect(x, y, base::ClampSub(right, x),
base::ClampSub(bottom, y));

DCHECK_LT(new_branch_index, current_branch);
(*branches)[new_branch_index] = std::move(branch);
Expand Down
2 changes: 1 addition & 1 deletion third_party/WebKit/Source/core/layout/LayoutListItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ inline int LayoutListItem::CalcValue() const {
// FIXME: This recurses to a possible depth of the length of the list.
// That's not good -- we need to change this to an iterative algorithm.
if (LayoutListItem* previous_item = PreviousListItem(list, this))
return SaturatedAddition(previous_item->Value(), value_step);
return ClampAdd(previous_item->Value(), value_step);

if (o_list_element)
return o_list_element->start();
Expand Down
6 changes: 2 additions & 4 deletions third_party/WebKit/Source/core/paint/NinePieceImageGrid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ NinePieceImageGrid::NinePieceImageGrid(const NinePieceImage& nine_piece_image,
// as its height, and Wside as the border image width offset for the side, let
// f = min(Lwidth/(Wleft+Wright), Lheight/(Wtop+Wbottom)). If f < 1, then all
// W are reduced by multiplying them by f.
int border_side_width =
std::max(1, SaturatedAddition(left_.width, right_.width));
int border_side_height =
std::max(1, SaturatedAddition(top_.width, bottom_.width));
int border_side_width = ClampAdd(left_.width, right_.width).Max(1);
int border_side_height = ClampAdd(top_.width, bottom_.width).Max(1);
float border_side_scale_factor =
std::min((float)border_image_area.Width() / border_side_width,
(float)border_image_area.Height() / border_side_height);
Expand Down
2 changes: 1 addition & 1 deletion third_party/WebKit/Source/core/style/ComputedStyle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1898,7 +1898,7 @@ int ComputedStyle::OutlineOutsetExtent() const {
return GraphicsContext::FocusRingOutsetExtent(
OutlineOffset(), std::ceil(GetOutlineStrokeWidthForFocusRing()));
}
return std::max(0, SaturatedAddition(OutlineWidth(), OutlineOffset()));
return ClampAdd(OutlineWidth(), OutlineOffset()).Max(0);
}

float ComputedStyle::GetOutlineStrokeWidthForFocusRing() const {
Expand Down
14 changes: 7 additions & 7 deletions third_party/WebKit/Source/platform/LayoutUnit.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace blink {
#define REPORT_OVERFLOW(doesOverflow) ((void)0)
#endif

static const int kLayoutUnitFractionalBits = 6;
static const unsigned kLayoutUnitFractionalBits = 6;
static const int kFixedPointDenominator = 1 << kLayoutUnitFractionalBits;

const int kIntMaxForLayoutUnit = INT_MAX / kFixedPointDenominator;
Expand Down Expand Up @@ -154,7 +154,7 @@ class LayoutUnit {
return ToInt();
}
ALWAYS_INLINE int Round() const {
return SaturatedAddition(RawValue(), kFixedPointDenominator / 2) >>
return ClampAdd(RawValue(), kFixedPointDenominator / 2) >>
kLayoutUnitFractionalBits;
}

Expand Down Expand Up @@ -516,7 +516,7 @@ inline LayoutUnit operator/(unsigned long long a, const LayoutUnit& b) {

ALWAYS_INLINE LayoutUnit operator+(const LayoutUnit& a, const LayoutUnit& b) {
LayoutUnit return_val;
return_val.SetRawValue(SaturatedAddition(a.RawValue(), b.RawValue()));
return_val.SetRawValue(ClampAdd(a.RawValue(), b.RawValue()).RawValue());
return return_val;
}

Expand Down Expand Up @@ -546,7 +546,7 @@ inline double operator+(const double a, const LayoutUnit& b) {

ALWAYS_INLINE LayoutUnit operator-(const LayoutUnit& a, const LayoutUnit& b) {
LayoutUnit return_val;
return_val.SetRawValue(SaturatedSubtraction(a.RawValue(), b.RawValue()));
return_val.SetRawValue(ClampSub(a.RawValue(), b.RawValue()).RawValue());
return return_val;
}

Expand Down Expand Up @@ -576,7 +576,7 @@ inline float operator-(const float a, const LayoutUnit& b) {

inline LayoutUnit operator-(const LayoutUnit& a) {
LayoutUnit return_val;
return_val.SetRawValue(SaturatedNegative(a.RawValue()));
return_val.SetRawValue((-MakeClampedNum(a.RawValue())).RawValue());
return return_val;
}

Expand Down Expand Up @@ -608,7 +608,7 @@ inline LayoutUnit operator%(int a, const LayoutUnit& b) {
}

inline LayoutUnit& operator+=(LayoutUnit& a, const LayoutUnit& b) {
a.SetRawValue(SaturatedAddition(a.RawValue(), b.RawValue()));
a.SetRawValue(ClampAdd(a.RawValue(), b.RawValue()).RawValue());
return a;
}

Expand All @@ -633,7 +633,7 @@ inline LayoutUnit& operator-=(LayoutUnit& a, int b) {
}

inline LayoutUnit& operator-=(LayoutUnit& a, const LayoutUnit& b) {
a.SetRawValue(SaturatedSubtraction(a.RawValue(), b.RawValue()));
a.SetRawValue(ClampSub(a.RawValue(), b.RawValue()).RawValue());
return a;
}

Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/platform/geometry/IntPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ class PLATFORM_EXPORT IntPoint {
y_ += dy;
}
void SaturatedMove(int dx, int dy) {
x_ = SaturatedAddition(x_, dx);
y_ = SaturatedAddition(y_, dy);
x_ = ClampAdd(x_, dx);
y_ = ClampAdd(y_, dy);
}

void Scale(float sx, float sy) {
Expand Down
13 changes: 7 additions & 6 deletions third_party/WebKit/Source/platform/wtf/SaturatedArithmetic.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,19 @@
#ifndef SaturatedArithmetic_h
#define SaturatedArithmetic_h

#include "base/numerics/clamped_math.h"
#include "base/numerics/saturated_arithmetic.h"

namespace WTF {
using base::SaturatedAddition;
using base::SaturatedSubtraction;
using base::SaturatedNegative;
using base::ClampAdd;
using base::ClampSub;
using base::MakeClampedNum;
using base::SaturatedSet;
} // namespace WTF

using WTF::SaturatedAddition;
using WTF::SaturatedSubtraction;
using WTF::SaturatedNegative;
using WTF::ClampAdd;
using WTF::ClampSub;
using WTF::MakeClampedNum;
using WTF::SaturatedSet;

#endif // SaturatedArithmetic_h
18 changes: 9 additions & 9 deletions ui/gfx/geometry/point.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <string>
#include <tuple>

#include "base/numerics/saturated_arithmetic.h"
#include "base/numerics/clamped_math.h"
#include "build/build_config.h"
#include "ui/gfx/geometry/vector2d.h"
#include "ui/gfx/gfx_export.h"
Expand Down Expand Up @@ -56,18 +56,18 @@ class GFX_EXPORT Point {
}

void Offset(int delta_x, int delta_y) {
x_ = base::SaturatedAddition(x_, delta_x);
y_ = base::SaturatedAddition(y_, delta_y);
x_ = base::ClampAdd(x_, delta_x);
y_ = base::ClampAdd(y_, delta_y);
}

void operator+=(const Vector2d& vector) {
x_ = base::SaturatedAddition(x_, vector.x());
y_ = base::SaturatedAddition(y_, vector.y());
x_ = base::ClampAdd(x_, vector.x());
y_ = base::ClampAdd(y_, vector.y());
}

void operator-=(const Vector2d& vector) {
x_ = base::SaturatedSubtraction(x_, vector.x());
y_ = base::SaturatedSubtraction(y_, vector.y());
x_ = base::ClampSub(x_, vector.x());
y_ = base::ClampSub(y_, vector.y());
}

void SetToMin(const Point& other);
Expand Down Expand Up @@ -116,8 +116,8 @@ inline Point operator-(const Point& lhs, const Vector2d& rhs) {
}

inline Vector2d operator-(const Point& lhs, const Point& rhs) {
return Vector2d(base::SaturatedSubtraction(lhs.x(), rhs.x()),
base::SaturatedSubtraction(lhs.y(), rhs.y()));
return Vector2d(base::ClampSub(lhs.x(), rhs.x()),
base::ClampSub(lhs.y(), rhs.y()));
}

inline Point PointAtOffsetFromOrigin(const Vector2d& offset_from_origin) {
Expand Down
Loading

0 comments on commit 4a572da

Please sign in to comment.