mRad private in Ogre::Radian

Minor issues with the Ogre API that can be trivial to fix
KarenRei
Gnoblar
Posts: 5
Joined: Wed Jan 05, 2011 12:58 am

mRad private in Ogre::Radian

Post by KarenRei »

In Ogre::Radian, mRad is private. This makes it a pain when extending the class. Please switch it to public or protected, like the member variables of other similar classes (Vector3, Quaternion, etc).
.
User avatar
so0os
Bugbear
Posts: 833
Joined: Thu Apr 15, 2010 7:42 am
Location: Poznan, Poland
x 33

Re: mRad private in Ogre::Radian

Post by so0os »

You should NEVER derive from Ogre::Radian
Sos Sosowski :)
http://www.sos.gd
User avatar
Jabberwocky
OGRE Moderator
OGRE Moderator
Posts: 2819
Joined: Mon Mar 05, 2007 11:17 pm
Location: Canada
x 218

Re: mRad private in Ogre::Radian

Post by Jabberwocky »

so0os wrote:You should NEVER derive from Ogre::Radian
Good advice, but it would be a more helpful post if you explained it a little. ;)
Image
User avatar
so0os
Bugbear
Posts: 833
Joined: Thu Apr 15, 2010 7:42 am
Location: Poznan, Poland
x 33

Re: mRad private in Ogre::Radian

Post by so0os »

Jabberwocky wrote:
so0os wrote:You should NEVER derive from Ogre::Radian
Good advice, but it would be a more helpful post if you explained it a little. ;)
OK.

A base class takes as many space in memory as its members, Radian has 1 float member, so it takes 4 bytes. When you derive, however, each class needs to store virtual function pointers, so you can call stuff on a base class and it gets executed for each derived child respectively. thus, size of the class grows with number of functions uhm.. methods there. It's all OK, until you do heavy math on radians (and tons of junk has to be moved in and out of stack every cycle) OR i.e. you put such struct into a VBO (that would be an overkill) or try to manually memcpy it somewhere.
Sos Sosowski :)
http://www.sos.gd
User avatar
syedhs
Silver Sponsor
Silver Sponsor
Posts: 2703
Joined: Mon Aug 29, 2005 3:24 pm
Location: Kuala Lumpur, Malaysia
x 51

Re: mRad private in Ogre::Radian

Post by syedhs »

I really think Radian class (along with Degree) should be treated like POD (Plain Old Data) and it is - so deriving from it make it complicated unnecessarily.
A willow deeply scarred, somebody's broken heart
And a washed-out dream
They follow the pattern of the wind, ya' see
Cause they got no place to be
That's why I'm starting with me
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5433
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1341

Re: mRad private in Ogre::Radian

Post by dark_sylinc »

I agree with syedhs, there should be no virtual members in the class, and the variable should change from "private" to "protected".
Not public to avoid accidental (or noobie) direct access to mRad while working with Radians.
User avatar
Kojack
OGRE Moderator
OGRE Moderator
Posts: 7157
Joined: Sun Jan 25, 2004 7:35 am
Location: Brisbane, Australia
x 535

Re: mRad private in Ogre::Radian

Post by Kojack »

When you derive, however, each class needs to store virtual function pointers, so you can call stuff on a base class and it gets executed for each derived child respectively. thus, size of the class grows with number of functions uhm.. methods there.
Inheritance on it's own doesn't add virtual function pointers. The vtable pointer is only added to an object if the class has methods which are declared as virtual.
Any number of virtual methods will cause the class to increase by only one vtable pointer, the size doesn't grow with the number of methods.
Since Radian has no virtuals, you can add as many methods to it in a derived class as you want (as long as they are non virtual too), it won't grow the object past 4 bytes.
User avatar
madmarx
OGRE Expert User
OGRE Expert User
Posts: 1671
Joined: Mon Jan 21, 2008 10:26 pm
x 50

Re: mRad private in Ogre::Radian

Post by madmarx »

A base class takes as many space in memory as its members, Radian has 1 float member, so it takes 4 bytes.
No. Not only does the C++ standard not force it to be the same size, but it actually happens rarely.
For that reason, Visual Studio provides some pragma to force data alignment.

class A
{
};

what is sizeof(A)?
=> 1 on VS2008

class B
{
bool b1;
bool b2;
bool b3;
double b4;
};

what is sizeof(B)?
=> 16 on VS2008

Such simple tests are very easy to set up...

Best,

Pierre

EDIT : and compiler options can make this size change too.
Tutorials + Ogre searchable API + more for Ogre1.7 : http://sourceforge.net/projects/so3dtools/
Corresponding thread : http://www.ogre3d.org/forums/viewtopic. ... 93&start=0
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5433
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1341

Re: mRad private in Ogre::Radian

Post by dark_sylinc »

@madmarx: I don't think data alignment could be highly relevant on the topic, and it is very compiler option specific, and architecture specific.. The main reason for not using derived classes are virtual functions, but since there are none, there is no overhead to worry about.
User avatar
madmarx
OGRE Expert User
OGRE Expert User
Posts: 1671
Joined: Mon Jan 21, 2008 10:26 pm
x 50

Re: mRad private in Ogre::Radian

Post by madmarx »

I don't think data alignment could be highly relevant on the topic
Yes, you are right, it is not really relevant. I just wanted to explain that a base class does not have to be the same size than its member.
Tutorials + Ogre searchable API + more for Ogre1.7 : http://sourceforge.net/projects/so3dtools/
Corresponding thread : http://www.ogre3d.org/forums/viewtopic. ... 93&start=0
User avatar
so0os
Bugbear
Posts: 833
Joined: Thu Apr 15, 2010 7:42 am
Location: Poznan, Poland
x 33

Re: mRad private in Ogre::Radian

Post by so0os »

I just didn't want to elaborate too much on that.
Sos Sosowski :)
http://www.sos.gd
CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
x 58

Re: mRad private in Ogre::Radian

Post by CABAListic »

You're getting slightly off-topic ;)
The simple answer is that Radian is not meant or designed to be extended, and as such there is no reason to make mRad non-private.