g-truc / glm

OpenGL Mathematics (GLM)
https://glm.g-truc.net
Other
9.27k stars 2.13k forks source link

VS2013 warning C4316 object allocated on the heap may not be aligned 16 #322

Closed bonizz closed 9 years ago

bonizz commented 9 years ago

Not sure if this is the same issue as https://github.com/g-truc/glm/issues/316

I upgraded from 0.9.5.4 to 0.9.6.3 and get this warning: warning C4316: 'B' : object allocated on the heap may not be aligned 16

Here's a simplified code sample which gives this warning:

class A
{
};

class B : public A
{
    glm::vec4 a;
};

int main()
{
    B *t = new B();
    return 0;
}

Is this a bug? Or are there new alignment requirements?

Thanks

tomveltmeijer commented 9 years ago

This seems to be the same issue as #235, where allocating a vec4 or mat4 on the heap causes warning C4316. Not sure if #316 is the same issue, but the workaround provided in the comment section there does get rid of this warning for me.

engineer0x47 commented 9 years ago

See my comment on #235 . This is not an issue unless you are attempting to run SSE instructions on it.

CrossVR commented 9 years ago

This issue breaks GLI with VS2013, it can no longer correctly read headers because the struct size of the DDS header is incorrect.

Groovounet commented 9 years ago

Are you using master branch of GLI?

Groovounet commented 9 years ago

I am not quite sure how this issue affects GLI.

CrossVR commented 9 years ago

Because of this bug, this member becomes larger than it should be: https://github.com/g-truc/gli/blob/master/gli/core/load_dds.inl#L78

I'm using g-truc/gli@2c317e7aa7e17a96faed6a8f4e995218ccb7bba3, I could update but I don't think that would help.

Groovounet commented 9 years ago

I'll update GLI to use master branch of GLM and that should fixed the problem.

Groovounet commented 9 years ago

This bug should be fixed by https://github.com/g-truc/glm/commit/c10df14b5855e8b0b7c4ad47968e828462ddf12e

I forgot to reference this bug number in the commit...

Thanks for reporting, Christophe

CrossVR commented 9 years ago

@Groovounet I recommend to also add a static assert on the size of that struct, to ensure it can't happen again.

Groovounet commented 9 years ago

Sounds good!

CrossVR commented 9 years ago

You can do it like this and check the entire ddsHeader struct at once: https://github.com/Microsoft/DirectXTex/blob/1167ebb9c7615a1f515903b12e61ab3e98209315/DirectXTex/DDS.h#L234