From 785a5b941a4d74a1f272729fe8dca55c6eda6bb8 Mon Sep 17 00:00:00 2001 From: mtklein Date: Fri, 27 May 2016 10:47:31 -0700 Subject: Clean up SkFloatBits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- tests/MathTest.cpp | 42 ++++++++++-------------------------------- 1 file changed, 10 insertions(+), 32 deletions(-) (limited to 'tests/MathTest.cpp') 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); - } } } -- cgit v1.2.3