2008/11/19 Robert Osfield <[EMAIL PROTECTED]>:
> 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.
>
Hi,
I see alternative solution here:
Deprecate valid() for all smart pointers, and implicit bool conversion
for ref_ptr, also making similar change observer_ptr. (Patch with
first change was submitted some time ago).
Then code like that would look like:
if (_current_context && _current_context->valid ()) {...}
It will also make code more generic, ie. using plain c-pointer, then
switching to smart pointer, or using tr1::shared_ptr, unique_ptr
etc..,
which was my main motivation when I submitted this change.
Although that's probably change that will take longer while to use to it.
Just my 2 cents, Piotr
> 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-submissions mailing list
>> [email protected]
>> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
>>
>>
> _______________________________________________
> osg-submissions mailing list
> [email protected]
> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
>
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
