Hi Robert

2013/5/28 Robert Osfield <[email protected]>

> Hi David,
>
> On 24 May 2013 10:11, David Callu <[email protected]> wrote:
> > I update the sumission to be based on last commit
>
> I have just reviewed your changes for pointer attribute type and
> currently not sure what the best thing to do is.  Right now I'm not
> comfortable with having two places to define the type as they could be
> conflict i.e. you assign a DOUBLE attribute but provide an osg::Array
> with a Vec3.  As the the osg::Array already has a parameter that
> explicitly specifies it's type it would seem sensible to leverage this
> when deciding upon when version of glVertexAttribPointer to call.
>
> Looking at the docs to glVertexAttribPointer it looks like the I and L
> versions are there to preserve type rather force the usual OpenGL
> style conversion to float, and sometimes it will still be appropriate
> to let this default case to float happen even for double or int arrays
> so this would imply that one should have a preserve type where
> possible flag on osg::Array and if this is defined then one would
> select the I or L versions when the type is U/CHAR, U/SHORT, U/INT, or
> DOUBLE respectively.
>

You right, a simple "preserve" flag will avoid use of glVertexAttrib*Pointer
with inappropriate data type


> Another concern I have is having if/switch statements used in code
> that is called often -

The best way i know to remove if/switch or other control flow keyword, is
virtual function
so my first idea was to add functor in ArrayData like this

        struct OSG_EXPORT ArrayData
        {
            struct ApplyOperatorABC
            {
                ApplyOperator(ArrayData & data) : _data(data) {}
                virtual void operator() (osg::State & state, unsigned int
index) = 0;

                ArrayData & _data;
            };

            struct ApplyFloatOperator : public ApplyOperatorABC
            {
                ApplyFloatOperator(ArrayData & data) : ApplyOperator(data)
{}

                virtual void operator() (osg::State & state, unsigned int
index)
                { state.setVertexAttribPointer( index, _data.array,
_data.normalize ); }
            };

            struct ApplyIntOperator : public ApplyOperatorABC
            {
                ApplyIntOperator(ArrayData & data) : ApplyOperator(data) {}

                virtual void operator() (osg::State & state, unsigned int
index)
                { state.setVertexAttribIPointer( index, _data.array ); }
            };

            struct ApplyDoubleOperator : public ApplyOperatorABC
            {
                ApplyDoubleOperator(ArrayData & data) : ApplyOperator(data)
{}

                virtual void operator() (osg::State & state, unsigned int
index)
                { state.setVertexAttribLPointer( index, _data.array ); }
            };

            ....
            ....
            ....
            ApplyOperatorABC * _applyOperator;
        };



> we are again introducing an extra CPU overhead
> to everyone for niche usage.  I haven't yet thought a way of avoid
> this though - it might require modifications to how
> osg::Array/osg::State/osg::Geoemtry all handle arrays, it may even be
> that we can't avoid some form of overhead if we want this feature.
>
>

Well, we introduce a new feature provide by OpenGL and not yet provided by
OSG.
The cost is an extra CPU overhead

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