From 42cf5e972d1e27a92048712bc79806e1a088b96c Mon Sep 17 00:00:00 2001 From: kwolekr Date: Sat, 1 Aug 2015 01:03:51 -0400 Subject: Improve accuracy and safety of float serialization Multiplying by a factor of 1/1000.f (rather than dividing by 1000.f) directly introduces an error of 1 ULP. With this patch, an exact comparison of a floating point literal with the deserialized F1000 form representing it is now guaranteed to be successful. In addition, the maxmium and minimum safely representible floating point numbers are now well-defined as constants. --- src/util/serialize.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'src/util') diff --git a/src/util/serialize.h b/src/util/serialize.h index 67b10d8fb..bf0d9c863 100644 --- a/src/util/serialize.h +++ b/src/util/serialize.h @@ -21,6 +21,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #define UTIL_SERIALIZE_HEADER #include "../irrlichttypes_bloated.h" +#include "../debug.h" // for assert #include "config.h" #if HAVE_ENDIAN_H #include @@ -30,7 +31,16 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #define FIXEDPOINT_FACTOR 1000.0f -#define FIXEDPOINT_INVFACTOR (1.0f/FIXEDPOINT_FACTOR) + +// 0x7FFFFFFF / 1000.0f is not serializable. +// The limited float precision at this magnitude may cause the result to round +// to a greater value than can be represented by a 32 bit integer when increased +// by a factor of FIXEDPOINT_FACTOR. As a result, [F1000_MIN..F1000_MAX] does +// not represent the full range, but rather the largest safe range, of values on +// all supported architectures. Note: This definition makes assumptions on +// platform float-to-int conversion behavior. +#define F1000_MIN ((float)(s32)((-0x7FFFFFFF - 1) / FIXEDPOINT_FACTOR)) +#define F1000_MAX ((float)(s32)((0x7FFFFFFF) / FIXEDPOINT_FACTOR)) #define STRING_MAX_LEN 0xFFFF #define WIDE_STRING_MAX_LEN 0xFFFF @@ -163,7 +173,7 @@ inline s64 readS64(const u8 *data) inline f32 readF1000(const u8 *data) { - return (f32)readS32(data) * FIXEDPOINT_INVFACTOR; + return (f32)readS32(data) / FIXEDPOINT_FACTOR; } inline video::SColor readARGB8(const u8 *data) @@ -252,6 +262,7 @@ inline void writeS64(u8 *data, s64 i) inline void writeF1000(u8 *data, f32 i) { + assert(i >= F1000_MIN && i <= F1000_MAX); writeS32(data, i * FIXEDPOINT_FACTOR); } -- cgit v1.2.3