type of crash: alignment exeption, how to fix ?

Glad to see that allowed you to go to the next steps ...

As you said, you will have to fix the part with MS3DTriangle, MS3DJoint, ...

Do you know if a working Linux PPC port exists?

@corto
Quote:

Yeah, it exists and working, through i do not test exactly that loader on linux ppc, but there is lot of loaders of all kin of 3d formats, and they mostly working (as they works on amigaos4).

Through i still think the reasson why that loader keepts as it , because probabaly linux ppc kernel emulate in softwate those unalignment access instead of crashes, so they just didn't noticed issues with that loader before

Just see it as a rule that any unaligned FPU access is problematic.

Seeing the various code snippets, isn't the one with the lwz asm faster or is your/corto's code optimized to lwbrx?

@Hedeon
Quote:

Dunno about speed, that need somehow to meassure it.. But probabaly differences not that big (expectually with CPUs which are faster than 100 mhz :) ).

@Corto

Did you maybe have any idea, of how to fix it all more cleaner : what i mean, is to avoid creating the same structures with the same name as it feels kind of bad. Those structs are generally only used for loading and discarded afterwards. So as long as we document the "real" type we wouldn't need a copy of struct .. Maybe something like this :



?

@kas1e

Ok, you're right about the management of this kind of exception by Linux.

About speed: no difference, that's a matter of few instructions at load time.

What I proposer could certainly be cleaner but how you can avoid a copy in a structure that add an abstraction level, as the initial structure is badly aligned and playing with fields and padding won't give the expected result.

I will have to look at your example as I'm not so familiar with C++ and so I'm not sure to understand what you want to do there.

@corto
Quote:

The idea behind the TPack1 template was to let the compiler create the copy and don't do it yourself. So we only have the structs with padding, no more PACK_STRUCT and irrPack includes. And for loading we create a new variable of that struct with TPack1 which creates a copy of the same struct without padding . After loading we copy that one back to the padded struct. On big endian just with = copy and on little endian with the swapping stuff and get_unaligned_le_float. And floats in the structs are replaced by the new union.

We probably anyway can forget about that TPack1 template thing,
maybe we not need it ? Just replace all float's with UnalignedFloat32() for example:




After we did that we get errors in any place that uses those floats. And in that place we can fix it. The byteswap can be done with the uval then. And for the assignment to the real structures we can use some function which either creates a copy or copies the bytes to the float.

When the float is used directly like with swapping the sign (+-) then we need to use a copy of UnalignedFloat32 in that place. Then copy it back with the uval. Or alternatively move the sign-swapping code completely to the place where it's assigned to the real mesh-vertices later.

@Corto
I at moment anyway trying to folllow just your way, as is not that i want to rewrite it all fully, just need to make that loader works :)

So, for the next structure i doing this:



Now there is original code for triangles:




That with what i currently come up with:



But there i crashes. I am sure something wrong there..

I mostly got the logic behind it, but not sure if i undersand things right how to skip first bytes (so see i count everything since pvrt[8], because structure have 1 u16 (2 bytes) and 3 u 16 (6 bytes), at top before floats starts).

Not sure if i not mess with those [x][x] things there, but probabaly everything is placed in memory one after another, so should be about right as i do it all ?

EDIT: thinking a bit more , probably that part is wrong in my rewrite:



I.e. i didn't align them as they not floats but u16, but then from another side, they now doesn't contain anything yet at that point (as we no longer start out with a pointer to the memory we loaded). So it contains random values probabaly .. ?

Your code looks difficult to read as you treat readings on a byte per byte basis with all those indiced pVPtr2[] access

You should better encapsulate your "read/reorder a float" in a single inline like Salass did :

static inline float GetFloat(void *ptr)
{
register union {
u8 u[4];
float f;
} tmp;

#ifdef __BIG_ENDIAN__
tmp.u[0] = ((u8 *)ptr)[3];
tmp.u[1] = ((u8 *)ptr)[2];
tmp.u[2] = ((u8 *)ptr)[1];
tmp.u[3] = ((u8 *)ptr)[0];
#else
tmp.f = *(float*)ptr;
#endif
return tmp.f;
}

of course it will need a triangle structure for read and another for writing the reordered floats but anyway the code will become more readable/maintainable

@thellier
You probably didnt read whole topic ?:)

Feel free to show on latest example from 2 posts above how to fix code with your getfloat().

We specially read byte per byte, for purpose.. The snipet you quote didnt fits here in code i want to fix now.

But of course, feel free plz to post code how you think it should ve in case of that MS3DTriangle structure and later byteswapping.

Currently i have no problems how code reads, i just need that boring and non-interesting shit works on our cpu. I have lots of ideas from everywhere, but at moment can only follow corto's way as it seems works (and i not about first problem which Salas00 help to fix, now its another issue, with different returns and in different case, so code you quote didnt fits)

fstruct MS3DTriangle
{
u16 Flags;
u16 VertexIndices[3];
float VertexNormals[3][3];
float S[3], T[3];
u8 SmoothingGroup;
u8 GroupIndex;
};
*/==========================================*/
static inline float GetFloat(void *ptr)
{
register union {
u8 u[4];
float f;
} tmp;

#ifdef __BIG_ENDIAN__
tmp.u[0] = ((u8 *)ptr)[3];
tmp.u[1] = ((u8 *)ptr)[2];
tmp.u[2] = ((u8 *)ptr)[1];
tmp.u[3] = ((u8 *)ptr)[0];
#else
tmp.f = *(float*)ptr;
#endif
return tmp.f;
}
*/==========================================*/
static inline float GetWord(void *ptr)
{
register union {
u8 u[4];
u16 w;
} tmp;

#ifdef __BIG_ENDIAN__
tmp.u[0] = ((u8 *)ptr)[1];
tmp.u[1] = ((u8 *)ptr)[0];
#else
tmp.w = *(u16 *)ptr;
#endif
return tmp.w;
}
*/==========================================*/

// I give only the important part :

register struct MS3DTriangle *tri=triangles;

#ifdef __BIG_ENDIAN__
for (u16 n=0; n<numTriangles; ++n)
{
tri->Flags = GetWord(&(tri->Flags));
for (u16 j=0; j<3; ++j)
{
tri->VertexIndices[j] = GetWord(&(tri->VertexIndices[j]));

tri->VertexNormals[j][0] = GetFloat(&(tri->VertexNormals[j][0]));
tri->VertexNormals[j][1] = GetFloat(&(tri->VertexNormals[j][1]));
tri->VertexNormals[j][2] = - GetFloat(&(tri->VertexNormals[j][2]));

tri->S[j] = GetFloat(&(tri->S[j]));
tri->T[j] = GetFloat(&(tri->T[j]));
}
tri++;
}
#endif

@thellier
Thanks! Will try

@Thellier
Doing as you show , and it didn't crash, there is:



And while it didn't crash, is it indeed right that we have "for (u16 n=0; n<numTriangles; ++n)" , but never use that "n" anywhere inside ?

Also should't we do somewhere later in the code, assignment of "tri" to "triangles" back, as this "triangles" used later eveyrwhere ?

Anyway, there next alignemnt crash (damn, why it can't be easy that i can just copy+paste the previous ways :) ):

https://sourceforge.net/p/irrlicht/cod ... 3DMeshFileLoader.cpp#l334

I tried to replace it like you did in example above, but still fail:



Probably because all those things now not on before of for(), but inside, it all mess the things ?

Also have a other question: why we use in previous example GetWords() ? I mean its only floats are unaligned, but u16 are aligned always (right ?).

@kas1e

I will be busy in the coming days but on the week-end, I will attend the Alchimie show here in France so I will be 2 days programming on Amiga with my friends (hey thellier!)

That could be nice to try to make it working there.

Could you provide a ms3d model? All files I find are corrupted archives or they failed to be extracted ...

@corto
There is one without material (there can be ones without material, so "empty" ones):

http://kas1e.mikendezign.com/temp/ms3d/barricade.ms3d

And there one with material (i put to archive some jpegs, one or two of them are used as material there):

http://kas1e.mikendezign.com/temp/ms3d/zmb.zip

But it need to be fixed exactly that file which we discuss, because we can then ask mantainer to put it to irrlicht's repo, and because i need exactly that loader working in whole engine.

Seems that Theiller's way also may work, just strange why it crashes on me on 3st structure when time to load materials (so material structure with 6 floats).

@theiller

Another question is : "register" got deprecated recently in C++. And it never was more than a hint to the compiler as one of devs told me. So is that really necesary here for us ? newer compilers should start ignoring it.

Also, don't lines like tri->VertexNormals[j][0] still write to a float which doesn't care about alignment? (don't know where this specific float sits now in memory, but basically - not sure that if it failed before why it can works now then ..)

@All
Are floating point exceptions only on reading the float, but we are allow to write ?

@kas1e
There isn't much benefit to using the register keyword nowadays. The optimizers in modern compilers are smart enough to put variables into registers whenever it's possible.

@BSZili
Yeah, register can be out , but i still have issues with understanding "don't lines like tri->VertexNormals[j][0] still write to a float which doesn't care about alignment? " ? Because as far as i understand for now alignemnt issues with floats mean and load and writing , right ?

When i read that article:
https://developer.ibm.com/articles/pa-dalign/

I can found:

Quote:

Ok "moder kernel" its not about amigaos4 one, but then they refer only "when asked to load an unaligned floating-point number from memory".

So did it mean only _reading_ are issues for alignment, not writing , so that "tri->VertexNormals[j][0] = getfloat(Balbal)" are fine ?


Also why that damn thing didn't crashes in first 2 strucutre parsings, but still crashes with alignemtn issues when i use it in "load materials" block. Yeah it different, as in first 2 stages where we load vertices and triangles we use struct[x].field->aaaa[x] , but then when we load materials its struct->field[x] , so maybe those GetFloats messed with pointers somehow dunno.. As well as "for" loop there a little bit different, but still about the same imho.

Damn why kernel just can't handle it all, let it me slower , its for sure should't be _that_ slower that anyone can notice visually imho , but we all will forget about possilbe issues with aligments ..

@Thellier
And don't we are trying to correct things in the wrong place with your example at all, i mean don't we simply byte reversing data in the same place, while that place still can be unaligned ?

@kas1e

Thank you for the links on models. I've sent you a modified loader by email, that could fix alignment problems in an easier way, padding the beginning of structures (still packed) in order to get the float fields aligned on 4 bytes. I dynamically allocate structures and copy the data to the address of the field Flags (just after the 3-byte padding).

I hope that will work!

@corto
Thanks for worry !

Through cheked, and it crashes with alignment exception when fill verticies. I.e. on that part:



And stack trace point out firstly on that line "vertices[tmp].Vertex[2] = -os::Byteswap::byteswap(vertices[tmp].Vertex[2]);" (i.e. last one), and then on memcpy line.