fix vmap assembly on 64bit systems

[Lynx3d]

Since for some strange reason the vmap_assebler.exe through wine needs 12h+, i decided to find out why it didn't work as 64bit linux binary. Turns out the reason is that it reads the memory dumps from 32bit systems, but doesn't write it 32bit compatible, so i implemented this.

Might not be the prettiest solution, but remains compatibility with existing vmaps.

Tested with [8622] on 64bit Ubuntu

>patch file<

diff --git a/src/shared/vmap/ModelContainer.cpp b/src/shared/vmap/ModelContainer.cpp
index ff4f935..8c07482 100644
--- a/src/shared/vmap/ModelContainer.cpp
+++ b/src/shared/vmap/ModelContainer.cpp
@@ -227,10 +227,15 @@ namespace VMAP
            if(result && fwrite(getTriangles(),sizeof(TriangleBox),getNTriangles(),wf) != getNTriangles()) result = false;

            if(result && fwrite("SUBM",4,1,wf) != 1) result = false;
-            size = sizeof(unsigned int)+ sizeof(SubModel)*iNSubModel;
+            size = sizeof(unsigned int) + SubModel::DumpSize*iNSubModel;
            if(result && fwrite(&size,4,1,wf) != 1) result = false;
            if(result && fwrite(&iNSubModel,sizeof(unsigned int),1,wf) != 1) result = false;
-            if(result && fwrite(iSubModel,sizeof(SubModel),iNSubModel,wf) != iNSubModel) result = false;
+            for(int i=0; i<iNSubModel; ++i)
+            {
+                uint8 triBuff[subModel::DumpSize];
+                iSubModel[i].putToBinBlock(triBuff);
+                if(result && fwrite(triBuff,SubModel::DumpSize,1,wf) != 1) result = false;
+            }

            fclose(wf);
        }
@@ -303,7 +308,7 @@ namespace VMAP
            {
                for(unsigned int i=0;i<iNSubModel && result; ++i)
                {
-                    unsigned char readBuffer[52];           // this is the size of SubModel on 32 bit systems
+                    unsigned char readBuffer[subModel::DumpSize];           // this equals the size of SubModel on 32 bit systems
                    if(fread(readBuffer,sizeof(readBuffer),1,rf) != 1) result = false;
                    iSubModel[i].initFromBinBlock(readBuffer);
                    iSubModel[i].setTriangleArray(getTriangles());
diff --git a/src/shared/vmap/SubModel.cpp b/src/shared/vmap/SubModel.cpp
index 17ca10f..d937878 100644
--- a/src/shared/vmap/SubModel.cpp
+++ b/src/shared/vmap/SubModel.cpp
@@ -17,6 +17,7 @@
 */

#include "SubModel.h"
+#include <cstring>

#ifdef _ASSEMBLER_DEBUG
extern FILE *::g_df;
@@ -52,6 +53,8 @@ namespace VMAP
    //==========================================================
    //==========================================================
    //==========================================================
+    const unsigned int SubModel::DumpSize;
+    
    SubModel::SubModel(unsigned int pNTriangles, TriangleBox *pTriangles, unsigned int pTrianglesPos, unsigned int pNNodes, TreeNode *pTreeNodes, unsigned int pNodesPos) :
    BaseModel(pNNodes, pTreeNodes, pNTriangles, pTriangles)
    {
@@ -111,6 +114,13 @@ namespace VMAP
        iHasInternalMemAlloc = *((bool *) (((char *) pBinBlock) + BP_iHasInternalMemAlloc));
        iBox =  *((ShortBox *) (((char *) pBinBlock) + BP_iBox));
    }
+    
+    void SubModel::PutToBinBlock(void *pBinBlock)
+    {
+        // pointers of SubModel are redundant, but existing format expects 2*32bit (8 Bytes)
+        memcpy(pBinBlock, "\\0\\0\\0\\0\\0\\0\\0\\0",8);
+        memcpy((uint8*)pBinBlock+8, &this->iNTriangles,52-8);
+    }

    //==========================================================

diff --git a/src/shared/vmap/SubModel.h b/src/shared/vmap/SubModel.h
index eff1403..7747711 100644
--- a/src/shared/vmap/SubModel.h
+++ b/src/shared/vmap/SubModel.h
@@ -54,6 +54,7 @@ namespace VMAP
            ~SubModel(void);
            //Gets a 50 byte binary block
            void initFromBinBlock(void *pBinBlock);
+            void putToBinBlock(void *pBinBlock);

            void fillRenderArray(G3D::Array<TriangleBox> &pArray, const TreeNode* pTreeNode);

@@ -92,6 +93,7 @@ namespace VMAP
            void intersectRay(const G3D::Ray& ray, RayCallback& intersectCallback, float& distance, bool pStopAtFirstHit, bool intersectCallbackIsFast = false);
            bool operator==(const SubModel& pSm2) const;
            unsigned int hashCode() const { return BaseModel::getNTriangles(); }
+            static const unsigned int dumpSize = 52;
    };

    unsigned int hashCode(const SubModel& pSm);

Notes:

• * while it is completely redundant to dump the BaseModel pointers which cause the incompatibility in the first place, i decided to not change the format so your existing vmaps should work. For more sophisticated dumping, SubModel::initFromBinBlock(), SubModel::PutToBinBlock() and SubModel::DumpSize of SubModel have to be changed only.
  * vmaps are still endian specific, vmaps from PPC will not work on Intel architectures and vice versa
  

For compiling vmap_assembler i used this simple SCons script after compiling MaNGOS in objdir (as suggested by the guides):

import os

env = Environment(ENV=os.environ, CPPPATH = ['.', '../../src/shared/vmap', '../../dep/include/g3dlite'])

env.Program(target = 'vmap_assembler', 
       source = ['vmap_assembler.cpp'],
       LIBS = ['mangosvmaps','g3dlite'],
       LIBPATH = ['../../objdir/src/shared/vmap','../../objdir/dep/src/g3dlite'])

Now the only thing left is the vmap_extractor...it's slow too but not quite that slow as the assembler was.

[VladimirMangos]

I worry that this still not let compatible results for platforms.

if(result && fwrite(&val,sizeof(unsigned int),1,wf) != 1) result = false;

but sizeof(unsigned int) is platform dependent size

[XTZGZoReX]

Yes, we must either:

a) change code in core to use platform-dependent integer sizes.

or

b) change vmap extractor/assembler to use typedef'd integers.

[VladimirMangos]

(a) will not help for case: generated files at one host (under Windows as more easy case) still will not be compatible with another host mangosd.

Not inmportant infact how core work with data, important only how it read from file/write to file

[XTZGZoReX]

(a) will not help for case: generated files at one host (under Windows as more easy case) still will not be compatible with another host mangosd.

Yes, I forgot to mention that... So (b) is how it should be done.

[Lynx3d]

I worry that this still not let compatible results for platforms.

but sizeof(unsigned int) is platform dependent size

Well yes, the size of "int" is not set in stone, although it i can't name a relevant platform where it differs from 32bit...

As i said, it doesn't make the vmap code really platform independent because there's at least still the endian problem too.

The main goal was to make the code read and write the vmaps in the same format on the same platform.

Before, on 64bit linux for example, the written vmaps could not be read again on the very same system.

Since it uses sizeof(unsigned int) for writing and reading, it should work at least in this regard.

But maybe it's time for some bigger cleanup than this...

[XTZGZoReX]

Well yes, the size of "int" is not set in stone, although it i can't name a relevant platform where it differs from 32bit...

It's 8 bytes on most 64-bit systems, as well as PPC archs.

In the end, the best cleanup of this would be to make both extractor, assembler, and core code use uint32/uint64/etc appropriately where needed, so we always know what size variables are written and read as.

[Lynx3d]

It's 8 bytes on most 64-bit systems, as well as PPC archs.

Hm i beg to differ.

From http://en.wikipedia.org/wiki/64-bit

Data model  short    int    long    long long    pointers    Sample operating systems
LLP64       16       32     32      64           64          Microsoft Win64 (X64/IA64)
LP64        16       32     64      64           64          Most Unix (like Solaris) and Unix-like systems (like Linux)

Many 64-bit compilers today use the LP64 model (including Solaris, AIX, HP-UX, Linux, Mac OS X, FreeBSD, and IBM z/OS native compilers). Microsoft's VC++ compiler uses the LLP64 model.

But i agree, proper integer defines never hurt.

Maybe i'll try to make code more strict, am going to switch to 3.3 soon now

[XTZGZoReX]

Wikipedia article only seems to mention most common OS's - but there is in fact no real standard that guarantees you these sizes, anyway. You can't just assume that they'll be the size you want.

[darkstalker]

use types defined in stdint.h

[XTZGZoReX]

The ones in our Define.h are just fine.

[Lynx3d]

Well if stdint.h was actually part of the C++ standard...but it's only officially part of C99. So assuming its existence somewhat just as portable as assuming that int is 32bit IMHO

Hm g3dmath.h which is indirectly included already also defines int32 etc. so including Define.h will probably conflict.

Though the definition in g3dmath.h is as sophisticated as "typedef int int32;"...so you might aswell just stay with (unsigned) int in vmap code...

Any good idea left?

[XTZGZoReX]

Well if stdint.h was actually part of the C++ standard...but it's only officially part of C99. So assuming its existence somewhat just as portable as assuming that int is 32bit IMHO

Agreed.

Hm g3dmath.h which is indirectly included already also defines int32 etc. so including Define.h will probably conflict.

Aren't they in the G3D namespace?

[Lynx3d]

Aren't they in the G3D namespace?

D'oh...yes you're totally right.

It's just a matter of replacing "using namespace G3D;" with some more selective "using" directives.

Btw. i just used my above patch with [9175], still working fine for me on 64bit Linux. So i think i'll go from there and use fixed width integers from Define.h where apropriate.

[XTZGZoReX]

Yep, sounds good.

[kaio94700]

Hello its work for new core ? and its fix fly mob ?

Thanx you

[Lynx3d]

I just finished cleaning up the integer types (i hope), and made made sure unused bytes are not written as random garbage. Now vmaps should be identical on each run.

Any platform should now be able to read the vmaps that have been created on it, and existing vmaps from win32 vmap_assembler should still work on all little-endian machines (32bit and 64bit compiles).

I don't know if it builds without any modifications on windows, but the Makefiles already had all include paths set for the ACE integer definitions.

Would be nice to get some feedback.

I created a branch on my github fork:

http://github.com/Lynx3d/mangos/tree/vmap_cleanup

Next step could be bumping the vmap version and drop the unused bytes aswell as adding endian conversion so vmaps are truly cross-platform.

Since there's already "magic bytes" in the file headers, it can be made backwards compatible.

@kaio: Uhm...are you sure you posted in the right thread?

[Lynx3d]

*scratches head*

I think this one is really outdated now...I ended up with a slightly more extensive "fix"

*grabs big "rejected" stamp again*

[VladimirMangos]

extensive "fix"