Clean up SkFloatBits
- remove dead code
- rewrite float -> int converters
The strategy for the new converters is:
- convert input to double
- floor/ceil/round in double space
- pin that double to [SK_MinS32, SK_MaxS32]
- truncate that double to int32_t
This simpler strategy does not work:
- floor/ceil/round in float space
- pin that float to [SK_MinS32, SK_MaxS32]
- truncate that float to int32_t
SK_MinS32 and SK_MaxS32 are not representable as floats:
they round to the nearest float, ±2^31, which makes the
pin insufficient for floats near SK_MinS32 (-2^31+1) or
SK_MaxS32 (+2^31-1).
float only has 24 bits of precision, and we need 31.
double can represent all integers up to 50-something bits.
An alternative is to pin in float to ±2147483520, the last
exactly representable float before SK_MaxS32 (127 too small).
Our tests test that we round as floor(x+0.5), which can
return different numbers than round(x) for negative x.
So this CL explicitly uses floor(x+0.5).
I've updated the tests with ±inf and ±NaN, and tried to
make them a little clearer, especially using SK_MinS32
instead of -SK_MaxS32.
I have not timed anything here. I have never seen any of these
methods in a profile.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2012333003
Review-Url: https://codereview.chromium.org/2012333003
diff --git a/bench/MathBench.cpp b/bench/MathBench.cpp
index 77ba2f0..877d426 100644
--- a/bench/MathBench.cpp
+++ b/bench/MathBench.cpp
@@ -607,3 +607,29 @@
DEF_BENCH( return new NormalizeBench(); )
DEF_BENCH( return new FixedMathBench(); )
+
+
+struct FloatToIntBench : public Benchmark {
+ enum { N = 1000000 };
+ float fFloats[N];
+ int fInts [N];
+
+ const char* onGetName() override { return "float_to_int"; }
+ bool isSuitableFor(Backend backend) override { return backend == kNonRendering_Backend; }
+
+ void onDelayedSetup() override {
+ const auto f32 = 4294967296.0f;
+ for (int i = 0; i < N; ++i) {
+ fFloats[i] = -f32 + i*(2*f32/N);
+ }
+ }
+
+ void onDraw(int loops, SkCanvas*) override {
+ while (loops --> 0) {
+ for (int i = 0; i < N; i++) {
+ fInts[i] = SkFloatToIntFloor(fFloats[i]);
+ }
+ }
+ }
+};
+DEF_BENCH( return new FloatToIntBench; )
diff --git a/gyp/core.gypi b/gyp/core.gypi
index 38aa1b7..2ae75be 100644
--- a/gyp/core.gypi
+++ b/gyp/core.gypi
@@ -123,7 +123,6 @@
'<(skia_src_path)/core/SkFindAndPlaceGlyph.h',
'<(skia_src_path)/core/SkFlattenable.cpp',
'<(skia_src_path)/core/SkFlattenableSerialization.cpp',
- '<(skia_src_path)/core/SkFloatBits.cpp',
'<(skia_src_path)/core/SkFont.cpp',
'<(skia_src_path)/core/SkFontLCDConfig.cpp',
'<(skia_src_path)/core/SkFontMgr.cpp',
diff --git a/include/private/SkFloatBits.h b/include/private/SkFloatBits.h
index caad342..7aa13cf 100644
--- a/include/private/SkFloatBits.h
+++ b/include/private/SkFloatBits.h
@@ -1,4 +1,3 @@
-
/*
* Copyright 2008 The Android Open Source Project
*
@@ -11,6 +10,7 @@
#define SkFloatBits_DEFINED
#include "SkTypes.h"
+#include <math.h>
/** Convert a sign-bit int (i.e. float interpreted as int) into a 2s compliement
int. This also converts -0 (0x80000000) to 0. Doing this to a float allows
@@ -36,27 +36,6 @@
return x;
}
-/** Given the bit representation of a float, return its value cast to an int.
- If the value is out of range, or NaN, return return +/- SK_MaxS32
-*/
-int32_t SkFloatBits_toIntCast(int32_t floatBits);
-
-/** Given the bit representation of a float, return its floor as an int.
- If the value is out of range, or NaN, return return +/- SK_MaxS32
- */
-SK_API int32_t SkFloatBits_toIntFloor(int32_t floatBits);
-
-/** Given the bit representation of a float, return it rounded to an int.
- If the value is out of range, or NaN, return return +/- SK_MaxS32
- */
-SK_API int32_t SkFloatBits_toIntRound(int32_t floatBits);
-
-/** Given the bit representation of a float, return its ceiling as an int.
- If the value is out of range, or NaN, return return +/- SK_MaxS32
- */
-SK_API int32_t SkFloatBits_toIntCeil(int32_t floatBits);
-
-
union SkFloatIntUnion {
float fFloat;
int32_t fSignBitInt;
@@ -92,36 +71,29 @@
return SkBits2Float(Sk2sComplimentToSignBit(x));
}
-/** Return x cast to a float (i.e. (float)x)
-*/
-float SkIntToFloatCast(int x);
-
-/** Return the float cast to an int.
- If the value is out of range, or NaN, return +/- SK_MaxS32
-*/
-static inline int32_t SkFloatToIntCast(float x) {
- return SkFloatBits_toIntCast(SkFloat2Bits(x));
+static inline int32_t pin_double_to_int(double x) {
+ return (int32_t)SkTPin<double>(x, SK_MinS32, SK_MaxS32);
}
/** Return the floor of the float as an int.
If the value is out of range, or NaN, return +/- SK_MaxS32
*/
static inline int32_t SkFloatToIntFloor(float x) {
- return SkFloatBits_toIntFloor(SkFloat2Bits(x));
+ return pin_double_to_int(floor(x));
}
/** Return the float rounded to an int.
If the value is out of range, or NaN, return +/- SK_MaxS32
*/
static inline int32_t SkFloatToIntRound(float x) {
- return SkFloatBits_toIntRound(SkFloat2Bits(x));
+ return pin_double_to_int(floor((double)x + 0.5));
}
/** Return the ceiling of the float as an int.
If the value is out of range, or NaN, return +/- SK_MaxS32
*/
static inline int32_t SkFloatToIntCeil(float x) {
- return SkFloatBits_toIntCeil(SkFloat2Bits(x));
+ return pin_double_to_int(ceil(x));
}
// Scalar wrappers for float-bit routines
diff --git a/src/core/SkFloatBits.cpp b/src/core/SkFloatBits.cpp
deleted file mode 100644
index ea70551..0000000
--- a/src/core/SkFloatBits.cpp
+++ /dev/null
@@ -1,205 +0,0 @@
-/*
- * Copyright 2011 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkFloatBits.h"
-#include "SkMathPriv.h"
-
-/******************************************************************************
- SkFloatBits_toInt[Floor, Round, Ceil] are identical except for what they
- do right before they return ... >> exp;
- Floor - adds nothing
- Round - adds 1 << (exp - 1)
- Ceil - adds (1 << exp) - 1
-
- Floor and Cast are very similar, but Cast applies its sign after all other
- computations on value. Also, Cast does not need to check for negative zero,
- as that value (0x80000000) "does the right thing" for Ceil. Note that it
- doesn't for Floor/Round/Ceil, hence the explicit check.
-******************************************************************************/
-
-#define EXP_BIAS (127+23)
-#define MATISSA_MAGIC_BIG (1 << 23)
-
-static inline int unpack_exp(uint32_t packed) {
- return (packed << 1 >> 24);
-}
-
-#if 0
-// the ARM compiler generates an extra BIC, so I use the dirty version instead
-static inline int unpack_matissa(uint32_t packed) {
- // we could mask with 0x7FFFFF, but that is harder for ARM to encode
- return (packed & ~0xFF000000) | MATISSA_MAGIC_BIG;
-}
-#endif
-
-// returns the low 24-bits, so we need to OR in the magic_bit afterwards
-static inline int unpack_matissa_dirty(uint32_t packed) {
- return packed & ~0xFF000000;
-}
-
-// same as (int)float
-int32_t SkFloatBits_toIntCast(int32_t packed) {
- int exp = unpack_exp(packed) - EXP_BIAS;
- int value = unpack_matissa_dirty(packed) | MATISSA_MAGIC_BIG;
-
- if (exp >= 0) {
- if (exp > 7) { // overflow
- value = SK_MaxS32;
- } else {
- value <<= exp;
- }
- } else {
- exp = -exp;
- if (exp > 25) { // underflow
- exp = 25;
- }
- value >>= exp;
- }
- return SkApplySign(value, SkExtractSign(packed));
-}
-
-// same as (int)floor(float)
-int32_t SkFloatBits_toIntFloor(int32_t packed) {
- // curse you negative 0
- if (SkLeftShift(packed, 1) == 0) {
- return 0;
- }
-
- int exp = unpack_exp(packed) - EXP_BIAS;
- int value = unpack_matissa_dirty(packed) | MATISSA_MAGIC_BIG;
-
- if (exp >= 0) {
- if (exp > 7) { // overflow
- value = SK_MaxS32;
- } else {
- value <<= exp;
- }
- // apply the sign after we check for overflow
- return SkApplySign(value, SkExtractSign(packed));
- } else {
- // apply the sign before we right-shift
- value = SkApplySign(value, SkExtractSign(packed));
- exp = -exp;
- if (exp > 25) { // underflow
-#ifdef SK_CPU_FLUSH_TO_ZERO
- // The iOS ARM processor discards small denormalized numbers to go faster.
- // The comparision below empirically causes the result to agree with the
- // tests in MathTest test_float_floor
- if (exp > 149) {
- return 0;
- }
-#else
- exp = 25;
-#endif
- }
- // int add = 0;
- return value >> exp;
- }
-}
-
-// same as (int)floor(float + 0.5)
-int32_t SkFloatBits_toIntRound(int32_t packed) {
- // curse you negative 0
- if (SkLeftShift(packed, 1) == 0) {
- return 0;
- }
-
- int exp = unpack_exp(packed) - EXP_BIAS;
- int value = unpack_matissa_dirty(packed) | MATISSA_MAGIC_BIG;
-
- if (exp >= 0) {
- if (exp > 7) { // overflow
- value = SK_MaxS32;
- } else {
- value <<= exp;
- }
- // apply the sign after we check for overflow
- return SkApplySign(value, SkExtractSign(packed));
- } else {
- // apply the sign before we right-shift
- value = SkApplySign(value, SkExtractSign(packed));
- exp = -exp;
- if (exp > 25) { // underflow
- exp = 25;
- }
- int add = 1 << (exp - 1);
- return (value + add) >> exp;
- }
-}
-
-// same as (int)ceil(float)
-int32_t SkFloatBits_toIntCeil(int32_t packed) {
- // curse you negative 0
- if (SkLeftShift(packed, 1) == 0) {
- return 0;
- }
-
- int exp = unpack_exp(packed) - EXP_BIAS;
- int value = unpack_matissa_dirty(packed) | MATISSA_MAGIC_BIG;
-
- if (exp >= 0) {
- if (exp > 7) { // overflow
- value = SK_MaxS32;
- } else {
- value <<= exp;
- }
- // apply the sign after we check for overflow
- return SkApplySign(value, SkExtractSign(packed));
- } else {
- // apply the sign before we right-shift
- value = SkApplySign(value, SkExtractSign(packed));
- exp = -exp;
- if (exp > 25) { // underflow
-#ifdef SK_CPU_FLUSH_TO_ZERO
- // The iOS ARM processor discards small denormalized numbers to go faster.
- // The comparision below empirically causes the result to agree with the
- // tests in MathTest test_float_ceil
- if (exp > 149) {
- return 0;
- }
- return 0 < value;
-#else
- exp = 25;
-#endif
- }
- int add = (1 << exp) - 1;
- return (value + add) >> exp;
- }
-}
-
-float SkIntToFloatCast(int32_t value) {
- if (0 == value) {
- return 0;
- }
-
- int shift = EXP_BIAS;
-
- // record the sign and make value positive
- int sign = SkExtractSign(value);
- value = SkApplySign(value, sign);
-
- if (value >> 24) { // value is too big (has more than 24 bits set)
- int bias = 8 - SkCLZ(value);
- SkDebugf("value = %d, bias = %d\n", value, bias);
- SkASSERT(bias > 0 && bias < 8);
- value >>= bias; // need to round?
- shift += bias;
- } else {
- int zeros = SkCLZ(value << 8);
- SkASSERT(zeros >= 0 && zeros <= 23);
- value <<= zeros;
- shift -= zeros;
- }
-
- // now value is left-aligned to 24 bits
- SkASSERT((value >> 23) == 1);
- SkASSERT(shift >= 0 && shift <= 255);
-
- SkFloatIntUnion data;
- data.fSignBitInt = SkLeftShift(sign, 31) | SkLeftShift(shift, 23) | (value & ~MATISSA_MAGIC_BIG);
- return data.fFloat;
-}
diff --git a/tests/MathTest.cpp b/tests/MathTest.cpp
index ec1f936..23785f7 100644
--- a/tests/MathTest.cpp
+++ b/tests/MathTest.cpp
@@ -209,34 +209,29 @@
/* returns true if a == b as resulting from (int)x. Since it is undefined
what to do if the float exceeds 2^32-1, we check for that explicitly.
*/
-static bool equal_float_native_skia(float x, uint32_t ni, uint32_t si) {
- if (!(x == x)) { // NAN
- return ((int32_t)si) == SK_MaxS32 || ((int32_t)si) == SK_MinS32;
+static bool equal_float_native_skia(float x, int32_t ni, int32_t si) {
+ // When the float is out of integer range (NaN, above, below),
+ // the C cast is undefined, but Skia's methods should have clamped.
+ if (!(x == x)) { // NaN
+ return si == SK_MaxS32 || si == SK_MinS32;
}
- // for out of range, C is undefined, but skia always should return NaN32
if (x > SK_MaxS32) {
- return ((int32_t)si) == SK_MaxS32;
+ return si == SK_MaxS32;
}
- if (x < -SK_MaxS32) {
- return ((int32_t)si) == SK_MinS32;
+ if (x < SK_MinS32) {
+ return si == SK_MinS32;
}
return si == ni;
}
static void assert_float_equal(skiatest::Reporter* reporter, const char op[],
- float x, uint32_t ni, uint32_t si) {
+ float x, int32_t ni, int32_t si) {
if (!equal_float_native_skia(x, ni, si)) {
ERRORF(reporter, "%s float %g bits %x native %x skia %x\n",
op, x, SkFloat2Bits(x), ni, si);
}
}
-static void test_float_cast(skiatest::Reporter* reporter, float x) {
- int ix = (int)x;
- int iix = SkFloatToIntCast(x);
- assert_float_equal(reporter, "cast", x, ix, iix);
-}
-
static void test_float_floor(skiatest::Reporter* reporter, float x) {
int ix = (int)floor(x);
int iix = SkFloatToIntFloor(x);
@@ -257,23 +252,17 @@
}
static void test_float_conversions(skiatest::Reporter* reporter, float x) {
- test_float_cast(reporter, x);
test_float_floor(reporter, x);
test_float_round(reporter, x);
test_float_ceil(reporter, x);
}
-static void test_int2float(skiatest::Reporter* reporter, int ival) {
- float x0 = (float)ival;
- float x1 = SkIntToFloatCast(ival);
- REPORTER_ASSERT(reporter, x0 == x1);
-}
-
static void unittest_fastfloat(skiatest::Reporter* reporter) {
SkRandom rand;
size_t i;
static const float gFloats[] = {
+ 0.f/0.f, -0.f/0.f, 1.f/0.f, -1.f/0.f,
0.f, 1.f, 0.5f, 0.499999f, 0.5000001f, 1.f/3,
0.000000001f, 1000000000.f, // doesn't overflow
0.0000000001f, 10000000000.f // does overflow
@@ -289,17 +278,6 @@
float x = nextFloat(rand);
test_float_conversions(reporter, x);
}
-
- test_int2float(reporter, 0);
- test_int2float(reporter, 1);
- test_int2float(reporter, -1);
- for (i = 0; i < 100000; i++) {
- // for now only test ints that are 24bits or less, since we don't
- // round (down) large ints the same as IEEE...
- int ival = rand.nextU() & 0xFFFFFF;
- test_int2float(reporter, ival);
- test_int2float(reporter, -ival);
- }
}
}