Browse Source

Fixed glm::mask function and tests

Christophe Riccio 11 years ago
parent
commit
c36f3630eb
4 changed files with 39 additions and 12 deletions
  1. 3 6
      glm/detail/func_integer.inl
  2. 2 2
      glm/gtc/bitfield.inl
  3. 1 1
      test/core/core_func_integer.cpp
  4. 33 3
      test/gtc/gtc_bitfield.cpp

+ 3 - 6
glm/detail/func_integer.inl

@@ -45,9 +45,7 @@ namespace detail
 	template <typename T>
 	GLM_FUNC_QUALIFIER T mask(T Bits)
 	{
-		GLM_STATIC_ASSERT(!std::numeric_limits<T>::is_signed, "'Bits' type must be unsigned");
-
-		return ~((~static_cast<T>(0)) << Bits);
+		return Bits >= sizeof(T) * 8 ? ~static_cast<T>(0) : (static_cast<T>(1) << Bits) - static_cast<T>(1);
 	}
 
 	template <bool EXEC = false>
@@ -189,8 +187,7 @@ namespace detail
 	{
 		GLM_STATIC_ASSERT(std::numeric_limits<T>::is_integer, "'bitfieldExtract' only accept integer inputs");
 
-		T const Mask = static_cast<T>(detail::mask(detail::make_unsigned<T>::type(Bits)));
-		return (Value >> static_cast<T>(Offset)) & static_cast<T>(Mask);
+		return (Value >> static_cast<T>(Offset)) & static_cast<T>(detail::mask(Bits));
 	}
 
 	// bitfieldInsert
@@ -205,7 +202,7 @@ namespace detail
 	{
 		GLM_STATIC_ASSERT(std::numeric_limits<T>::is_integer, "'bitfieldInsert' only accept integer values");
 
-		T Mask = static_cast<T>(detail::mask(detail::make_unsigned<T>::type(Bits)) << Offset);
+		T const Mask = static_cast<T>(detail::mask(Bits) << Offset);
 		return (Base & ~Mask) | (Insert & Mask);
 	}
 

+ 2 - 2
glm/gtc/bitfield.inl

@@ -250,7 +250,7 @@ namespace detail
 	{
 		GLM_STATIC_ASSERT(std::numeric_limits<genIUType>::is_integer, "'mask' accepts only integer values");
 
-		return ~((~static_cast<genIUType>(0)) << Bits);
+		return Bits >= sizeof(genIUType) * 8 ? ~static_cast<genIUType>(0) : (static_cast<genIUType>(1) << Bits) - static_cast<genIUType>(1);
 	}
 
 	template <typename T, precision P, template <typename, precision> class vecIUType>
@@ -258,7 +258,7 @@ namespace detail
 	{
 		GLM_STATIC_ASSERT(std::numeric_limits<T>::is_integer, "'mask' accepts only integer values");
 
-		return ~((~static_cast<T>(0)) << v);
+		return detail::functor1<T, T, P, vecIUType>::call(mask, v);
 	}
 
 	template <typename genIType>

+ 1 - 1
test/core/core_func_integer.cpp

@@ -37,8 +37,8 @@ namespace bitfieldInsert
 
 	typeU32 const Data32[] =
 	{
-		{0x00000000, 0xffffffff,  0, 31, 0x7fffffff},
 		{0x00000000, 0xffffffff,  0, 32, 0xffffffff},
+		{0x00000000, 0xffffffff,  0, 31, 0x7fffffff},
 		{0x00000000, 0xffffffff,  0,  0, 0x00000000},
 		{0xff000000, 0x0000ff00,  8,  8, 0xff00ff00},
 		{0xffff0000, 0x0000ffff, 16, 16, 0x00000000},

+ 33 - 3
test/gtc/gtc_bitfield.cpp

@@ -33,7 +33,20 @@ namespace mask
 
 	inline int mask_mix(int Bits)
 	{
-		return Bits >= 32 ? 0xffffffff : (static_cast<int>(1) << Bits) - static_cast<int>(1);
+		return Bits >= sizeof(int) * 8 ? 0xffffffff : (static_cast<int>(1) << Bits) - static_cast<int>(1);
+	}
+
+	inline int mask_half(int Bits)
+	{
+		// We do the shift in two steps because 1 << 32 on an int is undefined.
+
+		int const Half = Bits >> 1;
+		int const Fill = ~0;
+		int const ShiftHaft = (Fill << Half);
+		int const Rest = Bits - Half;
+		int const Reversed = ShiftHaft << Rest;
+
+		return ~Reversed;
 	}
 
 	inline int mask_loop(int Bits)
@@ -86,15 +99,26 @@ namespace mask
 
 		std::clock_t Timestamp5 = std::clock();
 
+		{
+			std::vector<int> Mask;
+			Mask.resize(Count);
+			for(int i = 0; i < Count; ++i)
+				Mask[i] = mask_half(i % 32);
+		}
+
+		std::clock_t Timestamp6 = std::clock();
+
 		std::clock_t TimeMix = Timestamp2 - Timestamp1;
 		std::clock_t TimeLoop = Timestamp3 - Timestamp2;
 		std::clock_t TimeDefault = Timestamp4 - Timestamp3;
 		std::clock_t TimeZero = Timestamp5 - Timestamp4;
+		std::clock_t TimeHalf = Timestamp6 - Timestamp5;
 
 		printf("mask[mix]: %d\n", static_cast<unsigned int>(TimeMix));
 		printf("mask[loop]: %d\n", static_cast<unsigned int>(TimeLoop));
 		printf("mask[default]: %d\n", static_cast<unsigned int>(TimeDefault));
 		printf("mask[zero]: %d\n", static_cast<unsigned int>(TimeZero));
+		printf("mask[half]: %d\n", static_cast<unsigned int>(TimeHalf));
 
 		return TimeDefault < TimeLoop ? 0 : 1;
 	}
@@ -112,19 +136,25 @@ namespace mask
 		};
 
 		int Error(0);
-
+/* mask_zero is sadly not a correct code
 		for(std::size_t i = 0; i < sizeof(Data) / sizeof(type<int>); ++i)
 		{
 			int Result = mask_zero(Data[i].Value);
 			Error += Data[i].Return == Result ? 0 : 1;
 		}
-
+*/
 		for(std::size_t i = 0; i < sizeof(Data) / sizeof(type<int>); ++i)
 		{
 			int Result = mask_mix(Data[i].Value);
 			Error += Data[i].Return == Result ? 0 : 1;
 		}
 
+		for(std::size_t i = 0; i < sizeof(Data) / sizeof(type<int>); ++i)
+		{
+			int Result = mask_half(Data[i].Value);
+			Error += Data[i].Return == Result ? 0 : 1;
+		}
+
 		for(std::size_t i = 0; i < sizeof(Data) / sizeof(type<int>); ++i)
 		{
 			int Result = mask_loop(Data[i].Value);