Ogre Vector3 and amd64

r=en->RegisterObjectBehaviour( "Xml",asBEHAVE_FACTORY,"Xml @f()",asFUNCTION(CXml::Factory),asCALL_CDECL);assert(r>=0);
r=en->RegisterObjectBehaviour( "Xml",asBEHAVE_ADDREF,"void f()",asMETHOD(CXml,AddRef),asCALL_THISCALL);assert(r>=0);
r=en->RegisterObjectBehaviour( "Xml",asBEHAVE_RELEASE,"void f()",asMETHOD(CXml,Release),asCALL_THISCALL);assert(r>=0);

asBEHAVE_FACTORY
asBEHAVE_ADDREF
asBEHAVE_RELEASE

I don't see any real difference between Ogre::vector3 and the vector3 add-on. You really should be able to use them interchangeably. You should in theory even be able to register the vector3 add-on with AngelScript, and still register the methods that use Ogre::vector3 without any wrapping (though this isn't recommended).

I do see a difference in how you registered the Ogre::vector3 type though. You have registered the opAssign() method, whereas with the vector3 add-on this is not done. Since the type is registered with asOBJ_POD this is not necessary. It really shouldn't be the problem, but it might be the cause of your troubles. I suggest you try not registering this method, and see if it works.

Please let me know if doing that works, and I'll investigate why it causes an error when registering the method.

Regards,
Andreas

Even if I take that line out, it fails to work.

The correct link to Ogre::Vector3 is:
https://bitbucket.org/sinbad/ogre/src/3e643321c782/OgreMain/include/OgreVector3.h
We've moved to Mercurial some time ago.

This is odd because it works really well on Windows (and IIRC *nix 32bit too).

If I use _CA instead of _CAK I get that error that it can't be passed by value (which is correct).

I can't really spot where the difference lies between Ogre::Vector3 and Angelscript::Vector3, but I am not a technical C++ guru (unfortunately).

Thanks for the correct link.

I don't see the difference between the implementations that is causing the problem either. Perhaps it is the fact that Ogre::Vector3 has everything inlined, maybe the C++ compiler does something differently in this case (though I don't think it should). Are you using GNUC?

I'll see if I can reproduce the problem somehow. Though I don't really have a 64bit Linux platform to debug on. I'll have to rely on the buildbot that Jeremy set up.

GCC 4.6.1 here.

I noticed that the operators are inlined as well. And explicit.

Maybe I should create a stand-alone Vector3 and take it for a spin with Angelscript.

I'd appreciate any help in isolating the cause for the problem.

I could have sworn I saw the copy constructor on the Ogre::Vector3 class. But as I started writing the test case for reproducing the problem I discovered that for some reason Ogre::Vector3 really doesn't have a copy constructor. This means that the real flag to use when registering the type is asOBJ_APP_CLASS_CA, and not asOBJ_APP_CLASS_CAK that you were using.

This is what is causing your problem on Linux/amd64. On Linux/amd64 with GNUC a class is treated very differently if the copy constructor is declared or not. A class with a copy constructor is always passed through a reference even when the function is declared to pass the type by value. When the copy constructor is not declared, the compiler will pass the type in the CPU registers instead, if the class is small enough. The real complicated part is that when the type is passed in the CPU registers, it is split up by the type of each member, so integer members are passed in the general purpose registers, and float members are passed in the floating point registers. This is why AngelScript doesn't allow these types to be passed by value, the implementation to support that in the native calling convention is just too difficult.

You're not seeing this problem on Windows or Linux 32bit because the existence of the copy constructor or not doesn't make a difference on these platforms.



To get things to work, without changing the Ogre::Vector3 class, it is necessary to use the asOBJ_APP_CLASS_CA flag and then also change all your functions that take the Vector3 type by value in any of the parameters to take the type by reference instead. For example 'setCameraPosition(Vector3 cp)' needs to be changed to 'setCameraPosition(const Vector3 &cp)'.

Awesome!
I'll definitely try that and report back.

I might even file a request for enhancement in the Ogre issue tracker if they'll accept a copy constructor.
That might save people from this issue on 64bit gcc empowered platforms.

I added this code to Ogre::Vector3:
inline Vector3(const Vector3& other)
: x(other.x), y(other.y), z(other.z)
{
}


And this issues went away - it works!

Now I am lobbying to get that into mainstream Ogre3d - wish me luck.

Alternatively, I could just cast between Ogre::Vector3 and my own version of it (with an explicit copy constructor), but I'd rather not.

Maybe this could be added to the documentation somewhere?
That a POD needs to have an explicit copy constructor on 64bit *nix?

They refused to add the explicit copy constructors to Ogre core, so I need to work around it.


Passing this flag to GCC will turn off the value to reference optimization trick I think:
-fno-ipa-sra

-fipa-sra
Perform interprocedural scalar replacement of aggregates, removal of unused parameters and replacement of parameters passed by reference by parameters passed by value.

Enabled at levels -O2, -O3 and -Os. [/quote]

Don't know if it works, though.

I think I'll just use some ugly casts between Angelic proxy classes and Ogre's POD classes instead.

Do I have to teach Angelscript how to handle that?

Did the Ogre team by any chance state the reason why they don't want to include the copy constructor? I'm curious as to whether they have a specific reason, or just that don't really see it as a problem in their code (which it isn't).

If you cannot use your own custom version of Ogre::Vector3, or change the methods that take the Vector3 type by value to take them by reference (note, it should still be possible to return the type by value), then the only other solution is to use function wrappers. For this, I believe you can use the auto wrappers, that you'll find in the add-ons.

[source]
// Example

#include "add_on/autowrapper/aswrappedcall.h"

// Implemente a method wrapper for the class method that takes the Ogre::Vector3 type by value in parameter
asDECLARE_METHOD_WRAPPER(AngelScriptTest_setCameraPosition_Wrapper, AngelScriptTest, setCameraPosition);

...

// Register the wrapper, instead of the real method
r = mEngine->RegisterObjectMethod("AngelscriptTest", "void setcp(vector3)", asFUNCTION(AngelscriptTest_setCameraPosition_Wrapper), asCALL_GENERIC); assert( r >= 0);
[/source]

This only needs to be done for the functions and class methods that take the Vector3 by value. Other functions and methods can be registered normally without any wrappers.

It was deemed not kosher by a team member:
http://www.ogre3d.org/forums/viewtopic.php?f=22&t=65876

A shame. ;)

I will try your suggestion.
Thanks a lot for your patience.

I read the thread over at Ogre. While I feel it is a pity they didn't want add the copy constructor, I can't really say I blame them. After all, they are not doing anything wrong by leaving it as an implicit copy constructor. And who knows, it may even cause a minor performance impact if the copy constructor is added, as it would prevent the GCC compiler from passing the type in the CPU registers.

If possible I'll add support for these kind of types in AngelScript. After all, it is quite common structure (i.e. all members are float).

It was deemed not kosher by a team member:
http://www.ogre3d.or...hp?f=22&t=65876

A shame. ;)

I will try your suggestion.
Thanks a lot for your patience.

Wow. Their acerbic response is a little bit distasteful for a fix that is so simple, but provides so much to some of their user-base. Not to mention, it wouldn't be the first time they disregarded standards-compliant behavior for a non-standard compiler implementation. Has anyone in that topic seen OgreSingleton.h?



Singleton( void )
{
assert( !ms_Singleton );
#if defined( _MSC_VER ) && _MSC_VER < 1200
int offset = (int)(T*)1 - (int)(Singleton <T>*)(T*)1;
ms_Singleton = (T*)((int)this + offset);
#else
ms_Singleton = static_cast< T* >( this );
#endif
}


In fact, it's a simple as implementing the following code in any offending classes:


//
// Provide an explict copy-constructor for GCC 4.6.1 on 64-bit Linux architectures
// (in-depth explanation here)
//
#if OGRE_PLATFORM == OGRE_PLATFORM_LINUX && \
OGRE_ARCH_TYPE == OGRE_ARCHITECTURE_64 && \
OGRE_COMPILTER == OGRE_COMPILER_GCC && \
OGRE_COMP_VER > 461 // (assume version greater than 4.6.1 exhibit the same behavior
inline Vector3(const Vector3& other)
: x(other.x), y(other.y), z(other.z)
{
}
#endif

There they go. A standards-compliant version of the class for standard-compliant compilers, and a non-standard-compliant version for non-standard versions. It's really quite simple. This small fix wouldn't decrease maintainability in the least, and the increase in line count is hardly noticeable.That entire thread put a bad taste in my mouth. But hey, I guess I'm not on the Ogre development team for a reason.

Thanks a lot for that post, Halifax2.
My thoughts exactly.

I read the thread over at Ogre. While I feel it is a pity they didn't want add the copy constructor, I can't really say I blame them. After all, they are not doing anything wrong by leaving it as an implicit copy constructor. And who knows, it may even cause a minor performance impact if the copy constructor is added, as it would prevent the GCC compiler from passing the type in the CPU registers.

*Them* is one team member. Don't expect a reply from anyone else.
And, yes: it is a pity.

I am aware of the possible performance impact, of course..

If possible I'll add support for these kind of types in AngelScript. After all, it is quite common structure (i.e. all members are float).

That would be lovely.

Using the autowrapper works!

I'll let you know when I'm done modifying the real code, but it does work a treat in my test project.

Thank you Andreas - you're the best.


asDECLARE_METHOD_WRAPPER(AngelscriptTest_getCameraPosition_Wrapper, AngelscriptTest, getCameraPosition);
and
// Register the wrapper, instead of the real method
r = mEngine->RegisterObjectMethod("AngelscriptTest", "vector3 getcp()",
asFUNCTION(AngelscriptTest_getCameraPosition_Wrapper),
asCALL_GENERIC); assert( r >= 0);

I've added an additional flag asOBJ_APP_ALLFLOATS that when used with the registration of the Ogre::Vector3 should allow you to use this type in native functions without wrappers. It tells AngelScript that the class consists only of floats, and will thus allow AngelScript to know that the type is returned in the XMM registers instead of the general purpose registers.