Merge pull request #10490 from hpvb/v2.1-fix-nan-index

[v2.1] Correct hash behavior for floating point numbers

core/hash_map.h

@@ -30,36 +30,43 @@
 #ifndef HASH_MAP_H
 #define HASH_MAP_H
 
-#include "error_macros.h"
-#include "hashfuncs.h"
 #include "list.h"
+#include "hashfuncs.h"
+#include "math_funcs.h"
 #include "os/memory.h"
 #include "ustring.h"
 
-class HashMapHahserDefault {
-public:
-	static _FORCE_INLINE_ uint32_t hash(const String &p_string) { return p_string.hash(); }
-	static _FORCE_INLINE_ uint32_t hash(const char *p_cstr) { return hash_djb2(p_cstr); }
-	static _FORCE_INLINE_ uint32_t hash(const uint64_t p_int) {
-		uint64_t v = p_int;
-		v = (~v) + (v << 18); // v = (v << 18) - v - 1;
-		v = v ^ (v >> 31);
-		v = v * 21; // v = (v + (v << 2)) + (v << 4);
-		v = v ^ (v >> 11);
-		v = v + (v << 6);
-		v = v ^ (v >> 22);
-		return (int)v;
-	}
-	static _FORCE_INLINE_ uint32_t hash(const int64_t p_int) { return hash(uint64_t(p_int)); }
+struct HashMapHasherDefault {
+	static _FORCE_INLINE_ uint32_t hash(const String &p_string)  { return p_string.hash(); }
+	static _FORCE_INLINE_ uint32_t hash(const char *p_cstr)  { return hash_djb2(p_cstr); }
+	static _FORCE_INLINE_ uint32_t hash(const uint64_t p_int)  { return hash_one_uint64(p_int);	}
 
+	static _FORCE_INLINE_ uint32_t hash(const int64_t p_int)  { return hash(uint64_t(p_int)); }
+	static _FORCE_INLINE_ uint32_t hash(const float p_float)  { return hash_djb2_one_float(p_float); }
+	static _FORCE_INLINE_ uint32_t hash(const double p_double){ return hash_djb2_one_float(p_double); }
 	static _FORCE_INLINE_ uint32_t hash(const uint32_t p_int) { return p_int; }
-	static _FORCE_INLINE_ uint32_t hash(const int32_t p_int) { return (uint32_t)p_int; }
+	static _FORCE_INLINE_ uint32_t hash(const int32_t p_int)  { return (uint32_t)p_int; }
 	static _FORCE_INLINE_ uint32_t hash(const uint16_t p_int) { return p_int; }
-	static _FORCE_INLINE_ uint32_t hash(const int16_t p_int) { return (uint32_t)p_int; }
-	static _FORCE_INLINE_ uint32_t hash(const uint8_t p_int) { return p_int; }
-	static _FORCE_INLINE_ uint32_t hash(const int8_t p_int) { return (uint32_t)p_int; }
-	static _FORCE_INLINE_ uint32_t hash(const wchar_t p_wchar) { return (uint32_t)p_wchar; }
-	//	static _FORCE_INLINE_ uint32_t hash(const void* p_ptr)  { return uint32_t(uint64_t(p_ptr))*(0x9e3779b1L); }
+	static _FORCE_INLINE_ uint32_t hash(const int16_t p_int)  { return (uint32_t)p_int; }
+	static _FORCE_INLINE_ uint32_t hash(const uint8_t p_int)  { return p_int; }
+	static _FORCE_INLINE_ uint32_t hash(const int8_t p_int)   { return (uint32_t)p_int; }
+	static _FORCE_INLINE_ uint32_t hash(const wchar_t p_wchar){ return (uint32_t)p_wchar; }
+	//static _FORCE_INLINE_ uint32_t hash(const void* p_ptr)  { return uint32_t(uint64_t(p_ptr))*(0x9e3779b1L); }
+};
+
+template <typename T>
+struct HashMapComparatorDefault {
+	static bool compare(const T& p_lhs, const T& p_rhs) {
+		return p_lhs == p_rhs;
+	}
+
+	bool compare(const float& p_lhs, const float& p_rhs) {
+		return (p_lhs == p_rhs) || (Math::is_nan(p_lhs) && Math::is_nan(p_rhs));
+	}
+
+	bool compare(const double& p_lhs, const double& p_rhs) {
+		return (p_lhs == p_rhs) || (Math::is_nan(p_lhs) && Math::is_nan(p_rhs));
+	}
 };
 
 /**
@@ -72,13 +79,14 @@ public:
  * @param TKey  Key, search is based on it, needs to be hasheable. It is unique in this container.
  * @param TData Data, data associated with the key
  * @param Hasher Hasher object, needs to provide a valid static hash function for TKey
+ * @param Comparator comparator object, needs to be able to safely compare two TKey values. It needs to ensure that x == x for any items inserted in the map. Bear in mind that nan != nan when implementing an equality check.
  * @param MIN_HASH_TABLE_POWER Miminum size of the hash table, as a power of two. You rarely need to change this parameter.
  * @param RELATIONSHIP Relationship at which the hash table is resized. if amount of elements is RELATIONSHIP
  * times bigger than the hash table, table is resized to solve this condition. if RELATIONSHIP is zero, table is always MIN_HASH_TABLE_POWER.
  *
 */
 
-template <class TKey, class TData, class Hasher = HashMapHahserDefault, uint8_t MIN_HASH_TABLE_POWER = 3, uint8_t RELATIONSHIP = 8>
+template <class TKey, class TData, class Hasher = HashMapHasherDefault, class Comparator=HashMapComparatorDefault<TKey>, uint8_t MIN_HASH_TABLE_POWER=3,uint8_t RELATIONSHIP=8>
 class HashMap {
 public:
 	struct Pair {
@@ -200,7 +208,7 @@ private:
 		while (e) {
 
 			/* checking hash first avoids comparing key, which may take longer */
-			if (e->hash == hash && e->pair.key == p_key) {
+			if (e->hash == hash && Comparator::compare(e->pair.key,p_key) ) {
 
 				/* the pair exists in this hashtable, so just update data */
 				return e;
@@ -367,7 +375,7 @@ public:
 		while (e) {
 
 			/* checking hash first avoids comparing key, which may take longer */
-			if (e->hash == hash && e->pair.key == p_custom_key) {
+			if (e->hash == hash && Comparator::compare(e->pair.key,p_custom_key) ) {
 
 				/* the pair exists in this hashtable, so just update data */
 				return &e->pair.data;
@@ -393,7 +401,7 @@ public:
 		while (e) {
 
 			/* checking hash first avoids comparing key, which may take longer */
-			if (e->hash == hash && e->pair.key == p_custom_key) {
+			if (e->hash == hash && Comparator::compare(e->pair.key,p_custom_key) ) {
 
 				/* the pair exists in this hashtable, so just update data */
 				return &e->pair.data;
@@ -422,7 +430,7 @@ public:
 		while (e) {
 
 			/* checking hash first avoids comparing key, which may take longer */
-			if (e->hash == hash && e->pair.key == p_key) {
+			if (e->hash == hash && Comparator::compare(e->pair.key,p_key) ) {
 
 				if (p) {
 

core/hashfuncs.h

@@ -30,6 +30,8 @@
 #ifndef HASHFUNCS_H
 #define HASHFUNCS_H
 
+#include "math_funcs.h"
+#include "math_defs.h"
 #include "typedefs.h"
 
 /**
@@ -68,22 +70,35 @@ static inline uint32_t hash_djb2_one_32(uint32_t p_in, uint32_t p_prev = 5381) {
 	return ((p_prev << 5) + p_prev) + p_in;
 }
 
-static inline uint32_t hash_djb2_one_float(float p_in, uint32_t p_prev = 5381) {
+static inline uint32_t hash_one_uint64(const uint64_t p_int) {
+	uint64_t v=p_int;
+	v = (~v) + (v << 18); // v = (v << 18) - v - 1;
+	v = v ^ (v >> 31);
+	v = v * 21; // v = (v + (v << 2)) + (v << 4);
+	v = v ^ (v >> 11);
+	v = v + (v << 6);
+	v = v ^ (v >> 22);
+	return (int) v;
+}
+
+static inline uint32_t hash_djb2_one_float(double p_in, uint32_t p_prev = 5381) {
 	union {
-		float f;
-		uint32_t i;
+		double d;
+		uint64_t i;
 	} u;
 
-	// handle -0 case
-	if (p_in == 0.0f)
-		u.f = 0.0f;
+	// Normalize +/- 0.0 and NaN values so they hash the same.
+	if (p_in==0.0f)
+		u.d=0.0;
+	else if (Math::is_nan(p_in))
+		u.d=NAN;
 	else
-		u.f = p_in;
+		u.d=p_in;
 
-	return ((p_prev << 5) + p_prev) + u.i;
+	return ((p_prev<<5)+p_prev) + hash_one_uint64(u.i);
 }
 
-template <class T>
+template<class T>
 static inline uint32_t make_uint32_t(T p_in) {
 
 	union {

core/object.h

@@ -676,7 +676,7 @@ class ObjectDB {
 				unsigned long i;
 			} u;
 			u.p = p_obj;
-			return HashMapHahserDefault::hash((uint64_t)u.i);
+			return HashMapHasherDefault::hash((uint64_t)u.i);
 		}
 	};
 

core/variant.cpp

@@ -30,6 +30,7 @@
 #include "variant.h"
 #include "core_string_names.h"
 #include "io/marshalls.h"
+#include "math_funcs.h"
 #include "print_string.h"
 #include "resource.h"
 #include "scene/gui/control.h"
@@ -2598,14 +2599,10 @@ uint32_t Variant::hash() const {
 		case INT: {
 
 			return _data._int;
-
 		} break;
 		case REAL: {
 
-			MarshallFloat mf;
-			mf.f = _data._real;
-			return mf.i;
-
+			return hash_djb2_one_float(_data._real);
 		} break;
 		case STRING: {
 
@@ -2840,6 +2837,187 @@ uint32_t Variant::hash() const {
 	return 0;
 }
 
+#define hash_compare_scalar(p_lhs, p_rhs) \
+	((p_lhs) == (p_rhs)) || (Math::is_nan(p_lhs) && Math::is_nan(p_rhs))
+
+#define hash_compare_vector2(p_lhs, p_rhs)\
+	(hash_compare_scalar((p_lhs).x, (p_rhs).x)) && \
+	(hash_compare_scalar((p_lhs).y, (p_rhs).y))
+
+#define hash_compare_vector3(p_lhs, p_rhs)\
+	(hash_compare_scalar((p_lhs).x, (p_rhs).x)) && \
+	(hash_compare_scalar((p_lhs).y, (p_rhs).y)) && \
+	(hash_compare_scalar((p_lhs).z, (p_rhs).z))
+
+#define hash_compare_quat(p_lhs, p_rhs)\
+	(hash_compare_scalar((p_lhs).x, (p_rhs).x)) && \
+	(hash_compare_scalar((p_lhs).y, (p_rhs).y)) && \
+	(hash_compare_scalar((p_lhs).z, (p_rhs).z)) && \
+	(hash_compare_scalar((p_lhs).w, (p_rhs).w))
+
+#define hash_compare_color(p_lhs, p_rhs)\
+	(hash_compare_scalar((p_lhs).r, (p_rhs).r)) && \
+	(hash_compare_scalar((p_lhs).g, (p_rhs).g)) && \
+	(hash_compare_scalar((p_lhs).b, (p_rhs).b)) && \
+	(hash_compare_scalar((p_lhs).a, (p_rhs).a))
+
+#define hash_compare_pool_array(p_lhs, p_rhs, p_type, p_compare_func)\
+		const DVector<p_type>& l = *reinterpret_cast<const DVector<p_type>*>(p_lhs);\
+		const DVector<p_type>& r = *reinterpret_cast<const DVector<p_type>*>(p_rhs);\
+		\
+		if(l.size() != r.size()) \
+			return false; \
+		\
+		DVector<p_type>::Read lr = l.read(); \
+		DVector<p_type>::Read rr = r.read(); \
+		\
+		for(int i = 0; i < l.size(); ++i) { \
+			if(! p_compare_func((lr[0]), (rr[0]))) \
+				return false; \
+		}\
+		\
+		return true
+
+bool Variant::hash_compare(const Variant& p_variant) const {
+	if (type != p_variant.type)
+		return false;
+
+	switch( type ) {
+		case REAL: {
+			return hash_compare_scalar(_data._real, p_variant._data._real);
+		} break;
+
+		case VECTOR2: {
+			const Vector2* l = reinterpret_cast<const Vector2*>(_data._mem);
+			const Vector2* r = reinterpret_cast<const Vector2*>(p_variant._data._mem);
+
+			return hash_compare_vector2(*l, *r);
+		} break;
+
+		case RECT2: {
+			const Rect2* l = reinterpret_cast<const Rect2*>(_data._mem);
+			const Rect2* r = reinterpret_cast<const Rect2*>(p_variant._data._mem);
+
+			return (hash_compare_vector2(l->pos, r->pos)) &&
+					(hash_compare_vector2(l->size, r->size));
+		} break;
+
+		case MATRIX32: {
+			Matrix32* l = _data._matrix32;
+			Matrix32* r = p_variant._data._matrix32;
+
+			for(int i=0;i<3;i++) {
+				if (! (hash_compare_vector2(l->elements[i], r->elements[i])))
+					return false;
+			}
+
+			return true;
+		} break;
+
+		case VECTOR3: {
+			const Vector3* l = reinterpret_cast<const Vector3*>(_data._mem);
+			const Vector3* r = reinterpret_cast<const Vector3*>(p_variant._data._mem);
+
+			return hash_compare_vector3(*l, *r);
+		} break;
+
+		case PLANE: {
+			const Plane* l = reinterpret_cast<const Plane*>(_data._mem);
+			const Plane* r = reinterpret_cast<const Plane*>(p_variant._data._mem);
+
+			return (hash_compare_vector3(l->normal, r->normal)) &&
+					(hash_compare_scalar(l->d, r->d));
+		} break;
+
+		case _AABB: {
+			const AABB* l = _data._aabb;
+			const AABB* r = p_variant._data._aabb;
+
+			return (hash_compare_vector3(l->pos, r->pos) &&
+					(hash_compare_vector3(l->size, r->size)));
+
+		} break;
+
+		case QUAT: {
+			const Quat* l = reinterpret_cast<const Quat*>(_data._mem);
+			const Quat* r = reinterpret_cast<const Quat*>(p_variant._data._mem);
+
+			return hash_compare_quat(*l, *r);
+		} break;
+
+		case MATRIX3: {
+			const Matrix3* l = _data._matrix3;
+			const Matrix3* r = p_variant._data._matrix3;
+
+			for(int i=0;i<3;i++) {
+				if (! (hash_compare_vector3(l->elements[i], r->elements[i])))
+					return false;
+			}
+
+			return true;
+		} break;
+
+		case TRANSFORM: {
+			const Transform* l = _data._transform;
+			const Transform* r = p_variant._data._transform;
+
+			for(int i=0;i<3;i++) {
+				if (! (hash_compare_vector3(l->basis.elements[i], r->basis.elements[i])))
+					return false;
+			}
+
+			return hash_compare_vector3(l->origin, r->origin);
+		} break;
+
+		case COLOR: {
+			const Color* l = reinterpret_cast<const Color*>(_data._mem);
+			const Color* r = reinterpret_cast<const Color*>(p_variant._data._mem);
+
+			return hash_compare_color(*l, *r);
+		} break;
+
+		case ARRAY: {
+			const Array& l = *(reinterpret_cast<const Array*>(_data._mem));
+			const Array& r = *(reinterpret_cast<const Array*>(p_variant._data._mem));
+
+			if(l.size() != r.size())
+				return false;
+
+			for(int i = 0; i < l.size(); ++i) {
+				if(! l[0].hash_compare(r[0]))
+					return false;
+			}
+
+			return true;
+		} break;
+
+		case REAL_ARRAY: {
+			hash_compare_pool_array(_data._mem, p_variant._data._mem, real_t, hash_compare_scalar);
+		} break;
+
+		case VECTOR2_ARRAY: {
+			hash_compare_pool_array(_data._mem, p_variant._data._mem, Vector2, hash_compare_vector2);
+		} break;
+
+		case VECTOR3_ARRAY: {
+			hash_compare_pool_array(_data._mem, p_variant._data._mem, Vector3, hash_compare_vector3);
+		} break;
+
+		case COLOR_ARRAY: {
+			hash_compare_pool_array(_data._mem, p_variant._data._mem, Color, hash_compare_color);
+		} break;
+
+		default:
+			bool v;
+			Variant r;
+			evaluate(OP_EQUAL,*this,p_variant,r,v);
+			return r;
+		}
+
+	return false;
+}
+
+
 bool Variant::is_ref() const {
 
 	return type == OBJECT && !_get_obj().ref.is_null();

core/variant.h

@@ -401,6 +401,7 @@ public:
 	bool operator<(const Variant &p_variant) const;
 	uint32_t hash() const;
 
+	bool hash_compare(const Variant& p_variant) const;
 	bool booleanize(bool &valid) const;
 
 	void static_assign(const Variant &p_variant);
@@ -438,8 +439,12 @@ struct VariantHasher {
 	static _FORCE_INLINE_ uint32_t hash(const Variant &p_variant) { return p_variant.hash(); }
 };
 
-Variant::ObjData &Variant::_get_obj() {
+struct VariantComparator {
+
+	static _FORCE_INLINE_ bool compare(const Variant &p_lhs, const Variant &p_rhs)  { return p_lhs.hash_compare(p_rhs); }
+};
 
+Variant::ObjData &Variant::_get_obj() {
 	return *reinterpret_cast<ObjData *>(&_data._mem[0]);
 }
 

drivers/gles2/shader_gles2.h

@@ -134,7 +134,7 @@ private:
 
 	struct VersionKeyHash {
 
-		static _FORCE_INLINE_ uint32_t hash(const VersionKey &p_key) { return HashMapHahserDefault::hash(p_key.key); };
+		static _FORCE_INLINE_ uint32_t hash( const VersionKey& p_key) { return HashMapHasherDefault::hash(p_key.key); };
 	};
 
 	//this should use a way more cachefriendly version..

editor/plugins/baked_light_baker.cpp

@@ -1217,7 +1217,7 @@ void BakedLightBaker::_make_octree_texture() {
 			base <<= 16;
 			base |= int((pos.z + cell_size * 0.5) / cell_size);
 
-			uint32_t hash = HashMapHahserDefault::hash(base);
+			uint32_t hash = HashMapHasherDefault::hash(base);
 			uint32_t idx = hash % hash_table_size;
 			octhashptr[oct_idx].next = hashptr[idx];
 			octhashptr[oct_idx].hash = hash;
@@ -1243,7 +1243,7 @@ void BakedLightBaker::_make_octree_texture() {
 			base <<= 16;
 			base |= int((pos.z + cell_size * 0.5) / cell_size);
 
-			uint32_t hash = HashMapHahserDefault::hash(base);
+			uint32_t hash = HashMapHasherDefault::hash(base);
 			uint32_t idx = hash % hash_table_size;
 
 			uint32_t bucket = hashptr[idx];

modules/gdscript/gd_compiler.h

@@ -91,8 +91,8 @@ class GDCompiler {
 		}
 
 		//int get_identifier_pos(const StringName& p_dentifier) const;
-		HashMap<Variant, int, VariantHasher> constant_map;
-		Map<StringName, int> name_map;
+		HashMap<Variant,int,VariantHasher,VariantComparator> constant_map;
+		Map<StringName,int> name_map;
 
 		int get_name_map_pos(const StringName &p_identifier) {
 			int ret;

modules/gdscript/gd_tokenizer.cpp

@@ -1124,9 +1124,9 @@ Vector<uint8_t> GDTokenizerBuffer::parse_code_string(const String &p_code) {
 
 	Vector<uint8_t> buf;
 
-	Map<StringName, int> identifier_map;
-	HashMap<Variant, int, VariantHasher> constant_map;
-	Map<uint32_t, int> line_map;
+	Map<StringName,int> identifier_map;
+	HashMap<Variant,int,VariantHasher,VariantComparator> constant_map;
+	Map<uint32_t,int> line_map;
 	Vector<uint32_t> token_array;
 
 	GDTokenizerText tt;

scene/resources/packed_scene.cpp

@@ -306,7 +306,7 @@ static int _nm_get_string(const String &p_string, Map<StringName, int> &name_map
 	return idx;
 }
 
-static int _vm_get_variant(const Variant &p_variant, HashMap<Variant, int, VariantHasher> &variant_map) {
+static int _vm_get_variant(const Variant& p_variant, HashMap<Variant,int,VariantHasher,VariantComparator> &variant_map) {
 
 	if (variant_map.has(p_variant))
 		return variant_map[p_variant];
@@ -316,7 +316,7 @@ static int _vm_get_variant(const Variant &p_variant, HashMap<Variant, int, Varia
 	return idx;
 }
 
-Error SceneState::_parse_node(Node *p_owner, Node *p_node, int p_parent_idx, Map<StringName, int> &name_map, HashMap<Variant, int, VariantHasher> &variant_map, Map<Node *, int> &node_map, Map<Node *, int> &nodepath_map) {
+Error SceneState::_parse_node(Node *p_owner,Node *p_node,int p_parent_idx, Map<StringName,int> &name_map,HashMap<Variant,int,VariantHasher,VariantComparator> &variant_map,Map<Node*,int> &node_map,Map<Node*,int> &nodepath_map) {
 
 	// this function handles all the work related to properly packing scenes, be it
 	// instanced or inherited.
@@ -673,7 +673,7 @@ Error SceneState::_parse_node(Node *p_owner, Node *p_node, int p_parent_idx, Map
 	return OK;
 }
 
-Error SceneState::_parse_connections(Node *p_owner, Node *p_node, Map<StringName, int> &name_map, HashMap<Variant, int, VariantHasher> &variant_map, Map<Node *, int> &node_map, Map<Node *, int> &nodepath_map) {
+Error SceneState::_parse_connections(Node *p_owner,Node *p_node, Map<StringName,int> &name_map,HashMap<Variant,int,VariantHasher,VariantComparator> &variant_map,Map<Node*,int> &node_map,Map<Node*,int> &nodepath_map) {
 
 	if (p_node != p_owner && p_node->get_owner() && p_node->get_owner() != p_owner && !p_owner->is_editable_instance(p_node->get_owner()))
 		return OK;
@@ -860,10 +860,10 @@ Error SceneState::pack(Node *p_scene) {
 
 	Node *scene = p_scene;
 
-	Map<StringName, int> name_map;
-	HashMap<Variant, int, VariantHasher> variant_map;
-	Map<Node *, int> node_map;
-	Map<Node *, int> nodepath_map;
+	Map<StringName,int> name_map;
+	HashMap<Variant,int,VariantHasher,VariantComparator> variant_map;
+	Map<Node*,int> node_map;
+	Map<Node*,int> nodepath_map;
 
 	//if using scene inheritance, pack the scene it inherits from
 	if (scene->get_scene_inherited_state().is_valid()) {

scene/resources/packed_scene.h

@@ -88,8 +88,8 @@ class SceneState : public Reference {
 
 	Vector<ConnectionData> connections;
 
-	Error _parse_node(Node *p_owner, Node *p_node, int p_parent_idx, Map<StringName, int> &name_map, HashMap<Variant, int, VariantHasher> &variant_map, Map<Node *, int> &node_map, Map<Node *, int> &nodepath_map);
-	Error _parse_connections(Node *p_owner, Node *p_node, Map<StringName, int> &name_map, HashMap<Variant, int, VariantHasher> &variant_map, Map<Node *, int> &node_map, Map<Node *, int> &nodepath_map);
+	Error _parse_node(Node *p_owner,Node *p_node,int p_parent_idx, Map<StringName,int> &name_map,HashMap<Variant,int,VariantHasher,VariantComparator> &variant_map,Map<Node*,int> &node_map,Map<Node*,int> &nodepath_map);
+	Error _parse_connections(Node *p_owner,Node *p_node, Map<StringName,int> &name_map,HashMap<Variant,int,VariantHasher,VariantComparator> &variant_map,Map<Node*,int> &node_map,Map<Node*,int> &nodepath_map);
 
 	String path;