Patches for new SharedPtr in Ghadamon

Discussion area about developing or extending OGRE, adding plugins for it or building applications on it. No newbie questions please, use the Help forum for that.
Transporter
Minaton
Posts: 933
Joined: Mon Mar 05, 2012 11:37 am
Location: Germany
x 1

Patches for new SharedPtr in Ghadamon

Post by Transporter » Sat Jul 13, 2013 1:23 am

As SharedPtr now uses atomics, you now have to explicitly cast to the target data type, for example:

Code: Select all

MaterialPtr matPtr = MaterialManager::getSingleton().create("MyMaterial", "General").staticCast<Material>();
http://www.ogre3d.org/tikiwiki/tiki-ind ... ting_Notes

It's easy to patch files:

Code: Select all

#if (OGRE_VERSION < ((1 << 16) | (9 << 8) | 0))
    Ogre::MaterialPtr mat = Ogre::MaterialManager::getSingleton().create("Testmaterial", "Test");
#else
    Ogre::MaterialPtr mat = Ogre::MaterialManager::getSingleton().create("Testmaterial", "Test").staticCast<Ogre::Material>();
#endif
btogre
btogre.patch
(851 Bytes) Downloaded 138 times
ogreassimp
https://bitbucket.org/jacmoe/ogreassimp ... -sharedptr

MyGUI
MyGUI.patch
(3.45 KiB) Downloaded 152 times
SkyX
https://docs.google.com/file/d/0B_ao-Cm ... sp=sharing or Ogitor

Hydrax
https://docs.google.com/file/d/0B_ao-Cm ... sp=sharing or Ogitor

Ogre-Procedural
http://code.google.com/r/ogretransporte ... 4a533135f3

Ogitor
https://bitbucket.org/jacmoe/ogitor/pul ... e-ghadamon

PagedGeometry
Changed as part of Ogitor.

Caelum
Changed as part of Ogitor.

CEGUI
cegui.patch
(7.09 KiB) Downloaded 157 times
0 x

rndbit
Gnoblar
Posts: 20
Joined: Sat Mar 05, 2011 7:57 pm

Re: Patches for new SharedPtr in Ghadamon

Post by rndbit » Sun Jul 14, 2013 8:28 am

Err.. Why on earth such redundancy was introduced in the first place?
0 x

drwbns
Orc Shaman
Posts: 777
Joined: Mon Jan 18, 2010 6:06 pm
Location: Costa Mesa, California

Re: Patches for new SharedPtr in Ghadamon

Post by drwbns » Sun Jul 14, 2013 12:45 pm

New sharedPtr takes up much less memory per use.
0 x

rndbit
Gnoblar
Posts: 20
Joined: Sat Mar 05, 2011 7:57 pm

Re: Patches for new SharedPtr in Ghadamon

Post by rndbit » Mon Jul 15, 2013 9:14 am

Okay cool, but bear with me here..

Code: Select all

MaterialManager::getSingleton().create("MyMaterial", "General").staticCast<Material>();
What prevents create() returning same thing staticCast() returns? I am sure it is pretty obvious that if we call MaterialManager::create() then we are expecting to get Material, not Texture, not anything else. So what is the point of typing obvious thing when it could be avoided?
0 x

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by Klaim » Mon Jul 15, 2013 9:53 am

rndbit wrote:Okay cool, but bear with me here..

Code: Select all

MaterialManager::getSingleton().create("MyMaterial", "General").staticCast<Material>();
What prevents create() returning same thing staticCast() returns? I am sure it is pretty obvious that if we call MaterialManager::create() then we are expecting to get Material, not Texture, not anything else. So what is the point of typing obvious thing when it could be avoided?
I was surprised too but I just reminded that it's because of ResourceManager base class having a virtual create() function. See http://www.ogre3d.org/docs/api/html/cla ... nager.html

However in the provided context it is indeed not that useful. One possible improvements is to add a more specific function in each ResourceManager child which return the specific type, something like:

Code: Select all

MaterialManager::getSingleton().createMaterial("MyMaterial", "General")
I don't know if a refactoring would avoid this kind of problem, but in doubt as there is a GSoC going on about resource management system, I'll point the author to read this.
0 x

Transporter
Minaton
Posts: 933
Joined: Mon Mar 05, 2012 11:37 am
Location: Germany
x 1

Re: Patches for new SharedPtr in Ghadamon

Post by Transporter » Tue Jul 16, 2013 9:41 am

It should be possibe to integrate the static cast in create method. Something like this:

Code: Select all

MaterialManager::getSingleton().create<Material>("MyMaterial", "General");
Note: CEGUI will drop Ogre support
0 x

rndbit
Gnoblar
Posts: 20
Joined: Sat Mar 05, 2011 7:57 pm

Re: Patches for new SharedPtr in Ghadamon

Post by rndbit » Tue Jul 16, 2013 1:26 pm

Both solutions do not sound perfect to be honest. They are one bit too redundant, but definitely better than static cast.

What about making ResourceManager::create() non-virtual? As far as i can see there is no real requirement for polymorphism, we are using resource managers directly. So in the end we would keep create() in all classes and yet returning object of correct type without any manual casts or useless typing. Best part is that child resource managers will still be able to utilize ResourceManager::create() if needed so there would be no code duplication either.
0 x

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by Klaim » Tue Jul 16, 2013 2:00 pm

I would have removed the ResourceManager as a base class altogether or made it CRTP, personally, but I see in the Ogre code base that the abstract interface is used in a lot of cases, in particular in mechanisms systems like the background loader (which will be rewritten anyway).
0 x

rndbit
Gnoblar
Posts: 20
Joined: Sat Mar 05, 2011 7:57 pm

Re: Patches for new SharedPtr in Ghadamon

Post by rndbit » Tue Jul 16, 2013 2:31 pm

In that case ResourceManager::create() could be refactored to something like ResourceManager::createInternal() freeing up straightforward non-virtual create() for use by users. Cant wait to see the direction this goes ^_^
0 x

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by Klaim » Tue Jul 16, 2013 4:22 pm

You can already take a look at the plan and status in the GSoC thread: http://www.ogre3d.org/forums/viewtopic.php?f=13&t=77193
0 x

Owen
Google Summer of Code Student
Google Summer of Code Student
Posts: 91
Joined: Mon May 01, 2006 11:36 am

Re: Patches for new SharedPtr in Ghadamon

Post by Owen » Mon Jul 22, 2013 8:58 pm

As has been discussed, the current state is very much "interim". I drafted the patch for 2.0 and it got pulled into 1.9 because it is a significant memory (and sometimes performance) improvement.

In my working copy the way the resource system works is very different. There should be, by the end of it, convenience functions for most of these use cases.
0 x

User avatar
Mind Calamity
Ogre Magi
Posts: 1255
Joined: Sat Dec 25, 2010 2:55 pm
Location: Macedonia

Re: Patches for new SharedPtr in Ghadamon

Post by Mind Calamity » Wed Jul 24, 2013 7:59 am

Transporter - CEGUI will not be dropping OGRE support anytime soon - look at the last comment by Paul (aka Crazy Eddie).

The patch/pull request wasn't accepted because it broke compatibility with older versions of OGRE, and adding Ogre.h would just increase compile time unnecessarily.

I've made a new patch that doesn't break compatibility and doesn't include Ogre.h, you can find my pull request (not yet reviewed by the CEGUI team) here and my repository with support for OGRE 1.9 here.
0 x
BitBucket username changed to iboshkov (from MindCalamity)
Do you need help? What have you tried?
- xavier
---------------------
HkOgre - a Havok Integration for OGRE | Simple SSAO | My Blog | My YouTube | My DeviantArt

Transporter
Minaton
Posts: 933
Joined: Mon Mar 05, 2012 11:37 am
Location: Germany
x 1

Re: Patches for new SharedPtr in Ghadamon

Post by Transporter » Wed Jul 24, 2013 8:44 am

Mind Calamity wrote:Transporter - CEGUI will not be dropping OGRE support anytime soon - look at the last comment by Paul (aka Crazy Eddie).
I have noticed that Paul makes fun of me. :evil:
0 x

User avatar
Mind Calamity
Ogre Magi
Posts: 1255
Joined: Sat Dec 25, 2010 2:55 pm
Location: Macedonia

Re: Patches for new SharedPtr in Ghadamon

Post by Mind Calamity » Mon Aug 05, 2013 9:58 pm

Hey guys, since I'm using Gorilla, I updated it and remembered this thread, and thought it might save someone the trouble of adding all those version checks. (Easy, but annoying and boring as hell)
So, here's a patch. If you want, please add it to the original post for better visibility, transporter. :)
Gorilla.patch
Patch for betajaen's Gorilla.
(6.27 KiB) Downloaded 121 times
0 x
BitBucket username changed to iboshkov (from MindCalamity)
Do you need help? What have you tried?
- xavier
---------------------
HkOgre - a Havok Integration for OGRE | Simple SSAO | My Blog | My YouTube | My DeviantArt

Slicky
Bronze Sponsor
Bronze Sponsor
Posts: 537
Joined: Mon Apr 14, 2003 11:48 pm
Location: Was LA now France
x 12

Re: Patches for new SharedPtr in Ghadamon

Post by Slicky » Fri Aug 09, 2013 11:06 pm

When I am building my app I get the errors from sharedptr but no link to which line is creating the error. I end up searching code for ptr. Is there an easier way to find lines that need to be fixed?
0 x

drwbns
Orc Shaman
Posts: 777
Joined: Mon Jan 18, 2010 6:06 pm
Location: Costa Mesa, California

Re: Patches for new SharedPtr in Ghadamon

Post by drwbns » Sat Aug 10, 2013 5:11 am

Look at your output build log in visual studio, the first is the the sharedptr error, a few lines later it says "in file ... line blah blah"
0 x

scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 2

Re: Patches for new SharedPtr in Ghadamon

Post by scrawl » Thu Aug 29, 2013 3:26 am

Hi,
here is my attempt to fix this problem, please let me know if there are any issues with it.

Edit: it got merged, thanks masterfalcon!
0 x

User avatar
nevarim
Gnoll
Posts: 655
Joined: Mon Jul 05, 2010 6:16 pm
Location: Pavia Italy
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by nevarim » Thu Sep 19, 2013 11:47 pm

compiling skyx with ogre 1.9 i have this error

Code: Select all

d:\ogre\source\skyx\skyx\include\GPUManager.h(120): error C2039: 'staticCast': not a member of 'Ogre::SharedPtr<T>'
and this i

Code: Select all

#if (OGRE_VERSION < ((1 << 16) | (9 << 8) | 0))
			mSkydomeMaterial = static_cast<Ogre::MaterialPtr>(Ogre::MaterialManager::getSingleton().getByName(getSkydomeMaterialName()));
#else
			mSkydomeMaterial = (Ogre::MaterialManager::getSingleton().getByName(getSkydomeMaterialName())).staticCast<Ogre::Material>();
#endif
s code


anyone known how to resolve?

thanks
Neva
Last edited by nevarim on Fri Sep 20, 2013 1:23 am, edited 1 time in total.
0 x
i'm a noob until proven otherwise :D
used in my project ;) and thanks to everyone :D
Ogre 3d
Mygui
Skyx
Hydrax
MOC
CCS

User avatar
nevarim
Gnoll
Posts: 655
Joined: Mon Jul 05, 2010 6:16 pm
Location: Pavia Italy
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by nevarim » Thu Sep 19, 2013 11:52 pm

same for hydrax
0 x
i'm a noob until proven otherwise :D
used in my project ;) and thanks to everyone :D
Ogre 3d
Mygui
Skyx
Hydrax
MOC
CCS

User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by masterfalcon » Fri Sep 20, 2013 1:13 am

It should be noted that this is no longer a requirement, the previous behavior has been restored but with a more efficient backend behind it.
0 x

User avatar
nevarim
Gnoll
Posts: 655
Joined: Mon Jul 05, 2010 6:16 pm
Location: Pavia Italy
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by nevarim » Fri Sep 20, 2013 1:32 am

so i must remove

Code: Select all

.staticCast<Ogre::Material>();
?
0 x
i'm a noob until proven otherwise :D
used in my project ;) and thanks to everyone :D
Ogre 3d
Mygui
Skyx
Hydrax
MOC
CCS

User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by masterfalcon » Fri Sep 20, 2013 1:53 am

Yes, that's correct.
0 x

User avatar
nevarim
Gnoll
Posts: 655
Joined: Mon Jul 05, 2010 6:16 pm
Location: Pavia Italy
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by nevarim » Fri Sep 20, 2013 1:57 am

Transporter wrote:
PagedGeometry
Changed as part of Ogitor.
what's meaning that developing of this plugin is stopped and the only way to follow is implementing ogitor?
0 x
i'm a noob until proven otherwise :D
used in my project ;) and thanks to everyone :D
Ogre 3d
Mygui
Skyx
Hydrax
MOC
CCS

User avatar
nevarim
Gnoll
Posts: 655
Joined: Mon Jul 05, 2010 6:16 pm
Location: Pavia Italy
Contact:

Re: Patches for new SharedPtr in Ghadamon

Post by nevarim » Fri Sep 20, 2013 2:01 am

in cloudmanager i have another problem:

Code: Select all

NewCloudLayer->_registerCloudLayer(Ogre::MaterialManager::getSingleton().getByName(mSkyX->getGPUManager()->getSkydomeMaterialName())->getTechnique(0)->createPass());
Source\CloudsManager.cpp(230): error C2039: 'getTechnique': not a member of 'Ogre::Resource'
0 x
i'm a noob until proven otherwise :D
used in my project ;) and thanks to everyone :D
Ogre 3d
Mygui
Skyx
Hydrax
MOC
CCS

scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 2

Re: Patches for new SharedPtr in Ghadamon

Post by scrawl » Fri Sep 20, 2013 6:30 pm

This should definitely work. I think you are using an older revision of 1.9.
0 x

Post Reply