Re: [Opensg-core] Merging changes back into trunk

[Carsten N. <car...@gm...>]

[adding the core list back to cc; sorry I had dropped it accidentally in 
my last reply. I'm not trimming quotes here, to allow everyone to catch up.]

Gerrit Voss wrote:
> On Tue, 2008-01-29 at 16:10 -0600, Carsten Neumann wrote:
>> Gerrit Voss wrote:
>>> On Thu, 2008-01-24 at 18:18 -0600, Carsten Neumann wrote:
>> [SNIP]
>>> ok for the merge, I would actually prefer to do it manually, especially
>>> to get the naming right, which I don't really like right now. In
>>> particular the GlobalRefPtr and the TransitPtr things (but I understand
>>> where they come from).
>>>
>>> I finally managed to get my git working. I'll try to export it so we can
>>> both working with it.
>>>
>>> Ok, so lets get it going :-)
>>>
>>> First questions (decisions) ;-)
>>>
>>> Names
>>>
>>>  - for apps I would tend to go for RefPtr (e.g. NodeRefPtr) without 
>>>    the Global thing.
>> The GlobalRefPtr are only needed for things that potentially live past 
>> osgExit. If they are used far more widespread (i.e. become the default 
>> application pointer) I guess some more thought should be put into their 
>> implementation. At the very least, I think, the internal list they 
>> maintain should be sorted to speed up searching.
>>
>>>  - Simular I would change the current Ptr to CPtr (e.g. NodeCPtr)
>>>    - this way we should find most occurrences.
>> Uhm, find most occurrences of what?
>>
>>> Concepts
>>>
>>>  - RefPtr should never auto convert from/to normal pointers 
>>>     - this is similar to boost
>>>     - this should remove the transit ptr.
>>>     - basically make the transitptr the refptr
>> this is tightly related to the argument/return type used for functions, 
>> see below.
> 
> this is one of my problems, a lot of comments, including the boost
> shared_ptr FAQ, say auto conversion on this kind of pointer class is
> not really a brilliant idea.
> 
>> >From the boost FAQ
> 
> Q. Why doesn't shared_ptr (or any of the other Boost smart pointers)
> supply an automatic conversion to T*?
> 
> A. Automatic conversion is believed to be too error prone.

it is sad that the boost folks have settled for such a weak answer to 
this question, not even giving pointers to any discussions :(
I would have very much like to see the arguments that have led to that 
decision (I just read up on this in A. Alexandrescu "Modern C++ Design", 
Sect. 7.7, he does not seem to be that unequivocal about the issue).

> Similar I ran into trouble with other auto conversions where the 
> compiler choose a different code path which resulted in wrong
> results at run time. No crashes just wrong numbers popping up once
> in a while. And these were painfully hard to find.
> 
> So I'm a little bit sceptical of auto converting anything especially as
> you get hit by differences in compiler implementations.

Hm, for pointers I find it hard to think of any cases where this 
actually can happen. Most uses are: calling a member function through 
the pointer, pass the pointer to a function.
The general guideline should be that anyone not using ref ptrs should 
have a very clear understanding of what they are doing, or things will 
break easily.

>>>  - Factories/Loader return ref ptr ?
>>>     - this should be fine
>> they return TransitPtr now, which made it pretty straightforward to find 
>> a lot of cases where Ptr had to be replaced by RefPtr.
>>
>>>  - Member functions take/return ref ptr ?
>>>     - this is the tricky bit. 
>>>     - Internally we might to like use the raw ptrs whereas externally
>>>       it should be ref ptrs.
>>>     - The cleanest option would be to duplicate what we need as
>>>       explicit raw pointer access and clearly mark it for internal
>>>       use.
>>>     - or force the parts that need raw pointers to move through
>>>       the field instead of the container interface.
>> the interfaces currently intentionally use raw pointers (and BTW manage 
>> to render without any refcount changes). The question is where this 
>> causes problems.
>> To make it usable, this requires that RefPtr is convertible to raw 
>> pointer (it is only _explicitly_ convertible from raw pointer though). I 
>> don't think much harm can be done by this conversion, as things like 
>> delete pFC; are unusual in an OpenSG program anyways.
>> Can you point out what problems you foresee with this ?
> 
> It's not so much problems, I'm still struggling to find consistency.
> 
> Currently some pieces have changed pointers, e.g the factories, the
> loaders, some don't.

More pieces could/should be changed to ref ptrs, but the transit ptr 
basically made sure that the first pointer a newly created object is 
stored to is a ref ptr and not a raw pointer.

> Than there are all these pointer variants which
> I expect to be of some use. So I still trying to wrap my head around
> it. Again against the background of not auto converting these pointers
> to raw pointers. 
> 
> If everything inside OpenSG stays as common c-ptrs why would we need the
> transit ptr. OR are they just there to enforce something ?

If everything on the inside stays as c ptrs, the factories can not 
return ref ptrs.

> At least they force some of the internal places to deal with RefPtrs
> which as soon as they are passed to a function disappear.

Think about it like this: Everything that wants to store a pointer to a 
container either has to put that pointer into a field or use a ref ptr.
Hm... How early may the d'tors for a function argument be called?  If 
the arguments are passed by value, can the d'tor call be executed after 
the copy construction, but before the actual function call?  That could 
be a problem, when passing a ref ptr to a function that takes a c ptr 
and the ref ptr has no further use after the function call.

> Can't we keep it simpler and consistent by not using any RefPtr inside
> OpenSG. 

As I said above, that means factories can not return ref ptr (or transit 
ptr), which indeed was meant to force everyone to use ref ptrs (but 
since it uses the same interfaces OpenSG is included in everyone ;) ).

> And only provide RefPtrs to be used in user/app code.
> 
> The only drawback I see is that we can not force the user/app to use
> RefPtrs.

yes, how much of a drawback that is in reality is probably a central 
point to judge.

	Cheers,
		Carsten