Hi Jannik, On 28 May 2015 at 15:09, Jannik Heller <[email protected]> wrote: > There is still a bug though on this line: > _textureObjectBuffer[contextID] = textureObject = > generateTextureObject(this, contextID, GL_TEXTURE_1D); > > The generated texture object is first assigned to textureObject (a > TextureObject* raw pointer), and only then the result of that assignment to > _textureObjectBuffer. That means, there is a point where no ref is held, and > same crash as with VertexBufferObject could occur (although astronomically > unlikely).
This makes sense, a very subtle bug but a bug no less. > To fix this problem, the order of assignments might be reversed: > textureObject = _textureObjectBuffer[contextID] = > generateTextureObject(this, contextID, GL_TEXTURE_1D); This wouldn't fix the bug unless we include the change of generateTexturteObject to return a ref_ptr<>. We'd also need a .get() in case the OSG is built without automatic pointer conversion. > However, I felt that the for the order of assignments to have a semantic > meaning was not right. Therefore I went for the safer alternative of having > textureObject become a ref_ptr. > > I see your point though about overheads within apply(), since it's called on > every frame. What do you think? Keep on doing a ref/unref there isn't strictly required is something we need to avoid, if there is a fix that doesn't create this overhead then we should pursue it. The re-ordering suggestion achieves the aim of fixing the bug and avoiding the overhead, but it's so subtle I'd rather look for a more explict mechanism. First step is changing generateTextureObject(..) so it returns a ref_ptr<TextureObject>, then the next step would to result the textureObject = _textureObjectBuffer[contextID] = generateTextureObject(this, contextID, ..) My inclination would be the create a new method generateAndAssignTextureObject(...) that calls the TextureObjectManage::generateTextureObject(..) to create the TextureObject, assigns it to the local _textureObjectBuffer[contextID] and then returns a TextureObject* to this assigned TextureObject. The Texture*::apply() methods would then be ported across to the new method. Thoughts? Robert. _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
