Hi Aurelien,
I have now done a review of your changes and I don't feel as they
stand are suitable for merging. I still haven't settled upon exactly
how best to proceed, but am thinking about how your changes related to
shader composition side I we probably could make progress there to
make what you want to do possible.
Right now in osg::State we have a
State::_currentShaderCompositionUniformList member variable that can
be populated by StateAttribute::apply() methods, the shader
composition ShadeAttribute sets it's local uniforms in this way. Have
a look at the osgshadercomposition example to see this in action.
Here on attaches the Uniforms to the ShaderAttribute rather than the
StateSet. This approach is "push" uniforms from the StateAttribute on
each apply, rather than "pull" the Uniform's from the StateAttribute
on setup.
The use of "pushing" uniforms on each apply is key to shader
composition as it allows one to compute the values of uniforms on the
fly within the StateAttribute::apply(), so you can do things like
positional state like eye linear glTexGen or glLight, or glClipPlane
style functionality where the uniform values have to be multiplied by
the current modelview matrix to get the final value. You can't do
this with a pull scheme. You can also dynamically decide what
uniforms if any to apply. This approach makes the StateAttribute
responsible for what happens and when.
Now the uniforms still have to be applied, and this currently is done
just in the shader uniform specific code in osg::State::apply(const
StateSet*), the code looks like:
if (_shaderCompositionEnabled)
{
applyShaderComposition();
if (dstate->getUniformList().empty())
{
if (_currentShaderCompositionUniformList.empty())
applyUniformMap(_uniformMap);
else applyUniformList(_uniformMap,
_currentShaderCompositionUniformList);
}
else
{
if (_currentShaderCompositionUniformList.empty())
applyUniformList(_uniformMap, dstate->getUniformList());
else
{
// need top merge uniforms lists, but cheat for
now by just applying both.
_currentShaderCompositionUniformList.insert(dstate->getUniformList().begin(),
dstate->getUniformList().end());
applyUniformList(_uniformMap,
_currentShaderCompositionUniformList);
}
}
}
else
{
applyUniformList(_uniformMap,dstate->getUniformList());
}
Currently in your usage the code will be relying upon the non
_shaderCompositionEnabled else block and standard
StateSet::getUniformList() albeit with your new StateAttribute method
and both will ignore the _currentShaderCompositionUniformList. I'm
now thinking perhaps we should change the State::apply(const
StateSet*) code to be like:
if (_shaderCompositionEnabled)
{
applyShaderComposition();
}
if (dstate->getUniformList().empty())
{
if (_currentShaderCompositionUniformList.empty())
applyUniformMap(_uniformMap);
else applyUniformList(_uniformMap,
_currentShaderCompositionUniformList);
}
else
{
if (_currentShaderCompositionUniformList.empty())
applyUniformList(_uniformMap, dstate->getUniformList());
else
{
// need top merge uniforms lists, but cheat for now by
just applying both.
_currentShaderCompositionUniformList.insert(dstate->getUniformList().begin(),
dstate->getUniformList().end());
applyUniformList(_uniformMap,
_currentShaderCompositionUniformList);
}
}
This approach would enable a StateAttribute::apply() to populate it's
extra uniforms via state.applyShaderCompositionUniform(itr->get());
This then leaves the issue about the modifications of osg::Material,
and the bit mask used to select what to enable/disable. Using a
"push" approach as suggested approach one could keep the bit mask in
osg::State and have the custom osg::Matrial query this within it's
Material::apply() and call state.applyShaderCompositionUniform(..) as
required. This won't require any modifications to osg::StateAttribute
or osg::StateSet.
It also means that one could subclass from osg::Material to create the
custom Material and not require us to make decisions about how best to
implement the uniforms within StateAttribute's like this. This is a
whole topic in itself but one that I think needs to be tackled
separately - basically we need to avoid having multiple member
variables holding the same value, the solution will likely be
something like a templated Uniform, but I'm really not sure yet. I
don't want to have to solve this problem for OSG-3.2 so also don't
feel it'll be productive trying to solve it. What I can say is that
the hoops you jumped through to get osg::Material working with
Uniforms is rather ugly and not something I want to include in the
core OSG as is. It's ugly because we don't yet have a good solution
for managing the Uniforms, but post 3.2 we can look at solving it.
The changes for the bitmask to replace the existing bools for enabling
the built in uniforms something I feel is a step forward and non
detrimental to other codes. What you have implemented so far is
almost what I'd be happy merging but I believe that we'd need to make
sure it replace the existing bools rather than sitting along side it.
Part of me wonders that perhaps this is the wrong way though, if we
want osg::StateAttirubte subclasses be enabled to provide their built
ins then perhaps we should be use the StateAttribute::Type as a part
of osg::State scheme where you can enable/disable the uniforms hint
for each StateAttribute::Type. Actually now I've written it down I
think perhaps this is the correct way to do it - it would avoid the
need for pushing lots of enum's into osg::State header and allow
custom StateAttribute to use their own Type values and still take
advantage of the scheme.
Thoughts?
Robert.
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
