Torque 3D's Achilles heel
by Konrad Kiss · in Torque 3D Professional · 03/24/2012 (11:49 am) · 184 replies
Recently I spent some time getting myself familiar with static code analysis and the more common (indie-budget freindly) tools associated with the topic. #AltDevBlogADay had several posts about such tools by Bruce Dawson and John Carmack. Definitely worth checking out.
So, it all began with me having a very elusive bug somewhere in code. I hoped that it would be something that a static code analyzer would pick up, so I lined up what I could from trial versions to open source software, and got some amazing results once these went through the Torque 3D 1.2 codebase.
I can't remember if the actual problem was found by these tools (it's been a while ago). What I recall is being taken aback by the results I got. But first a disclaimer...
I've been reviewing engine code changes for the past 4 years. Torque 3D is huge. One of the most surprising thing was the fact, that these code analyzers could - proportionally - find only a few problems. However, the few they did find were hard to see even when I knew the problem.
The kinds of problems I found primarily were:
- Uninitialized member variables used without initialization
- Checking the existence of an object and using the same object outside the check condition
- Passing local pointers that could become invalid
- Using && instead of &, || instead of | operations
- Bad casts for variables while the code assumes another cast (ie. U32 vs S32 - especially when negative values are used for error codes - common in some of the linked libraries)
- Typos, cut & paste programming mistakes that compile (probably the most vicious of all)
I'm only going to list my favorites, hoping that this will be a big enough motivation for everyone to start using some basic form of static code analysis. The complete list would be way too long for me to list here.
gfxD3D9TextureManager: using boolean evaluation against variables that were clearly intended to be bitmasks.
guiIconButtonCtrl.cpp: find the highlighted button, I dare you
processedFFMaterial.cpp: making sure (though what about blendDest?)
mRectF.cpp: intersectTriangle
winDInputDevice.cpp: POV_down, not much of a choice (twice in the same file)
rectClipper: clipRect (near eof)
win32Window.cpp: equilibrium
Finally - I need to clarify - by all means, no disrespect was intended towards Torque 3D developers. I would have done far worse most likely. I can't count the many new things in the source that made me go "Wow" and "Yey" - it is clearly visible that the team is bigger and better at what they do. Torque 1.2 is the best Torque yet - and I mean it by comparing the overall usability, the number of bugs, etc..
With this post I'm only trying to point at a solution that can save everyone from such blatant bugs in the future, while - perhaps - also nudging GG a little to take a step in the right direction. ;)
Cheers
--Konrad
So, it all began with me having a very elusive bug somewhere in code. I hoped that it would be something that a static code analyzer would pick up, so I lined up what I could from trial versions to open source software, and got some amazing results once these went through the Torque 3D 1.2 codebase.
I can't remember if the actual problem was found by these tools (it's been a while ago). What I recall is being taken aback by the results I got. But first a disclaimer...
I've been reviewing engine code changes for the past 4 years. Torque 3D is huge. One of the most surprising thing was the fact, that these code analyzers could - proportionally - find only a few problems. However, the few they did find were hard to see even when I knew the problem.
The kinds of problems I found primarily were:
- Uninitialized member variables used without initialization
- Checking the existence of an object and using the same object outside the check condition
- Passing local pointers that could become invalid
- Using && instead of &, || instead of | operations
- Bad casts for variables while the code assumes another cast (ie. U32 vs S32 - especially when negative values are used for error codes - common in some of the linked libraries)
- Typos, cut & paste programming mistakes that compile (probably the most vicious of all)
I'm only going to list my favorites, hoping that this will be a big enough motivation for everyone to start using some basic form of static code analysis. The complete list would be way too long for me to list here.
gfxD3D9TextureManager: using boolean evaluation against variables that were clearly intended to be bitmasks.
if (retTex->mProfile->isRenderTarget() && mslevel != 0 && (mDeviceCaps.Caps2 && D3DDEVCAPS2_CAN_STRETCHRECT_FROM_TEXTURES))
guiIconButtonCtrl.cpp: find the highlighted button, I dare you
ColorI fontColor = mActive ? (highlight ? mProfile->mFontColor : mProfile->mFontColor) : mProfile->mFontColorNA;
processedFFMaterial.cpp: making sure (though what about blendDest?)
result.blendSrc = GFXBlendOne;
result.blendSrc = GFXBlendOne;mRectF.cpp: intersectTriangle
if(contains(a) || contains(b) || contains(b))
winDInputDevice.cpp: POV_down, not much of a choice (twice in the same file)
newEvent.objInst = ( objInst == 0 ) ? SI_DPOV : SI_DPOV
rectClipper: clipRect (near eof)
out_rRect.extent.x = bottomR.x - out_rRect.point.x + 1; out_rRect.extent.x = bottomR.y - out_rRect.point.y + 1;
win32Window.cpp: equilibrium
newLeft = mClamp(newLeft, 0, newLeft);
newTop = mClamp(newLeft, 0, newTop);Finally - I need to clarify - by all means, no disrespect was intended towards Torque 3D developers. I would have done far worse most likely. I can't count the many new things in the source that made me go "Wow" and "Yey" - it is clearly visible that the team is bigger and better at what they do. Torque 1.2 is the best Torque yet - and I mean it by comparing the overall usability, the number of bugs, etc..
With this post I'm only trying to point at a solution that can save everyone from such blatant bugs in the future, while - perhaps - also nudging GG a little to take a step in the right direction. ;)
Cheers
--Konrad
About the author
http://about.me/konrad.kiss
#162
Changelog:
06/07/2012 (7:43 am)
A good commit on the Torque3D CEChangelog:
- [Optimizations] - Improved some iterator
- [Optimizations] - Removed unnecessary multiple assignments of values
- [Optimizations] - Fixed some logical errors
- [Optimizations] - Removed some checks unnecessarily repeated several times
- [Optimizations] - Redefined the AssertFatal? function
- [Optimizations] - Replaced unnecessary calls with assignments
- [Optimizations] - Declared as virtual some destructors, so as to free the memory if inherited objects are deleted.
#163
06/07/2012 (8:59 am)
Great work Alfio! Now that's the kind of thing I was hoping to see as a result of this thread :)
#164
I think I just ate your bandwidth - should've been more careful there.
06/07/2012 (11:43 am)
200mb versioned --- ooops!I think I just ate your bandwidth - should've been more careful there.
awkward smile
#165
I have over 2.4T pro month. And the server hosts only the services used by the trac and svn
06/07/2012 (12:02 pm)
Is a hosted server in a serverfarm, the bandiwidht isn't a problem :DI have over 2.4T pro month. And the server hosts only the services used by the trac and svn
#166
Sorry :P
06/07/2012 (12:13 pm)
I forgot to add to the changelog, that i have removed the flush of a query on the d3d device. And that in case of re-build of the solution, you first need to re-generate it with the appropriate generateProjects.bat file.Sorry :P
#170
06/17/2012 (3:58 am)
oobs.. double post
#171
EDIT 2:
OOPs, nop not yet, but i'm on to it:-
EDIT 1: Prefect Alfio, exactly as you said!
As Alfio say in the note below: generateProjects.bat and a fresh (recursive) download fixed the problem.
EDIT 1: EDIT 2:
EDIT3:
Hello and great effort.
One simple question (probably something i miss:-).
If i have a default T3D 1.2 installation (which compiles ok) and drop all the files from changelog 64 then i can't compile anymore...
I'm running on a 64b Win7 E box, with TFS2010 and VS 2010 U.
This is the first error messsage. ( I can fix some of them but the amount is to big..)
Error 11 error C2039: 'CompressImageOMP' : is not a member of 'squish' d:torquecommunitytorque 3d 1.2enginelibsquishsquishomp.cpp 30 1 squish
The second seem to have to do with c-old-style ????
int ZEXPORT gzgetc(file)
gzFile file;
{
It doesnt like the parameters, like's to have them in 'modern' form, like this:
int ZEXPORT gzgetc( gzFile file)
{
So, is there some simple setting i'm missing or has it maybe to do with the 64bit thing?
Help would be nice, even if its only me missing something i can take that :-)
Best regards
Kaj
06/18/2012 (2:29 am)
EDIT 3: SOLVED, see my post below.EDIT 2:
OOPs, nop not yet, but i'm on to it:-
EDIT 1: Prefect Alfio, exactly as you said!
As Alfio say in the note below: generateProjects.bat and a fresh (recursive) download fixed the problem.
EDIT 1: EDIT 2:
EDIT3:
Hello and great effort.
One simple question (probably something i miss:-).
If i have a default T3D 1.2 installation (which compiles ok) and drop all the files from changelog 64 then i can't compile anymore...
I'm running on a 64b Win7 E box, with TFS2010 and VS 2010 U.
This is the first error messsage. ( I can fix some of them but the amount is to big..)
Error 11 error C2039: 'CompressImageOMP' : is not a member of 'squish' d:torquecommunitytorque 3d 1.2enginelibsquishsquishomp.cpp 30 1 squish
The second seem to have to do with c-old-style ????
int ZEXPORT gzgetc(file)
gzFile file;
{
It doesnt like the parameters, like's to have them in 'modern' form, like this:
int ZEXPORT gzgetc( gzFile file)
{
So, is there some simple setting i'm missing or has it maybe to do with the 64bit thing?
Help would be nice, even if its only me missing something i can take that :-)
Best regards
Kaj
#172
Before making any changes, try to re-generate the Visual Studio solution, with the proper generateProjects.bat file.
We are light years away from 64bit. I suggest you not to think about it just for now. :D
06/18/2012 (4:25 am)
Is impossible. The Rev. 64 does not introduce changes to the engine. The errors you're having, seem caused by an incomplete update of some external libraries. Check that you have checked out the Rev. 55, because i have the feeling that you skipped a few steps.Before making any changes, try to re-generate the Visual Studio solution, with the proper generateProjects.bat file.
We are light years away from 64bit. I suggest you not to think about it just for now. :D
#173
The way i have set it up (adding CE over default T3D) the file gzio.c was still included in the zlib-project. (Not a problem in the CE direct version because the file is not there :-) Removed the file and it compiled!
EDIT 4:
Hmm, still getting the errors, i have to dig into this, probably im missing something simple. :-)
EDIT 3: Tested with a default install (T3D 1.2) on both a a 32b and a 64b box to make sure, with VS 2010 U. Default builds on both machines.
Then i make a svn folder, downloads all upto rev 64. Then a simply copy everything ontop of the original default folder to make sure that all is in place.
Then we run the generateProject bat's. Then trying the build, with the result as described.
Hopefully we are just missing something, but at the moment we need to think.
EDIT 2 :
OOPs, nop not yet, but i'm on to it:-
EDIT 1 : Prefect Alfio, exactly as you said!
As Alfio say in the note before: generateProjects.bat and a fresh (recursive) download fixed the problem.
EDIT 1: EDIT 2: EDIT 3: EDIT 4:
EDIT 5:
Hello, and thanx for the fast response!
I just did a full recursive checkout, let's see if that fix it.
I'm not compiling for 64b, it's just my box that is 64bit, just started to wonder if that could have coused the problem :-)
I will try the generateProjects, forgot about that one.
Thanx again!
Kaj
06/18/2012 (4:42 am)
EDIT 5: SOLVEDThe way i have set it up (adding CE over default T3D) the file gzio.c was still included in the zlib-project. (Not a problem in the CE direct version because the file is not there :-) Removed the file and it compiled!
EDIT 4:
Hmm, still getting the errors, i have to dig into this, probably im missing something simple. :-)
EDIT 3: Tested with a default install (T3D 1.2) on both a a 32b and a 64b box to make sure, with VS 2010 U. Default builds on both machines.
Then i make a svn folder, downloads all upto rev 64. Then a simply copy everything ontop of the original default folder to make sure that all is in place.
Then we run the generateProject bat's. Then trying the build, with the result as described.
Hopefully we are just missing something, but at the moment we need to think.
EDIT 2 :
OOPs, nop not yet, but i'm on to it:-
EDIT 1 : Prefect Alfio, exactly as you said!
As Alfio say in the note before: generateProjects.bat and a fresh (recursive) download fixed the problem.
EDIT 1: EDIT 2: EDIT 3: EDIT 4:
EDIT 5:
Hello, and thanx for the fast response!
I just did a full recursive checkout, let's see if that fix it.
I'm not compiling for 64b, it's just my box that is 64bit, just started to wonder if that could have coused the problem :-)
I will try the generateProjects, forgot about that one.
Thanx again!
Kaj
#176
jordan(dot)parsons(at)gpinteractive(dot)com
Mahalo Alfio for all the great work as well!
06/26/2012 (9:57 pm)
I'm stoked on this community collaboration effort! I'd like to help where I can, so if you could please add me:jordan(dot)parsons(at)gpinteractive(dot)com
Mahalo Alfio for all the great work as well!
#178
06/29/2012 (5:43 pm)
I'm here. Sorry for everyone, but i am on vacation. Monday i am back in office ;)
#179
07/03/2012 (6:00 pm)
Please add thomascorkran at gmail.com ?
#180
sorry to bother you on this, could you please create my account so I can start jumping into this?
Edited
Thanks!
07/12/2012 (3:25 am)
@Alfio,sorry to bother you on this, could you please create my account so I can start jumping into this?
Edited
Thanks!
Torque Owner Alfio Saitta
Collateral Studios
Object in the scene ->
-> Fill the IndexBuffer with the offset and the len of Each object ->
-> Fill the VertexBuffer ->
-> Send all to the DirectX layer.
Increase the index buffer would be able to send at one time many more objects to the video card. So it is true, it would increase the bandwidth. Have to consider that the Torque3D has many other limitations to be removed to get the benefits from a 32bit IndexBuffer. I also believe that should be redesigned much of the physics engine.