Hi David, The main issue for me is that you want to push changes into the core OSG to support your particular usage case. Your suggested changes even have an overhead that everyone has to pay all for the convenience you want for your particular usage case. To be clear, this is not how I'm prepared to proceed with submissions from any user, the OSG is a general purpose graphics library, one can't go modifying the core to support individual user needs as it'd just end up bloated quagmire.
So far I haven't seen any reason why you can just implement your classes to do what you want. You have a very specific usage case and you'll be writing custom classes to support this any way, all I'm suggesting is that you keep all the elements on your side. Robert. On 30 May 2013 13:53, David Callu <[email protected]> wrote: > Hi Robert > > > The main issue is I want to avoid data duplication. > I don't care of duplicate glDrawElement argument > (mode, size, type) but element array is the problem. > > on other side, just call osg::PrimitiveSet::draw(osg::State, bool) > is really simple and clean. Add other virtual function to use > specific instance, and in futur instanceBase or VertexBase > break the simplicity provided by PrimitiveSet and its utility. > > > So the solution I think is to dissociate PrimitiveSet and > element array, and use osg::Array instead of derive from > Vector{GLubyte, GLushort, GLuint}. PrimitiveSet no longer > derive from BufferData already in osg::Array base class > > Main idea is to dissociate a big data from how to used it. > This is already done with osg::Image and osg::Texture. > > We keep DrawElementsUByte DrawElementsUShort DrawElementsUInt > First reason is to avoid user error : user can't assign an osg::Array > to DrawElement* class but assign a osg::UByteArray, UShortArray > or UIntArray. > Second reason is backward compatibility. > > We could change DrawElement to template and do the same for > futur DrawElementInstanced, DrawElementsBaseVertex, > DrawElementsBaseVertex, DrawElementsBaseVertexBaseInstance, ... > > We could keep instance in PrimitiveSet and push it as depreciated > feature and use DrawElementInstanced instead in future development. > > With this design, user could use one array element with different > PrimitiveSet class/instance. > > to keep inchanged the PrimitiveSet API, DrawElement have to implement > MixinVector function that use under layer osg::Array instance. > > through ? > > > 2013/5/24 Robert Osfield <[email protected]> >> >> 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. >> > I create derived class of all osg::DrawElement* class but I > need add virtual function in PrimitiveSet > void PrimitiveSet::draw(State& state, bool useVertexBufferObjects, unsigned > int numInstance) const > > I use derived class of osg::Geometry. I effectively could solve all my > problem > without change anything to osg but I think this feature could be useful for > other > end-user. > > David > >> Robert. >> _______________________________________________ >> 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
