Re-posted from osg-submissions as requested....


On Wed, Nov 19, 2008 at 10:56 PM, Centers, Kyle
<[EMAIL PROTECTED]> wrote:
> Robert,
>
> I did say I might be doing something wrong :) But if the goal is to have the
> buffers initialize to the correct size before threading, why have the
> resize() call there at all? Also, what about the case where someone creates
> a new context after threading has begun? I don't see anything that prevents
> it, and in fact, it looks like CompositeViewer specifically is designed to
> support it.
>
> While I'm on the subject, why do ViewerBase::setUpThreading(), and
> ViewerBase::setThreadingModel() both call ViewerBase::startThreading()?
> Seems to me, if you're not currently threading, just because you set the
> ThreadingModel, that doesn't necessarily mean you want to start threading
> right away (although if you were already threading, you certianly want to
> stop threading before setting the Threading Model, and should probably
> restart threading after you have done so - that or refuse to set the
> Threading Model all together until threading stops). Seems like a more
> orthognonal approach would be more sensible, even if less convenient.
>
> Kyle
>
> ________________________________
> From: [EMAIL PROTECTED] on behalf of Robert
> Osfield
> Sent: Wed 11/19/2008 2:27 PM
> To: OpenSceneGraph Submissions
> Subject: Re: [osg-submissions] segfault in buffered_value and
> viewerbaserevisited
>
> Hi Kyle,
>
> There deliberately isn't a mutex as it would be a performance killer.
> Reviewing your change the resize code isn't thread safe anyway - as
> you have to protect all access to the internal vector as accessing it
> at the same time as resizing will cause problems.
>
> The design of osgViewer/OSG's OpenGL object
> management/osg::buffered_value is that the viewer initializes the
> buffers to the maximum number of contexts require prior to
> multi-threading.  osgViewer normally manages this automatically for
> you so you must have some corner case usage model that the buffer
> isn't initialized to the correct size.
>
> As for the catch on the dodgy earlier submission, I should have
> spotted this, and you are right in that the naming fooled both of us.
> Frustratingly when I wrote the original and correct code I would have
> been aware exactly which valid referenced to what object, only on
> review of your submission did I miss this. I'll have a think about
> naming of the context valid method.  Changing its name would break
> compatibility though...
>
> Robert.
>
> On Wed, Nov 19, 2008 at 5:27 PM, Centers, Kyle
> <[EMAIL PROTECTED]> wrote:
>>
>>
>> A few weeks ago, I submitted a change proposal to
>> include/osgViewer/ViewerBase, wherein I recommended removing a redundant
>> call to _currentContext.valid(), in releaseContext(). That was wrong. The
>> line in question (line 254) reads:
>>
>>     if (_currentContext.valid() && _currentContext->valid())
>>
>> note that the two calls to valid() are prefixed with the "." operator
>> first,
>> and then the "->" operator... turns out, _currentContext is actually an
>> observer_ptr<osg::GraphicsContext>, and not a osg::GraphicsContext...
>> oops.
>> In my defense, that is phenomenally unclear code. Perhaps renaming one of
>> those valid() calls would make it more clear? I don't know, but that line,
>> at the very least, needs a comment explaining that it is, in fact,
>> correct.
>>
>> Next, I ran across a couple of segfaults while running with three separate
>> GC's and three cameras (one master, two slaves), all rendering from the
>> same
>> scene. I traced the problem to include/osg/buffered_value, in both
>> buffered_value<T>::operator[], and in buffered_object<T>::operator[]. Both
>> dynamically resize _array any time pos exceeds _array.size(). In a
>> threaded
>> environment, if one thread tries to access _array while another thread is
>> resizing it, the result is a segfault.
>>
>> I don't know if I'm doing something wrong or not, but I added an
>> OpenThreads::Mutex to both classes and that seems to solve the problem for
>> me (updated buffered_value file attached).
>>
>> Kyle
>>

_______________________________________________
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Reply via email to