color conversion: avoid type punning by returning SColor4ub directly. that required a non-extern-C declaration of the SSE function, so i replaced it with straightforward intrinsics.

ia32: no longer needs #if since build system ensures it's not compiled
on amd64

This was SVN commit r6164.

source/graphics/Color.cpp

@@ -12,25 +12,56 @@
 
 #include "maths/MathUtil.h"
 #include "graphics/SColor.h"
-#include "lib/sysdep/arch/x86_x64/x86_x64.h"
 
-static u32 fallback_ConvertRGBColorTo4ub(const RGBColor& src)
+#if ARCH_IA32 || ARCH_AMD64
+# include <emmintrin.h>
+# include "lib/sysdep/arch/x86_x64/x86_x64.h"
+#endif
+
+static SColor4ub fallback_ConvertRGBColorTo4ub(const RGBColor& src)
 {
 	SColor4ub result;
 	result.R=clamp(int(src.X*255),0,255);
 	result.G=clamp(int(src.Y*255),0,255);
 	result.B=clamp(int(src.Z*255),0,255);
 	result.A=0xff;
-	return *(u32*)&result;
+	return result;
 }
 
 // on IA32, this is replaced by an SSE assembly version in ia32.cpp
-u32 (*ConvertRGBColorTo4ub)(const RGBColor& src) = fallback_ConvertRGBColorTo4ub;
+SColor4ub (*ConvertRGBColorTo4ub)(const RGBColor& src) = fallback_ConvertRGBColorTo4ub;
 
 
 // Assembler-optimized function for color conversion
-#if ARCH_IA32
-EXTERN_C u32 sse_ConvertRGBColorTo4ub(const RGBColor& src);
+#if ARCH_IA32 || ARCH_AMD64
+static SColor4ub sse_ConvertRGBColorTo4ub(const RGBColor& src)
+{
+	const __m128 zero = _mm_setzero_ps();
+	const __m128 _255 = _mm_set_ss(255.0f);
+	__m128 r = _mm_load_ss(&src.X);
+	__m128 g = _mm_load_ss(&src.Y);
+	__m128 b = _mm_load_ss(&src.Z);
+
+	// C = min(255, 255*max(C, 0)) ( == clamp(255*C, 0, 255) )
+	r = _mm_max_ss(r, zero);
+	g = _mm_max_ss(g, zero);
+	b = _mm_max_ss(b, zero);
+
+	r = _mm_mul_ss(r, _255);
+	g = _mm_mul_ss(g, _255);
+	b = _mm_mul_ss(b, _255);
+
+	r = _mm_min_ss(r, _255);
+	g = _mm_min_ss(g, _255);
+	b = _mm_min_ss(b, _255);
+
+	// convert to integer and combine channels using bit logic
+	int ri = _mm_cvtss_si32(r);
+	int gi = _mm_cvtss_si32(g);
+	int bi = _mm_cvtss_si32(b);
+
+	return SColor4ub(ri, gi, bi, 0xFF);
+}
 #endif
 
 void ColorActivateFastImpl()
@@ -38,7 +69,7 @@ void ColorActivateFastImpl()
 	if(0)
 	{
 	}
-#if ARCH_IA32
+#if ARCH_IA32 || ARCH_AMD64
 	else if (x86_x64_cap(X86_X64_CAP_SSE))
 	{
 		ConvertRGBColorTo4ub = sse_ConvertRGBColorTo4ub;

source/graphics/Color.h

@@ -9,6 +9,7 @@
 #ifndef INCLUDED_COLOR
 #define INCLUDED_COLOR
 
+#include "SColor.h"
 #include "maths/Vector3D.h"
 #include "maths/Vector4D.h"
 
@@ -19,7 +20,7 @@ typedef CVector4D RGBAColor;
 
 // exposed as function pointer because it is set at init-time to
 // one of several implementations depending on CPU caps.
-extern u32 (*ConvertRGBColorTo4ub)(const RGBColor& src);
+extern SColor4ub (*ConvertRGBColorTo4ub)(const RGBColor& src);
 
 // call once ia32_Init has run; detects CPU caps and activates the best
 // possible codepath.

source/graphics/Color_asm.asm

@@ -1,50 +0,0 @@
-%include "../lib/sysdep/arch/ia32/ia32.inc"
-
-;-------------------------------------------------------------------------------
-; Color conversion (SSE)
-;-------------------------------------------------------------------------------
-
-; extern "C" u32 ConvertRGBColorTo4ub(const RGBColor& color)
-[section .data]
-	align	16
-zero:
-	dd	0.0
-twofivefive:
-	dd	255.0
-
-
-__SECT__
-	align	16
-global sym(sse_ConvertRGBColorTo4ub)
-sym(sse_ConvertRGBColorTo4ub):
-	mov	eax, [esp+4]
-
-	; xmm0, 1, 2 = R, G, B
-	movss	xmm4, [zero]
-	movss	xmm0, [eax+8]
-	movss	xmm1, [eax+4]
-	movss	xmm2, [eax]
-	movss	xmm5, [twofivefive]
-
-	; C = min(255, 255*max(C, 0)) ( == clamp(255*C, 0, 255) )
-	maxss	xmm0, xmm4
-	maxss	xmm1, xmm4
-	maxss	xmm2, xmm4
-	mulss	xmm0, xmm5
-	mulss	xmm1, xmm5
-	mulss	xmm2, xmm5
-	minss	xmm0, xmm5
-	minss	xmm1, xmm5
-	minss	xmm2, xmm5
-
-	; convert to integer and combine channels using bit logic
-	cvtss2si eax, xmm0
-	cvtss2si ecx, xmm1
-	cvtss2si edx, xmm2
-	shl	eax, 16
-	shl	ecx, 8
-	or	eax, 0xff000000
-	or	edx, ecx
-	or	eax, edx
-
-	ret

source/graphics/tests/test_Color.h

@@ -12,12 +12,20 @@ public:
 
 	void test_Color4ub()
 	{
-#define T(r, g, b, ub) TS_ASSERT_EQUALS(ub | 0xff000000, ConvertRGBColorTo4ub(RGBColor(r,g,b)))
-		T(0, 0, 0, 0x000000);
-		T(1, 0, 0, 0x0000ff);
-		T(0, 1, 0, 0x00ff00);
-		T(0, 0, 1, 0xff0000);
-		T(1, 1, 1, 0xffffff);
-#undef T
+		CheckColor(0, 0, 0, 0x000000);
+		CheckColor(1, 0, 0, 0x0000ff);
+		CheckColor(0, 1, 0, 0x00ff00);
+		CheckColor(0, 0, 1, 0xff0000);
+		CheckColor(1, 1, 1, 0xffffff);
+	}
+
+private:
+	void CheckColor(int r, int g, int b, u32 expected)
+	{
+		SColor4ub colorStruct = ConvertRGBColorTo4ub(RGBColor(r,g,b));
+		u32 actual;
+		memcpy(&actual, &colorStruct, sizeof(u32));
+		expected |= 0xff000000;	// ConvertRGBColorTo4ub sets alpha to opaque 
+		TS_ASSERT_EQUALS(expected, actual);
 	}
 };

source/lib/sysdep/arch/ia32/ia32.cpp

@@ -10,8 +10,6 @@
 
 #include "precompiled.h"
 
-#if ARCH_IA32
-
 #include "ia32.h"
 
 #include "lib/sysdep/cpu.h"
@@ -114,5 +112,3 @@ void* cpu_memcpy(void* RESTRICT dst, const void* RESTRICT src, size_t size)
 {
 	return ia32_memcpy(dst, src, size);
 }
-
-#endif // ARCH_IA32

source/renderer/ModelRenderer.cpp

@@ -116,7 +116,7 @@ void ModelRenderer::BuildColor4ub(
 			tempcolor.X *= shadingColor.r;
 			tempcolor.Y *= shadingColor.g;
 			tempcolor.Z *= shadingColor.b;
-			*(u32*)&Color[j] = ConvertRGBColorTo4ub(tempcolor);
+			Color[j] = ConvertRGBColorTo4ub(tempcolor);
 		}
 	}
 	else
@@ -127,7 +127,7 @@ void ModelRenderer::BuildColor4ub(
 			tempcolor.X *= shadingColor.r;
 			tempcolor.Y *= shadingColor.g;
 			tempcolor.Z *= shadingColor.b;
-			*(u32*)&Color[j] = ConvertRGBColorTo4ub(tempcolor);
+			Color[j] = ConvertRGBColorTo4ub(tempcolor);
 		}
 	}
 }

source/renderer/PatchRData.cpp

@@ -376,7 +376,7 @@ void CPatchRData::BuildVertices()
 
 			RGBColor diffuse;
 			lightEnv.EvaluateDirect(normal, diffuse);
-			*(u32*)&vertices[v].m_DiffuseColor = ConvertRGBColorTo4ub(diffuse);
+			vertices[v].m_DiffuseColor = ConvertRGBColorTo4ub(diffuse);
 		}
 	}