Re: [osg-users] Questions about Geometry implementation details

2007-10-19 Thread Christian Muschick
Robert Osfield wrote:
 Hi Ralf,
 
 Thanks for the clarification, I'm afraid my trip meant not following
 threads too closely.
 
 I've reviewed the code and the if (!array-getVertex..()) line is
 wrong, and Ulrich pointed to the correct implementation.
 
 I've fixed the relevant code and checked it into SVN.

Sorry to bother you again, but did you take a look at the other lines of 
code I pointed out? I think the same problem exists there.

line 1123:
if (!array-getVertexBufferObject()) vbo = array-getVertexBufferObject();

line 1170:
if (!array-getVertexBufferObject()) array-setVertexBufferObject(0);

line 1178:
if (!elements-getElementBufferObject()) 
elements-setElementBufferObject(0);

The ! should be removed in all three cases.

thx
cm
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org


Re: [osg-users] Questions about Geometry implementation details

2007-10-19 Thread Christian Muschick
Robert Osfield wrote:
 Hi Christian,
 
 On 10/19/07, Christian Muschick [EMAIL PROTECTED] wrote:
 Sorry to bother you again, but did you take a look at the other lines of
 code I pointed out? I think the same problem exists there.

 line 1123:
 if (!array-getVertexBufferObject()) vbo = array-getVertexBufferObject();

 line 1170:
 if (!array-getVertexBufferObject()) array-setVertexBufferObject(0);

 line 1178:
 if (!elements-getElementBufferObject())
 elements-setElementBufferObject(0);
 
 No I didn't, I assumed it was just the line illustrated.  With
 problems like this its best to be explict, or ideally fix the code to
 what you think it shoudl be and then post a complete file for review,
 I then can do an graphical diff and spot exactly whats changed, what
 problems they address and any pattern involved.

I'll remember that for the next time. Your latest changes seem to fix 
the problems that I spotted, thanks.

regards
cm
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org


Re: [osg-users] Questions about Geometry implementation details

2007-10-19 Thread Ralph Kern
Robert Osfield schrieb:
 On 10/19/07, Ulrich Hertlein [EMAIL PROTECTED] wrote:
 Hello Christian,

 Quoting Christian Muschick [EMAIL PROTECTED]:
 My last request concerning this question didn't get any reply, so let me
 try this reformulated version:
 ...
  osg::Array* array = *vitr;
  if (!array-getVertexBufferObject()) vbo =
 array-getVertexBufferObject(); /// This will only set vbo to NULL!?
  }

  if (!vbo) vbo = new osg::VertexBufferObject;
 }

 In the loop, vbo will never get a value different from the null-pointer.
 Is this on purpose? Did I miss something entirely?
 This certainly looks like a type to me too (copy-and-paste anti-pattern).
 I assume it should read 'if(array-getVertexBufferObject())...' so that if 
 the
 array has a VBO associated it's used.

 But I guess in the end Robert has to check that.
 
 I'm rather lost on this topic as I've just spotted this single email
 on its own, which bit of OSG code are we talking about?
 
 Robert.

from the original post:

In Geometry.cpp, there are several lines of code that don't make sense
to me. More precisely, the conditions in a few if-statements seem to be
incorrect.
As an example, in Geometry::getOrCreateVertexBuffer, it says
{
 ArrayList arrayList;
 getArrayList(arrayList);

 osg::VertexBufferObject* vbo = 0;

 ArrayList::iterator vitr;
 for(vitr = arrayList.begin();
 vitr != arrayList.end()  !vbo;
 ++vitr)
 {
 osg::Array* array = *vitr;
 if (!array-getVertexBufferObject()) vbo =
array-getVertexBufferObject(); /// This will only set vbo to NULL!?
 }

 if (!vbo) vbo = new osg::VertexBufferObject;
}

regards Ralph

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


Re: [osg-users] Questions about Geometry implementation details

2007-10-19 Thread Robert Osfield
Hi Christian,

I've done a check through the code and changed where I think there are
issues - 4 more beyond the one I fixed earlier.  I've checked these
changes in.

The changes I've made can be seen on the Tracs revision log:

http://www.openscenegraph.org/projects/osg/log/OpenSceneGraph/trunk/src/osg/Geometry.cpp

Could you check the code over and see if I've finally licked this copy
and paste bugs.

Cheers,
Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org


Re: [osg-users] Questions about Geometry implementation details

2007-10-19 Thread Robert Osfield
Hi Christian,

On 10/19/07, Christian Muschick [EMAIL PROTECTED] wrote:
 Sorry to bother you again, but did you take a look at the other lines of
 code I pointed out? I think the same problem exists there.

 line 1123:
 if (!array-getVertexBufferObject()) vbo = array-getVertexBufferObject();

 line 1170:
 if (!array-getVertexBufferObject()) array-setVertexBufferObject(0);

 line 1178:
 if (!elements-getElementBufferObject())
 elements-setElementBufferObject(0);

No I didn't, I assumed it was just the line illustrated.  With
problems like this its best to be explict, or ideally fix the code to
what you think it shoudl be and then post a complete file for review,
I then can do an graphical diff and spot exactly whats changed, what
problems they address and any pattern involved.

Cheers,
Robert.

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


Re: [osg-users] Questions about Geometry implementation details

2007-10-19 Thread Robert Osfield
Hi Ralf,

Thanks for the clarification, I'm afraid my trip meant not following
threads too closely.

I've reviewed the code and the if (!array-getVertex..()) line is
wrong, and Ulrich pointed to the correct implementation.

I've fixed the relevant code and checked it into SVN.

Robert.

On 10/19/07, Ralph Kern [EMAIL PROTECTED] wrote:
 Robert Osfield schrieb:
  On 10/19/07, Ulrich Hertlein [EMAIL PROTECTED] wrote:
  Hello Christian,
 
  Quoting Christian Muschick [EMAIL PROTECTED]:
  My last request concerning this question didn't get any reply, so let me
  try this reformulated version:
  ...
   osg::Array* array = *vitr;
   if (!array-getVertexBufferObject()) vbo =
  array-getVertexBufferObject(); /// This will only set vbo to NULL!?
   }
 
   if (!vbo) vbo = new osg::VertexBufferObject;
  }
 
  In the loop, vbo will never get a value different from the null-pointer.
  Is this on purpose? Did I miss something entirely?
  This certainly looks like a type to me too (copy-and-paste anti-pattern).
  I assume it should read 'if(array-getVertexBufferObject())...' so that if 
  the
  array has a VBO associated it's used.
 
  But I guess in the end Robert has to check that.
 
  I'm rather lost on this topic as I've just spotted this single email
  on its own, which bit of OSG code are we talking about?
 
  Robert.

 from the original post:

 In Geometry.cpp, there are several lines of code that don't make sense
 to me. More precisely, the conditions in a few if-statements seem to be
 incorrect.
 As an example, in Geometry::getOrCreateVertexBuffer, it says
 {
  ArrayList arrayList;
  getArrayList(arrayList);

  osg::VertexBufferObject* vbo = 0;

  ArrayList::iterator vitr;
  for(vitr = arrayList.begin();
  vitr != arrayList.end()  !vbo;
  ++vitr)
  {
  osg::Array* array = *vitr;
  if (!array-getVertexBufferObject()) vbo =
 array-getVertexBufferObject(); /// This will only set vbo to NULL!?
  }

  if (!vbo) vbo = new osg::VertexBufferObject;
 }

 regards Ralph

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

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


Re: [osg-users] Questions about Geometry implementation details

2007-10-18 Thread Ulrich Hertlein
Hello Christian,

Quoting Christian Muschick [EMAIL PROTECTED]:
 My last request concerning this question didn't get any reply, so let me
 try this reformulated version:
...
  osg::Array* array = *vitr;
  if (!array-getVertexBufferObject()) vbo =
 array-getVertexBufferObject(); /// This will only set vbo to NULL!?
  }

  if (!vbo) vbo = new osg::VertexBufferObject;
 }

 In the loop, vbo will never get a value different from the null-pointer.
 Is this on purpose? Did I miss something entirely?

This certainly looks like a type to me too (copy-and-paste anti-pattern).
I assume it should read 'if(array-getVertexBufferObject())...' so that if the
array has a VBO associated it's used.

But I guess in the end Robert has to check that.
/ulrich
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org