Hi David, On 24 May 2013 11:55, David Callu <[email protected]> wrote: > Up > no change since last submission
I've just done another code review of the original code and your code and unhappy with both, neither is how I'd do it if I had clean slate. I now regret not thinking more about the design when I merged the numInstances submission. If I had a clean slate then I'd be inclined to not have the base class have it's dedicated numInstances member variable, and instead have subclasses deal with this, so we'd have a DrawArrays and DrawArraysInstanced. For you own case you'd have you own PrimitiveSet class. This approach would avoid the need to check the numInstances variable on each invocation to decide which method to call. However, introducing a DrawArrayInstance etc. would break backwards compatibility so isn't ideal. I don't know how widespread use of set/getNumInstances() is so it may not be that widespread of an issue, the serialization issue would be more of an issue though. With retaining backwards compatibility the only other way I can see to make things slightly better than your original solution would be to have the PrimitiveSet::draw(osg::State,bool) method become an inline method rather a virtual. However, it's still an extra CPU overhead that has to be paid by everyone just for privilidge of one user. With such a low level method even an inline method is still too much of an overhead so wouldn't be happy merging even this. Is there a reason why you can just create your own classes that are equivalent to the various PrimitiveSet classes? You could even subclass yourself, I presume you already have your own osg::Geometry or osg::Drawable subclass that generates the numInstance values for you. Robert. _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
