Page 1 of 1

RT Shader - Proposed usability improvements for 1.9 (RTSS)

Posted: Sat Mar 24, 2012 4:35 pm
by Mattan Furst
I have been working with the RTSS component for quite a while now. And over the course that my work I have gathered a list of features which I would like to implement in the RTSS. I belive these features will improve the usibility of the system. I submit this list for the approval of the community.
(Note that some of these ideas have been previously proposed by others but were never implemented)

RTSS - Real time shader system
SRS - Sub-render state

cleaner code features
  • Remove the internalOrder (third) parameter from the FunctionInvocation constructor, and make the groupOrder (second) parameter optional.

    From what I've seen the code for creating FunctionInvocation in SRSs is almost always the same. There are 2 parameters defined in every SRS one is used to carry the groupOrder the other the internalOrder. The value groupOrder parameter is almost never changed, while the internalOrder is constantly incremented for every creation of a FunctionInvocation. This is duplicate code which can be removed.

    Both values can be replaced by parameters defined in the RTShader::Function class. The internalOrder equivalent parameter will need to be updated once at the beginning of the call to addFunctionInvocations. The internalOrder equivalent parameter will be assigned and incremeted every time a FunctionInvocation is added to a function.

    The groupOrder parameter in the FunctionInvocation constructor should be kept as optional parameter to provide a way to overide the groupOrder parameter in the RTShader::Function class. This is both for easier converting of older code and to provide an overriding value for unique functions.
  • All commonly used functions such as assign, add, transform should have helper function to write them.

    Currently to write a common function into a shader takes several lines of code. This should be replaced by single line of code using "helper" functions. For instance, a function for creating assignment or multiplication can have the prototypes:

    Code: Select all

    void ShaderFunctionUtil::addFunctionAssign(Program* prg, FunctionArg assignedValue, FunctionArg target); 
    void ShaderFunctionUtil::addFunctionTransform(Program* prg, FunctionArg multiplicand, FunctionArg multiplier, FunctionArg target);
    FunctionArg (short for function argument) will be a new class which will describe either a parameter, a parameter with a mask, a const value (e.g. float, Vector3, etc...) or a GpuProgramParameters enum to describe a uniform parameter.

    So that this code:

    Code: Select all

    UniformParameterPtr wvpMatrix = vsProgram->resolveAutoParameterInt(GpuProgramParameters::ACT_WORLDVIEWPROJ_MATRIX, 0);
    	...
    	vsProgram->addDependency(FFP_LIB_TRANSFORM);
    	...
    	FunctionInvocation* transformFunc = OGRE_NEW FunctionInvocation(FFP_FUNC_TRANSFORM, FFP_VS_TRANSFORM, 0); 
    	transformFunc->pushOperand(wvpMatrix, Operand::OPS_IN);
    	transformFunc->pushOperand(positionIn, Operand::OPS_IN);
    	transformFunc->pushOperand(positionOut, Operand::OPS_OUT);
    	vsEntry->addAtomInstance(transformFunc);

    will be condenced to this code:

    Code: Select all

    ShaderFunctionUtil::addFunctionTransform(vsProgram, 
    		FunctionArg(GpuProgramParameters::ACT_WORLDVIEWPROJ_MATRIX),
    		FunctionArg(positionIn), FunctionArg(positionOut));

Easier integration and cleaner code features
  • Remove the concepts of input / local / output parameters from SRS building. (Note: I'm not 100% sure about the following idea)
    Currently when a user wants to use a specific type of non-uniform shader parameter (position, normal, texcoord, etc...), he has to define between 1 and 3 parameters to hold the value of that parameter.
    • An input parameter to recive the value of the parameter from the vertex buffer / vertex shader.
    • An optional local parameter to contain and manipulate the value of the parameter.
    • An output value to send the parameter to the pixel shader or color output.
    This way of programming is problematic for 2 reasons. One it create longer and less understandable code at the level of the SRS. The second is that it make integration of new SRSs problematic as it is hard for them to know sometimes from where or to where they should set or get the parameter value from (from an input a local or an output parameter).

    This can be replaced with the following:
    • The 3 parameter types (input, local and output) should be combined into a new single parameter ("combined parameter"). More akin in concept to the local parameter then to the other input or output.
    • When creating a combined parameter the user will need to specify how the parameter is initialized. Whether through a const value or through a value from a vertex buffer. This will tell the system whether an input parameter is needed as well.
    • For pixel shader parameters the user will be able to define vertex shader parameters from which the parameter will be initialized. This should automaticly create the input and output parameters for sending the value from the vertex shader and reciveing it in the pixel shader.
    • The first few lines of every shader should be resereved for initializing the values of these new types of parameter.
    • The last few lines of every shader should be reserved for assigning values from these type of parameters to shader output parameters.
  • The way that the RTSS is designed to work is that each SRS component is ment to do a small section of the overall computations required in the shader. Where each component is a "black box" unaware of what other SRS components do.

    This unfortunatlly does not produce the best results. Consider the transform SRS and the lighting SRS. The per pixel lighting SRS requires the vertex normal in camera space. Because lighting SRS is not aware the transform SRS it has to attempt to convert the normal from object to view space by itself. it takes the normal in object space and converts it through the world-matrix parameter. However some transform (hardware skinning) SRSs components don't use the world-matrix. They use other parameters. So to overcome the fact that the pixel shader is not aware of them they have to calculate the normal in world space then convert it back to object space, so that the lighting SRS can pick it up from there. This is awkward inefficient code.

    Before all the SRSs are called serially to produce their shader code (resolveParameters() and createCpuSubPrograms()), There needs to be a pre-stage where the SRSs declare what they need or what they expect. That way in the previous example the lighting stage should declare that it requires the normal in view space. And the transform SRS can create it.

    If I combine this idea with the previously listed idea I come up with this: When creating a combined parameter, instead of having to specify the initialization value, the programmer will be able to define the parameter as "waiting for initialization code". These type of parameters will be created at the above described decleration stage. When the next stage of shader code production arraives, SRSs such as the transform SRS will be able to tell whether there is a value the need to provide and do so if neccessary.
Minor missing features
  • Texture SRS should support using parameters with greater size than the texture requires.
  • Add a tag ("texture_use") that will inform the texture SRS that it does not require to use a texture. Extend this tag to SRSs that use textures such as normal map lighting.
  • Add tags that add the ability block basic SRSs, such as transform, texture, color, light and fog.
  • Add ability to tell the system that when an SRS should not be used, the next SRS for the same group order should be checked.
  • Better naming conventions for parameters inside shaders (for easier debugging).

Far future developments
  • support geometric shaders

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Mon Mar 26, 2012 5:25 pm
by al2950
I have run into the same problems as you, and I had planned to start making some changes to the RTSS to make it much easier to integrate your own SRS. See this post; http://www.ogre3d.org/forums/viewtopic.php?f=4&t=66521
However work got in the way, and is still in the way!I do have a chunk of time coming up which I hope to devote to improving the RTSS.

In that post I did get a bit carried away with the node based system, but I think some of the points are still valid. Here are my thoughts on your post;
  • Removing internalOrder from FunctoinInvocation - I agree with this to some point, however forcing the developer to increment it themselves helps them to understand how the RTSS creates programs.
  • Common function helper methods - I like this idea :)
  • Remove input/local/output parameters - Need to have a more in depth look at this but I think your basic concepts make it easier to understand and use
  • The last point! - Again I agree with the problem, but I am not sure I fully understand your solution
I also agree with you minor list, although I dont fully understand all of them! Below is a list of extra features I believe are important, they may well overlap with your ideas
  • Add built in support for texture parmeters - When defining a SRS in a material file i believe you should just have to reference the texture_unit block. That way you can use all the texture parameters, eg filtering. Moreover I think you should be able to define what parts of a texture it uses. For example with a specular map you might want to use a completely different texture or you might want to just use the alpha of the diffuse texture;

    Code: Select all

    material RTSS_Example
    {
    	technique 
    	{
    		pass
    		{
    			ambient 0.470588 0.470588 0.470588 1
    			diffuse 0.698039 0.698039 0.698039 1
    			specular 0.368627 0.368627 0.368627 1 20
    	
    			texture_unit Diffuse
    			{
    				texture_alias Diffuse
    				texture Diffuse.dds
    				filtering linear linear linear
    			}
    
    			texture_unit Normal
    			{
    				texture_alias Normal
    				texture Normal.dds
    			}
    			
     			rtshader_system
     			{	 
    				//NB The PPL and normal map are seperate stages
    				lighting_stage per_pixel Diffuse.rgb
    				normal_map Normal.rgb tangent_space 0
    				spec_map Diffuse.a 20
     			}			
    		}
    
    	}
    }
    
  • As you have seen from the above example, we must make it possible to separate the normal map stage from the per pixel stage, so you can easily alter the normal value before the per pixel stage. However there needs to be a way of setting SRS dependencies, for example in this case Normal map depends on having the per pixel lighting SRS. I dont think it should auto apply the missing dependencies but at least throw an error.
  • Further to the point above, I think your method for 'initialising parameters' is in the right direction, however I think we should add the ability to override certain parameters (ie ones offered up by SRS). This would make it a lot easier to, for example, to overwrite the input parameter for the normal in PPL. This would make it easy to insert a 'normal' effect and not worry about working out the exact execution order or your simple SRS, which may break if someone adds a SRS in the future. (Hope this makes sense!)
  • Add the ability to ignore certain SRS on some certain material. (I think this is similar to one of your minor points)
It has been a while since Ive had any time to look at the RTSS so please excuse me if I am technically wrong anyway, but please correct me!

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Mon Mar 26, 2012 8:05 pm
by Mattan Furst
"Removing internalOrder from FunctoinInvocation - I agree with this to some point, however forcing ..."
I already started to implement this on my computer. in order not to break compatibility I made both parameters optional. so for now both forms can be supported
Remove input/local/output ...
The last point! - Again I ...
I myself don't yet have a clear picture. Truth be told It's not a change I am eager to make. Any big change like that will mean a large break in backward compatibility. Where I work at I have already implemented over a dozen different SRSs. Which means I will need to re-write them. "I just made a bunch of changes to Ogre. So now everything I did in the RTSS needs to be rewritten" is not a conversation I would like to have with my bosses :?.

I won't check-in anything that breaks compatibility without a clear design and a wide community acceptance for it.
Add built in support for texture parmeters - When ...
If you look at the SRS factory class there are createInstance() has 3 prototype version (overloads). one of them is this:

Code: Select all

virtual SubRenderState* createInstance(ScriptCompiler* compiler, PropertyAbstractNode* prop, TextureUnitState* texState, SGScriptTranslator* translator)
(note the TextureUnitState parameter)

This overloaded version is set to accept material scripts in the form of

Code: Select all

material RTSS_Example
{
   technique 
   {
      pass
      {
        ...
         texture_unit 
         {
            ...

            rtshader_system
            {    
               [b]property value[/b]
            }
         }         
      }
   }
}
The code just needs a bit of rewiring in this case

More specifically about normal map SRS. I already had a bit of a struggle with the normal map SRS (see http://www.ogre3d.org/forums/viewtopic.php?f=4&t=69405). long story short I implemented my own version of normal map SRS to complement my own version of per pixel lighting (http://www.ogre3d.org/forums/viewtopic.php?f=4&t=69503)

The RTSS is not going to be an easy system to make both developer friendly and efficient.
... 'initialising parameters' is in the right direction, however I think we should add the ability to override certain parameters ...
I agree. I think I sort of thought about but than I though that choosing how to initialize the combined parameters might be a solution for that. Not so sure about it (my previous thoughts) right now.

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Mon Mar 26, 2012 8:21 pm
by Mattan Furst
P.S.
I had seen previously your node system. As a programmer it's something that looks really alluring to me. However @Assaf Raman (he's my boss at work) tried to show a program similar in concept to this to our art guy (It may have been cgfx). He wasn't as fond of the concept to say the least.

He basically said that he's not a programmer nor does he have any inclinations to be one. He just wants to have a bunch of preset options that he can choose to turn on and off. And not to write a complex shader tree.

If you still want to explore that path. I believe assaf started working on a cgfx to ogre converter (before we realized our art guy doesn't want anything to do with it). It should be in the forums somewhere.

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Mon Mar 26, 2012 8:57 pm
by spacegaier
Mattan Furst wrote:If you still want to explore that path. I believe assaf started working on a cgfx to ogre converter (before we realized our art guy doesn't want anything to do with it). It should be in the forums somewhere.
Yes, he has: http://www.ogre3d.org/forums/viewtopic.php?f=4&t=44405

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Fri Mar 30, 2012 5:55 pm
by al2950
I had seen previously your node system. As a programmer it's something that looks really alluring to me. However @Assaf Raman (he's my boss at work) tried to show a program similar in concept to this to our art guy (It may have been cgfx). He wasn't as fond of the concept to say the least
Yeah i definitely got carried away with that idea! However if you forget about the node thing, the concept of being able to override input parameters would be really useful.
If you look at the SRS factory class there are createInstance() has 3 prototype version (overloads). one of them is this:

Code: Select all

virtual SubRenderState* createInstance(ScriptCompiler* compiler, PropertyAbstractNode* prop, TextureUnitState* texState, SGScriptTranslator* translator)
I never noticed that that overload, thank you for pointing it out. :)
I won't check-in anything that breaks compatibility without a clear design and a wide community acceptance for it.
Breaking compatibility is always frowned apon in libraries such as Ogre, however best to do it now than later when it is used by more and more people, especially as DX11 and gles dont have fixed function pipelines. I would really like to get Nir's views on changing the RTSS and come up with an agreed design.

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Tue May 29, 2012 7:16 pm
by mukik182
The way that the RTSS is designed to work is that each SRS component is ment to do a small section of the overall computations required in the shader. Where each component is a "black box" unaware of what other SRS components do.

This unfortunatlly does not produce the best results. Consider the transform SRS and the lighting SRS. The per pixel lighting SRS requires the vertex normal in camera space. Because lighting SRS is not aware the transform SRS it has to attempt to convert the normal from object to view space by itself. it takes the normal in object space and converts it through the world-matrix parameter. However some transform (hardware skinning) SRSs components don't use the world-matrix. They use other parameters. So to overcome the fact that the pixel shader is not aware of them they have to calculate the normal in world space then convert it back to object space, so that the lighting SRS can pick it up from there. This is awkward inefficient code.

Before all the SRSs are called serially to produce their shader code (resolveParameters() and createCpuSubPrograms()), There needs to be a pre-stage where the SRSs declare what they need or what they expect. That way in the previous example the lighting stage should declare that it requires the normal in view space. And the transform SRS can create it.

If I combine this idea with the previously listed idea I come up with this: When creating a combined parameter, instead of having to specify the initialization value, the programmer will be able to define the parameter as "waiting for initialization code". These type of parameters will be created at the above described decleration stage. When the next stage of shader code production arraives, SRSs such as the transform SRS will be able to tell whether there is a value the need to provide and do so if neccessary.
I haven't worked with this system at all and my OGRE knowledge is quite thin but I thought of a possibly elegant solution to this problem: using smaller elements in a chainable fashion so that transformations between the stages you propose must always be defined.

I hope it's not completely stupid idea.

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Tue May 29, 2012 8:50 pm
by Mattan Furst
It's not a stupid suggestion. There are quite few programs (cgfx?) that try to go that way. The RTShadersystem is in a way also such a system, where an "element" as you call it is reffered to as a sub-render state. But In an "open ended" system such as Ogre which tries to provide the widest solution combining the diffrent sub-render states is problematic.

The whole black boxes concept (which I think your'e suggesting in a sense) relies on the idea that for a defined set of inputs a black box will produced a defined set of output. but the input in these cases are not always well defined or available.

Consider shaders that calculate dynamic light effect. Basic, per vertex, lighting effect work on vertex shaders and will expect world or view space normals and positions. more complex, per pixel effects will be calculated in the vertex shader and will again expect normal and position data in other world or view space.

Now what about bump map effect. Certain bump map work in world space while other work in tangent space. if you have tangent space calculations you have to either re-write the lighting calculations to work also in tangent space or move the tangent space data to world or view space which is performance wise costly.

In a few months someone may want to implement a parallax mapping shader which may have other requirement as to the space in which positions and normal need to be calculated.

And that doesn't even begin to address all the other elements you need such as texturing, transformation, fog, ... Or more advanced areas that aren't yet covered such in the RTShadersystem as geometric shaders.

As I see it you basically have 3 routes to take:
1. limit the RTShaderSystem to very specific set of shaders - This is very much against the Ogre engine spirit.
2. make each element able to support the widest variety of data inputs - This will mean a lot of code that will be very hard to maintain. and may also have performance penalties.
3. break the black box concept and on some level make elements aware of one another.

Re: RT Shader - Proposed usability improvements for 1.9 (RTS

Posted: Wed May 30, 2012 7:30 am
by mukik182
Thank you Mattan.

I think my idea would be best explained in terms of granularity, going to finer granularity. I mean:

You need to do a computation (some effect) that requires to compute vertexs and normals in two stages, each stage in different coordinates. If I understood correctly, right now this could be done chaining two SRS which have defined coordinates as input and output that may be incompatible, so to avoid that coordinates are transformed always to a standard system. This sometimes leads to repeatedly useless coordinate system transformations, as the internal computations transform from the standar input to the internal computations and again to the standard output even if the chained stages use internally the same coordinates and making thus at least 2 coordinate transformations completely useless. I uderstand it's like:

"world space coordinates (given)+(computation in world coordinates)+(world to tangent conversion+tangent computation+tangent to world conversion)+..." where each () is a SRS

My idea was referring to create new SRS (or a new granularity level) which simply transform from each coordinate space to each other and chain like:

"world space coordinates (given)+computation in world+world to tangent conversion+computation in tangent space+..." where each one of those is their own SRS (or a sub-SRS).

Adding that as a new sublevel would allow to have the actual SRS for easy effects that can be defined efficiently and everything else would require to use directly the sub-level or create a new user-defined SRS using the sub-level definitions. It may be quite too similar to writing shaders directly, though.

I hope my idea is much clearer than before (and at least as non-stupid as before :P )