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

Reply via email to