10-27-08 - 4

God damn casts are the fucking devil. I just found a huge bug in Galaxy3 where I was assuming that one struct was a prefix of another struct. In particular I was assuming that a vertex with Diffuse, 2 UVs, and a Normal, had as its prefix a vert with just Diffuse and a UV. Eight years ago or so when I wrote it, that was true. Then four years ago I changed some of the structs and it became broken, and I only just found it now. Yuck.

Obviously you want to avoid casts, but sometimes you have to do them. One thing I've started doing is making little helper functions instead of just casting. Like :

gVertexDUV & gVertPrefixDUV( gVertexDUV2N & vertex )
	return (gVertexDUV &) vertex;

then use it like :

FuncOnDUV( gVertPrefixDUV( m_vertexDUV2N ) );

instead of just doing :

FuncOnDUV( (gVertexDUV &) ( m_vertexDUV2N ) );

Now, that on its own already does a little bit for you, because it means that if you change the type of m_vertexDUV2N you will get a compile error, unlike the normal cast which will just keep casting anything. That is, the function more narrowly specifies, I want to cast X to Y, not just a cast of any type to any other type (you could get the same constraint by using static_cast and specifying both template arguments manually instead of letting one be inferred).

But the real win comes if you start checking the cast in the function :

// MEMBER_OFFSET tells you the offset of a member in a type
#define MEMBER_OFFSET(type,member)	( (size_t) &(((type *)0)->member) )

gVertexDUV & gVertPrefixDUV( gVertexDUV2N & vertex )
	COMPILER_ASSERT( sizeof(gVertexDUV) <= sizeof(gVertexDUV2N) );
	COMPILER_ASSERT( MEMBER_OFFSET(gVertexDUV,diffuse) == gVertexDUV(gVertexDUV2N,diffuse) );
	COMPILER_ASSERT( MEMBER_OFFSET(gVertexDUV,uv1) == gVertexDUV(gVertexDUV2N,uv1) );
	return (gVertexDUV &) vertex;

Stuff like that. Now you have a semi-safe cast that will warn you if you change the structs and mess up the cast.

I've also been using a little utility that looks like static_cast<> but ensures the value is of the same size :

// same_size_cast like static_cast or whatever
template < typename t_to, typename t_fm >
t_to same_size_cast( const t_fm & from )
	RR_COMPILER_ASSERT( sizeof(t_to) == sizeof(t_fm) );
	return *( (const t_to *) &from );

That's okay. I really hate the compiler warning about settings signed to unsigned and all that, because it makes me put casts in to make the warning go away, and that actually makes the code much less safe, because now it's casting god knows what if I change the type.


Ivan-Assen said...

Seed your random by a concatenated sorted list of all filenames and sizes?

cbloom said...

Yeah, certainly I could seed and generate the same permutation every time. But I also want to remember your spot in the permutation. Well that's just an int, I guess I could just stick that in the registry or something. So yeah, you're right it's pretty easy, I'll do that.

cbloom said...

Ok this is done in the new cbsaver.

Some day I should make the blit use hardware acceleration so its not so ungodly slow.

old rants