swordlegend / recastnavigation

Automatically exported from code.google.com/p/recastnavigation
zlib License
0 stars 0 forks source link

Endian swap routines are incorrect #37

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I'm working on saving a nav mesh with everything swapped to big endian and
noticed that the swapEndian routines weren't actually swapping anything.

The swap routines currently look like:

inline void swapEndian(int* v)
{
    unsigned char* x = (unsigned char*)&v;
    swapByte(x+0, x+3); swapByte(x+1, x+2);
}

The problem is they are taking the address of v and swapping it rather than
the data v points to. This can be fixed by not dereferencing v, e.g.:

inline void swapEndian(int* v)
{
    unsigned char* x = (unsigned char*)v;
    swapByte(x+0, x+3); swapByte(x+1, x+2);
}

A couple of other points which I should possibly open another issue for but
are related.

It might be nice if the swapEndian routines were in a header so other
modules could use them (I'm writing some serialisation stuff in another file).

The dtNavMeshHeaderSwapEndian and dtNavMeshDataSwapEndian functions presume
the user will want to swap endian on load, as it performs a swap on the
magic number and version before checking them. However I would prefer to
convert to my target platforms endianness on save so I don't need to do any
endian conversion on load in my game. It might be nice to change the
interface of these functions somehow so the user can specify which way they
are going.

Original issue reported on code.google.com by cameron....@gmail.com on 31 Jan 2010 at 10:30

GoogleCodeExporter commented 9 years ago
Thanks for the bug report.

For some reason I changed the input values from references to pointer at some 
point
(probably after I had tested the code). Fixes are in in R 113.

The functions are exposed in DetourNavMeshBuilder.h and should be used as 
follows
(there was a bug in there which made the other direction not to work). The 
intention
is to allow them to be used both when creating content as well as when loading.

// Native to foreign.
dtNavMeshDataSwapEndian(navData, navDataSize);
dtNavMeshHeaderSwapEndian(navData, navDataSize);

// Foreign to native.
dtNavMeshHeaderSwapEndian(navData, navDataSize);
dtNavMeshDataSwapEndian(navData, navDataSize);

I don't have any other way to currently test these other than inspecting some 
binary
dumps, let me know if there are other problems/wishes. 

Original comment by memono...@gmail.com on 1 Feb 2010 at 10:34

GoogleCodeExporter commented 9 years ago
Thanks for the fix, I will let you know if I come across any other endian 
problems.
I've still not quite got Detour going on PowerPC. My initial testing was done by
saving a mesh as big endian and loading it (swapping it again) into the sample 
app
again on PC.

Original comment by cameron....@gmail.com on 2 Feb 2010 at 7:35