Patch for GCC 4.2

Chewi

11-04-2008 14:53:47

Hi. I've created a Gentoo ebuild for OgreODE CVS, which I will submit to Gentoo shortly. However, I hit some compiler errors and I want to try and get this patch committed upstream first. They occur when using GCC 4.2. With 3.4, they were merely reported as warnings. Judging by the threads I've seen, this didn't affect 4.1. Here are the first errors.

OgreOdeBody.cpp: In member function 'OgreOde::Joint* OgreOde::Body::getJoint(int)':
OgreOdeBody.cpp:523: error: cast from 'dxJoint*' to 'unsigned int' loses precision
OgreOdeBody.cpp: In destructor 'virtual OgreOde::Body::~Body()':
OgreOdeBody.cpp:720: error: cast from 'dxBody*' to 'unsigned int' loses precision


I was able to fix that and the subsequent errors with this patch.

diff -Naur src.old/OgreOdeBody.cpp src/OgreOdeBody.cpp
--- src.old/OgreOdeBody.cpp 2008-04-10 15:36:12.287711000 +0100
+++ src/OgreOdeBody.cpp 2008-04-10 15:37:36.761626690 +0100
@@ -520,7 +520,7 @@
//-----------------------------------------------------------------------
Joint* Body::getJoint(int index)
{
- return (Joint*)_world->getJointList().findItem((unsigned int)dBodyGetJoint(_body,index));
+ return (Joint*)_world->getJointList().findItem((unsigned long)dBodyGetJoint(_body,index));
}
//-----------------------------------------------------------------------
size_t Body::getGeometryCount()
@@ -717,7 +717,7 @@
destroyDebugNode();
delete _mass;

- _world->getBodyList().unregisterItem((unsigned int)_body);
+ _world->getBodyList().unregisterItem((unsigned long)_body);
dBodyDestroy(_body);
}

diff -Naur src.old/OgreOdeGeometry.cpp src/OgreOdeGeometry.cpp
--- src.old/OgreOdeGeometry.cpp 2008-04-10 15:36:12.298626000 +0100
+++ src/OgreOdeGeometry.cpp 2008-04-10 15:37:36.762876728 +0100
@@ -211,7 +211,7 @@
{
_debug_contacts = new DebugContact*[_max_contacts];
for (unsigned int i = 0; i < _max_contacts; i++)
- _debug_contacts[i] = new DebugContact(Ogre::StringConverter::toString((int)_geom) +
+ _debug_contacts[i] = new DebugContact(Ogre::StringConverter::toString((long)_geom) +
"_Contact_" +
Ogre::StringConverter::toString(i),
_world);
@@ -273,7 +273,7 @@
//------------------------------------------------------------------------------------------------
Space* Geometry::getSpace()
{
- return (Space*)_world->getSpaceList().findItem((unsigned int)dGeomGetSpace(_geom));
+ return (Space*)_world->getSpaceList().findItem((unsigned long)dGeomGetSpace(_geom));
}


@@ -425,7 +425,7 @@
}
_debug_contacts = new DebugContact*[max_contacts];
for (unsigned int i = 0; i < max_contacts; i++)
- _debug_contacts[i] = new DebugContact(Ogre::StringConverter::toString((int)_geom) + + "_Contact_" + Ogre::StringConverter::toString(i),
+ _debug_contacts[i] = new DebugContact(Ogre::StringConverter::toString((long)_geom) + + "_Contact_" + Ogre::StringConverter::toString(i),
_world);
}
_max_contacts = max_contacts;
diff -Naur src.old/OgreOdeJoint.cpp src/OgreOdeJoint.cpp
--- src.old/OgreOdeJoint.cpp 2008-04-10 15:36:12.303420000 +0100
+++ src/OgreOdeJoint.cpp 2008-04-10 15:37:36.762876728 +0100
@@ -35,7 +35,7 @@

JointGroup::~JointGroup()
{
- _world->getJointGroupList().unregisterItem((unsigned int)_joint_group);
+ _world->getJointGroupList().unregisterItem((unsigned long)_joint_group);
dJointGroupDestroy(_joint_group);
}

diff -Naur src.old/OgreOdeSpace.cpp src/OgreOdeSpace.cpp
--- src.old/OgreOdeSpace.cpp 2008-04-10 15:36:12.312932000 +0100
+++ src/OgreOdeSpace.cpp 2008-04-10 15:37:36.763876673 +0100
@@ -62,7 +62,7 @@
//------------------------------------------------------------------------------------------------
Geometry* Space::getGeometry(int index)
{
- return (Geometry*) _world->getGeometryList().findItem((unsigned int)dSpaceGetGeom(_space,index));
+ return (Geometry*) _world->getGeometryList().findItem((unsigned long)dSpaceGetGeom(_space,index));
}
//------------------------------------------------------------------------------------------------
void Space::registerSpace()


To be honest, I'm not sure why these were casted to unsigned int before when an unsigned long is expected. The type being cast is a pointer and this would be 32-bit (int sized) on a 32-bit x86 machine so maybe that was the thinking behind it.

rewb0rn

11-04-2008 16:35:13

i wrote a pm to tuan

tuan kuranes

11-04-2008 18:29:09

Is this a x64 or x32 Gentoo ?

Does using size_t instead works better ?

The patch would work on x32 and x64 flawlessly then.

Chewi

11-04-2008 18:44:52

It's 64. I did think it would be some platform specific issue at first but if you look at include/OgreOdeMaintainedList.h, findItem takes an unsigned long regardless so shouldn't you just cast to that in either case? The problem has arisen because GCC has become stricter, not because I'm on a 64-bit system.

Chewi

11-04-2008 20:20:13

Sorry, I realise I was confused before. It's not the implicit cast from int to long (when calling findItem) but the cast from pointer to int that's causing the problem and so, yes, the problem is specific to 64-bit but GCC previously only treated it as a warning. It's just as well they made it stricter!

Unless you change the map in the MaintainedList class to use size_t instead of unsigned long, I still think you should just cast to unsigned long regardless because if you don't, it will just get cast to that implicitly anyway.

Hope I'm making sense now.

tuan kuranes

16-04-2008 17:45:50

Please have a try with latest CVS.

Chewi

16-04-2008 18:02:58

Still the same. To be honest, I can't see how any of the changes in that commit would have fixed this.

tuan kuranes

16-04-2008 18:12:19

does the cvs fix you get had 'MaintainedList class now use size_t' thing ?

Chewi

16-04-2008 18:23:53

Nope, OgreOdeMaintainedList.h hasn't been changed at all. See for yourself.

tuan kuranes

16-04-2008 18:27:05

Sorry forgot the lag bewteen developper CVS and public CVS.
You'll have to wait a bit.

Used to be a day before, perhaps shorter now... 5 hours was reported

Chewi

17-04-2008 10:57:51

I didn't realise there was a lag but that makes sense given the size of SourceForge. However, there is still no change to that file despite there being changes to some other files with the message "x64 patch (waiting for Gcc4.2 user test)" so are you sure you didn't just forget to commit that file or something? :wink:

tuan kuranes

17-04-2008 13:45:38

sorry, I mixed folders when merging multiples repositories...

Now, it's in CVS...

Chewi

17-04-2008 14:02:13

Still no good, I'm afraid.

OgreOdeGeometry.cpp: In member function 'virtual void OgreOde::Geometry::setDebugContact(bool)':
OgreOdeGeometry.cpp:219: error: cast from 'dxGeom*' to 'int' loses precision
OgreOdeGeometry.cpp: In member function 'OgreOde::Space* OgreOde::Geometry::getSpace()':
OgreOdeGeometry.cpp:281: error: cast from 'dxSpace*' to 'unsigned int' loses precision
OgreOdeGeometry.cpp: In member function 'void OgreOde::Geometry::setMaxContacts(unsigned int)':
OgreOdeGeometry.cpp:433: error: cast from 'dxGeom*' to 'int' loses precision
OgreOdeGeometry.cpp: In constructor 'OgreOde::TriangleMeshGeometry::TriangleMeshGeometry(const Ogre::Vector3*, unsigned int, const unsigned int*, unsigned int, OgreOde::World*, OgreOde::Space*)':
OgreOdeGeometry.cpp:901: error: ISO C++ forbids declaration of 'type name' with no type
OgreOdeGeometry.cpp:901: error: ISO C++ forbids declaration of 'type name' with no type
OgreOdeGeometry.cpp:901: error: expected primary-expression before 'const'
OgreOdeGeometry.cpp:901: error: expected `)' before 'const'


I think some of the changes in my patch were not related to MaintainedList so I guess you need some more size_t's there. The error on line 901 seems to be related to your previous commit.

tuan kuranes

17-04-2008 14:32:33

committed on CVS.

I'm afraid 901 is Ode related.

typedef unsigned int dTriIndex;

What version of ODE do you have ?

Chewi

17-04-2008 14:59:10

I'm using the latest source release, which is 0.9, because that's what Gentoo has. It would build if it weren't for this change so I guess I could reverse the change in the ebuild.

However, I think you should also change the declaration and definition of the TriangleMeshGeometry constructor to match since they still take "unsigned int" and I can see from ODE SVN that the size of dTriIndex can vary.

tuan kuranes

17-04-2008 15:13:52

I made an ogreode typedef then.
Committed... please test.

Chewi

17-04-2008 15:54:58

That doesn't work because other functions in ODE previously expected "int" rather than "unsigned int". If I were you, I'd scrap the TriangleIndex typedef and just define dTriIndex as "int" when necessary. This is starting to get a bit tedious and confusing so I've come up with the entire patch myself. I can successfully build after applying this.

Index: src/OgreOdeEntityInformer.cpp
===================================================================
RCS file: /cvsroot/ogre/ogreaddons/ogreode/src/OgreOdeEntityInformer.cpp,v
retrieving revision 1.12
diff -u -r1.12 OgreOdeEntityInformer.cpp
--- src/OgreOdeEntityInformer.cpp 17 Apr 2008 12:53:18 -0000 1.12
+++ src/OgreOdeEntityInformer.cpp 17 Apr 2008 14:53:58 -0000
@@ -114,7 +114,7 @@
const unsigned int prev_size = _index_count;
_index_count += (unsigned int)data->indexCount;

- unsigned int* tmp_ind = new unsigned int[_index_count];
+ dTriIndex* tmp_ind = new dTriIndex[_index_count];
if (_indices)
{
memcpy (tmp_ind, _indices, sizeof(unsigned int) * prev_size);
@@ -338,7 +338,7 @@
return _vertex_count;
}
//------------------------------------------------------------------------------------------------
-const unsigned int* EntityInformer::getIndices()
+const dTriIndex* EntityInformer::getIndices()
{
return _indices;
}
Index: src/OgreOdeGeometry.cpp
===================================================================
RCS file: /cvsroot/ogre/ogreaddons/ogreode/src/OgreOdeGeometry.cpp,v
retrieving revision 1.21
diff -u -r1.21 OgreOdeGeometry.cpp
--- src/OgreOdeGeometry.cpp 17 Apr 2008 14:14:57 -0000 1.21
+++ src/OgreOdeGeometry.cpp 17 Apr 2008 14:53:59 -0000
@@ -877,7 +877,7 @@
//------------------------------------------------------------------------------------------------
TriangleMeshGeometry::TriangleMeshGeometry(const Ogre::Vector3* vertices,
unsigned int vertex_count,
- const unsigned int* indices,
+ const dTriIndex* indices,
unsigned int index_count,
World *world, Space* space) :
Geometry(world, space),
@@ -900,7 +900,7 @@

_data = dGeomTriMeshDataCreate();

- dGeomTriMeshDataBuildSimple(_data,(const dReal*)_vertices, (int)vertex_count, (const TriangleIndex *)_indices, (int) index_count);
+ dGeomTriMeshDataBuildSimple(_data,(const dReal*)_vertices, (int)vertex_count, (const dTriIndex *)_indices, (int) index_count);

_geom = dCreateTriMesh(getSpaceID(space),_data,0,0,0);
registerGeometry();
Index: src/OgreOdeWorld.cpp
===================================================================
RCS file: /cvsroot/ogre/ogreaddons/ogreode/src/OgreOdeWorld.cpp,v
retrieving revision 1.11
diff -u -r1.11 OgreOdeWorld.cpp
--- src/OgreOdeWorld.cpp 17 Apr 2008 12:45:23 -0000 1.11
+++ src/OgreOdeWorld.cpp 17 Apr 2008 14:54:00 -0000
@@ -141,7 +141,7 @@
return (Real)dWorldGetAutoDisableTime(_world);
}
//------------------------------------------------------------------------------------------------
-void World::setAutoSleepAverageSamplesCount(unsigned int time)
+void World::setAutoSleepAverageSamplesCount(size_t time)
{
dWorldSetAutoDisableAverageSamplesCount(_world, time);
}
Index: include/OgreOdeEntityInformer.h
===================================================================
RCS file: /cvsroot/ogre/ogreaddons/ogreode/include/OgreOdeEntityInformer.h,v
retrieving revision 1.7
diff -u -r1.7 OgreOdeEntityInformer.h
--- include/OgreOdeEntityInformer.h 16 Apr 2008 16:45:39 -0000 1.7
+++ include/OgreOdeEntityInformer.h 17 Apr 2008 14:54:01 -0000
@@ -32,7 +32,7 @@

const Ogre::Vector3* getVertices();
unsigned int getVertexCount();
- const unsigned int* getIndices();
+ const dTriIndex* getIndices();
unsigned int getIndexCount();

protected:
@@ -51,7 +51,7 @@
Ogre::Vector3 _center;

Ogre::Vector3* _vertices;
- unsigned int* _indices;
+ dTriIndex* _indices;
unsigned int _vertex_count;
unsigned int _index_count;

Index: include/OgreOdeGeometry.h
===================================================================
RCS file: /cvsroot/ogre/ogreaddons/ogreode/include/OgreOdeGeometry.h,v
retrieving revision 1.14
diff -u -r1.14 OgreOdeGeometry.h
--- include/OgreOdeGeometry.h 17 Apr 2008 14:14:57 -0000 1.14
+++ include/OgreOdeGeometry.h 17 Apr 2008 14:54:01 -0000
@@ -340,7 +340,7 @@
public:
TriangleMeshGeometry(const Ogre::Vector3* vertices,
unsigned int vertex_count,
- const TriangleIndex* indices,
+ const dTriIndex* indices,
unsigned int index_count,
World *world,
Space* space = 0);
Index: include/OgreOdePreReqs.h
===================================================================
RCS file: /cvsroot/ogre/ogreaddons/ogreode/include/OgreOdePreReqs.h,v
retrieving revision 1.14
diff -u -r1.14 OgreOdePreReqs.h
--- include/OgreOdePreReqs.h 17 Apr 2008 14:14:57 -0000 1.14
+++ include/OgreOdePreReqs.h 17 Apr 2008 14:54:01 -0000
@@ -20,17 +20,8 @@
#endif

#ifndef dTriIndex
-#if dTRIMESH_16BIT_INDICES
-#if dTRIMESH_GIMPACT
- typedef uint32 dTriIndex;
-#else // dTRIMESH_GIMPACT
- typedef uint16 dTriIndex;
-#endif // dTRIMESH_GIMPACT
-#else // dTRIMESH_16BIT_INDICES
- typedef unsigned int dTriIndex;
-#endif // dTRIMESH_16BIT_INDICES
+typedef int dTriIndex;
#endif
-typedef dTriIndex TriangleIndex;


class World;

tuan kuranes

17-04-2008 16:39:55

cannot be int. 32bits indices buffer needs unsigned int.

Recommitted a version...

Chewi

17-04-2008 17:24:52

I'm not sure if that was meant to fix everything but I'm still hitting the same problem as before.

OgreOdeGeometry.cpp: In constructor 'OgreOde::TriangleMeshGeometry::TriangleMeshGeometry(const Ogre::Vector3*, unsigned int, const unsigned int*, unsigned int, OgreOde::World*, OgreOde::Space*)':
OgreOdeGeometry.cpp:903: error: invalid conversion from 'const OgreOde::TriangleIndex*' to 'const int*'
OgreOdeGeometry.cpp:903: error: initializing argument 4 of 'void dGeomTriMeshDataBuildSimple(dxTriMeshData*, const dReal*, int, const int*, int)'


Are you sure they need to be "unsigned int" because in the same commit that dTriIndex was introduced, the dGeomTriMeshDataBuildSimple function was changed from "int" to dTriIndex. Admittedly, I don't know anything about the ODE internals but maybe "int" was okay for the older version.

tuan kuranes

17-04-2008 17:30:35

int was an error, as some triangle soup can have more than 65535, but they may sorted that out with cast...

if you it works for you with
typedef int dTriIndex;

Then I'll add that o CVS.

Chewi

17-04-2008 17:47:35

You missed a couple of changes from my patch, specifically line 117 of OgreOdeEntityInformer.cpp (after new) and line 880 of OgreOdeGeometry.cpp. With these fixed and dTriIndex set to int, it works.

Having it as int may be a problem but if it is then it's ODE's bug rather than yours and it's already been fixed so you shouldn't worry need to worry about it.

tuan kuranes

17-04-2008 17:54:36

Ok. It's in CVS now. Thanks !

Chewi

17-04-2008 18:23:54

I would have done away with the other typedefs and just defined it as int in all cases to keep it consistent but I'm afraid there's a problem with the whole thing anyway. #ifndef doesn't work with typedefs. Looks like you'll have to find another way round it! But seriously, if it's easier for me to just patch it in the ebuild, I'll do that. I know it would be nice to support the latest release as well as CVS but don't go out of your way. I must say, your patience is impressive. :lol: