diff options
author | mtklein <mtklein@chromium.org> | 2016-05-27 10:47:31 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-05-27 10:47:32 -0700 |
commit | 785a5b941a4d74a1f272729fe8dca55c6eda6bb8 (patch) | |
tree | 895897fb08b687abaac95e9be208272db32e4342 | |
parent | 78d58d1084f0390c1c0f9123ac6e48efcd226f39 (diff) |
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
-rw-r--r-- | bench/MathBench.cpp | 26 | ||||
-rw-r--r-- | gyp/core.gypi | 1 | ||||
-rw-r--r-- | include/private/SkFloatBits.h | 40 | ||||
-rw-r--r-- | src/core/SkFloatBits.cpp | 205 | ||||
-rw-r--r-- | tests/MathTest.cpp | 42 |
5 files changed, 42 insertions, 272 deletions
diff --git a/bench/MathBench.cpp b/bench/MathBench.cpp index 77ba2f04be..877d42663c 100644 --- a/bench/MathBench.cpp +++ b/bench/MathBench.cpp @@ -607,3 +607,29 @@ DEF_BENCH( return new CLZBench(true); ) 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 38aa1b750e..2ae75befc4 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 caad342a4a..7aa13cf67b 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 @@ static inline int32_t Sk2sComplimentToSignBit(int32_t x) { 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 @@ static inline float Sk2sComplimentAsFloat(int32_t x) { 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 ea705513ac..0000000000 --- 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 ec1f936b9d..23785f742d 100644 --- a/tests/MathTest.cpp +++ b/tests/MathTest.cpp @@ -209,34 +209,29 @@ static float nextFloat(SkRandom& rand) { /* 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_ceil(skiatest::Reporter* reporter, float x) { } 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 @@ static void unittest_fastfloat(skiatest::Reporter* reporter) { 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); - } } } |