Hi Julian,

I have just reviewed the changes but get the sense that
TextureBuffer/BufferObject classes are being stretched in different
directions.

The approach you have taken assumes that BufferObject "owns" a list of
BufferData, whereas the way the OSG currently manages things is that
the BufferData (osg::Image, osg::Array) "own" their BufferObject, and
the BufferObject just hold a C pointer back to their parent which is
the BufferData.

The approach you are taking breaks this and will either result in
memory leaks and dangling pointers of force BufferObject to take a
ref_ptr<> to keep the BufferData around, which then will lead to a
circular reference and associated memory leak.

The replacement of ModifiedCount with a DirtyFlag also breaks the
naming convention and function.  The ModifiedCount is used as multiple
contexts can download GL data at different times, here if you store
the ModifiedCount at download for each context then you can then make
sure that all contexts GL objects will be properly updated, something
that can't be done with a DirtyFlag that gets set to true and then
back to false as soon as the first context downloads the data.

I think I now understand what functionality you are after i.e. to
enable TextureBuffer to be used directly with a BufferObject (and
hence associated GLBufferObjects) without the need for an osg::Image
to assign it to.  I'm not the author of TextureBuffer so can't comment
directly why the author decided to force the use of an osg::Image.

Looking at the git log for TextureBuffer the first commit is:

commit 1284a0dd40051738f7073afdd41667d960631f8b
Author: Robert Osfield <[email protected]>
Date:   Wed May 22 12:49:46 2013 +0000

    From Pawel Ksiezopolski, first email: "This submission adds
texture buffer object ( defined in GL_ARB_texture_buffer_object
extension ) to the osg::Texture* family.

    TextureBuffer objects may use osg::Texture::bindToImageUnit(), so
GLSL shaders are able to use not only texelFetch() function , but also
functions defined in GL_ARB_shader_image_load_store extension :
imageLoad(), imageStore(), imageAtomicAdd() etc."

    second email: "After a while I found that
osg::Texture::applyTexParameters() used with TextureBuffer may cause
some OpenGL errors ( applying texture filters and wraps to
TextureBuffer makes no sense ) so I fixed it."

The TextureBuffer also looks to be used in the osggpucull and
osgforest examples.

I haven't yet looked into these examples to see the exact usage of
TextureBuffer and how it might be affected.  I did wonder if
TextureBuffer could be modified to allow a BufferObject to be assigned
directly and the osg::Image dependency removed completely.  Or perhaps
just allow the osg::Image to be NULL if a BufferObject has been
directly assigned.  Another approach is to have a TextureBufferImage
as well as  TextureBuffer class.

Robert.


On 5 June 2016 at 22:35, Julien Valentin <[email protected]> wrote:
> Hi,
> Here's a version of TextureBuffer that seams clean to me..
> Its main interest is to bind any BufferObject to TextureBuffer:
> It allows (for example) to bind TransformFeedBack generated Buffer as TBO for 
> Instancing purpose.
> Waiting critisms and sugsestions
>
> Cheers,
> Julien
>
> ------------------
> Read this topic online here:
> http://forum.openscenegraph.org/viewtopic.php?p=67425#67425
>
>
>
>
> _______________________________________________
> 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

Reply via email to