sf::Shader -- API to set uniforms

[Nexus]

Hello,

Several times in the past it has been requested that sf::Shader support more setParameter() overloads, in order to be able to specify other GLSL uniform types.

We recently opened issue #538, where we started to discuss about a possible API to possibly support one or more the following GLSL types:

• 4x4 matrices (const float[16])
• 3x3 matrices (const float[9])
• signed and unsigned integers
• booleans
• arrays

The straightforward approach would be the addition of further setParameter() overloads. This has the problem that for some types there are ambiguities (e.g. float[9] and float[16] are the same types in parameter lists), so we would have to introduce wrappers. Also, the code may not be very expressive, as you can't infer from the function name which type is modified -- the question is, whether the polymorphism provided by overloads is an advantage or not. I think it is, see below.

Another idea is to provide named functions such as setFloat(), setMatrix3x3() and so on. This looks nice and expressive at first, but it comes with the drawback that it makes generic code more difficult. In particular, if we want to provide support for GLSL arrays (which can consist of almost any GLSL type), we effectively have to duplicate all the existing functions, which is of course not ideal.

That's why I personally tend to be in favor of further setParameter() overloads, combined with new types to clearly distinguish the parameters. A function template setParameterArray() or similar could be used to set a uniform array, and deduce from the parameter type which GLSL type is being set. This would avoid a large part of the code duplication.

Generic code also makes it significantly easier for SFML or an extension library to provide support for higher-level data (i.e. GLSL structs), and possibly other types like NxM matrices.

What do you think? Are there other ideas? If you are interested, consider also reading the discussion on GitHub.

[SpectreNectar]

I think it sounds like a great idea with more overloads.
I could personally really make use of a setParameterArray() for what I'm doing...

[Nexus]

What API would you like to use?

[SpectreNectar]

Considering my limited experience I can only say that trafficking of large chunks of all currently supported overloads but as arrays would be extremely useful (if not critically necessary).


Really, what I'd say is keep:
setParameter (const std::string &name, float x)
setParameter (const std::string &name, const Color &color)

...but add
setParameterArray (const std::string &name, float* x, int n)
setParameterArray (const std::string &name, Color* colors, int n)

I'm not too concerned with having all these variations of 1 to 4 floats etc. I''m really only interested in either passing a float or several possibly huge amounts of floats.

[Ixrec]

My two cents on this one:

void Shader::setParameter(const std::string& name, /* one to four floats */)
void Shader::setParameter(const std::string& name, /* one to four bools */)
void Shader::setParameter(const std::string& name, /* one to four ints */)
void Shader::setParameter(const std::string& name, /* one to four unsigned ints */)
// the 2-4 parameter versions are completely optional imo, but would be nice for consistency with the current API

void Shader::setParameterArray(const std::string& name, float* values, std:size_t length)
void Shader::setParameterArray(const std::string& name, bool* values, std:size_t length)
void Shader::setParameterArray(const std::string& name, int* values, std:size_t length)
void Shader::setParameterArray(const std::string& name, unsigned int* values, std:size_t length)

void Shader::setParameterMatrix(const std::string& name, float* values, std:size_t rows, std::size_t columns)
void Shader::setParameterMatrix(const std::string& name, bool* values, std:size_t rows, std::size_t columns)
void Shader::setParameterMatrix(const std::string& name, int* values, std:size_t rows, std::size_t columns)
void Shader::setParameterMatrix(const std::string& name, unsigned int* values, std:size_t rows, std::size_t columns)

Maybe some of these overloads can be template'd together, but I dunno enough about OpenGL to tell.  Looking at the current shader code, it seems unlikely.

Personally I think the "ambiguity" between setParameter(int) and setParameter(float) is acceptable, and probably unavoidable in the general case.  I don't think we can get any safer without requiring the user to pass STL containers or type traits instead of raw array & length tuples.

In fact, setInt(int) and setFloat(float) would be less safe, because with the former nothing stops you from erroneously calling setInt(float).  At least if you call setParameter(float) you're guaranteed to get the float version.

The only thing I know of that's missing here is arrays of arrays/matrices.  I'm assuming those are not very important, since an array of arrays is just a matrix, and if we really need arrays of matrices, then we might as well introduce setParameter3DArray() at that point.

[Nexus]

Thanks for your opinions. Currently, the shader implementation uses a lot of code duplication to set the uniforms, which would definitely have to be reduced if we add more functions.

Here is the list of OpenGL functions to set uniforms. As you see, the signatures follow a very consistent pattern. For me, it seems reasonable to use function pointers, especially to cope with the array signatures

void glUniform{N}{type}v(GLint location,  GLsizei count,  const {Type}* value);

At least internally, we could have a function template that would accept such a function pointer and the corresponding data to set. If we go with a type-inference solution (i.e. overloads of setParameter), the template could also be exposed in the API (of course without the function pointer), and internally we could map types to function pointers.

In fact, setInt(int) and setFloat(float) would be less safe, because with the former nothing stops you from erroneously calling setInt(float).  At least if you call setParameter(float) you're guaranteed to get the float version.

Yes, but you explicitly write that you set an int instead of a float, so it's much more likely that you're aware of the data type than when using something like

shader.setParameter(..., x.getPosition());
// does getPosition now return float or int vector? 2D or 3D?

whereas compilers emit an error or at least a warning when implicit conversion is about to happen. That's actually my main concern.

Something else: I personally think the name "setParameter" is a bit unfortunate. "setUniform" would be more precise, and "uniform" is a term you can assume to be known by the people manipulating shaders, as they have already written the GLSL program behind it. If we'd go with "setFloat" etc., I think the meaning would be clear enough from the context.

[Ixrec]

Now that you mention those points, I have no idea how I feel about setFloat vs setParameter vs setUniform anymore.

It occurs to me this is probably the only API in SFML in which the user is required to both use the API and then also implement it somewhere else (ie, write the shader), so our usual way of thinking about API design doesn't quite apply.  Clearly we can't hide/encapsulate much of anything from the user since he has to use GLSL anyway.

Maybe the goal should be to emulate GLSL as closely as possible, since there will essentially be a one-to-one mapping of GLSL variable declarations to SFML setWhatever() calls, and the user has to write both anyway.  In that case maybe setUniformFloat/setUniformInt/setUniformWhateverArray would be the best approach.

A change to setUniform() might have the additional perk of making backwards compatibility with the current API trivial, since there would be no name clashes.

[Nexus]

When we provide all the setFloat etc., there will be a lot of methods however.

setFloat, setInt, setUint, setBool
-> 4 * 4 = 16 methods 1, 2, 3, 4 scalars
-> 2 * 4 = 8 methods for 2D, 3D vectors

setColor, setTransform, setTexture (sf::Texture and CurrentTextureType)
-> 4 methods

setMatrix2, setMatrix3, setMatrix4
-> 3 methods

setMatrix2x3, setMatrix3x2, setMatrix2x4, setMatrix4x2, setMatrix3x4, setMatrix4x3
-> 6 methods

setFloatArray, setIntArray, setUintArray, setBoolArray
-> 4 * 4 = 16 methods for arrays of 1D, 2D, 3D, 4D vectors

setMatrixNxMArray
-> 9 methods for N,M = 2, 3, 4

=> totally 62 methods to support everything

Of course, we do not have to provide every single overload, but it's something to keep in mind, because eventually a lot of these features will be requested.

While I agree that in GLSL as well as OpenGL, you have to name all the different data types explicitly, C++ is a high-level language that provides abstraction mechanisms (templates) to reduce exactly such code duplication. Ironically, the above list contains even more functions to set basic data types than OpenGL, because OpenGL sets bools via float/int/uint, only provides functions to set arrays of matrices, not matrices themselves, and doesn't provide the high-level wrappers for color or transform.

What might be interesting is to see more real-world examples of code that sets the more complex uniforms from OpenGL code, to get an impression how it is typically used. If you have or know such a project, don't hesitate to bring it in

[binary1248]

What might be interesting is to see more real-world examples of code that sets the more complex uniforms from OpenGL code, to get an impression how it is typically used. If you have or know such a project, don't hesitate to bring it in

What more complex uniforms? A uniform is a uniform... They have certain types and depending on the selected type you have to use the appropriate GL function to upload the data. You might consider struct types or blocks as more "complex" but in the end they are just simple arrays of memory that the driver copies over to the GPU when you call the function, which is what it does for matrices as you already know.

I did a bit of digging around GitHub and found these parts of engine implementations that deal with uniforms:
OGRE
Irrlicht
Crystal Space
Torque 3D

They all look the same... probably because there is no other way to do it. What they all do that SFML doesn't is batch those sets together. SFML is quite... fat... in this regard. If you have to set many many uniforms every frame you end up with much more overhead than you would need because the setParameter() functions only function as a thin wrapper on top of GL and apply the changes immediately as opposed to say... when they are actually required i.e. when the shader is executed during drawing. If you retain the uniform state and set them all only when actually required, you could save all the unbinding and rebinding that is needed now.

If you ask me what the future would look like, I'd say, it looks like the ARB is promoting use of interface blocks in combination with uniform buffer objects and shader storage buffer objects. Since SFML still doesn't have support for any form of buffer objects despite having support for shaders (which I still do not understand...) I guess those are not your concern. But just keep in mind that no matter how clean the API is, anybody who goes so far as to mess with shaders (I don't consider these beginners anymore) will tend to care a bit more about performance as well, and it would be sad to see them have to resort to raw OpenGL just to keep the performance acceptable for whatever their application tends to be.

Put simply: I don't think it matters as much as some might think what the shader API looks like. Whether you call setParameter or setFloat or setFloatParamater or setUniformFloat, it is something you do once (or should do once) in a single place in your code. If you spent that amount of time actually writing and testing your shader code (let's face it, nobody gets it right the first time in non-trivial scenarios) then you would have realized errors in your C++ code anyway and designing the API in terms of reducing coding errors is a waste of time if you can still screw up everywhere in your shader code, which is what will happen 95% of the time. What matters is the implementation.

Just give those who want the extra flexibility/power they need what they need and stop worrying about the rest . As you should only optimize code after profiling you should also only redesign APIs after getting feedback from the people who use them (good luck finding these people). It might turn out that all of your assumptions were false and you are just wasting time trying to think of ways to prevent things that never happen. You know my opinion: more overloads, buffer objects, more performance.

[Nexus]

Thanks a lot for the research and detailed answer.

What more complex uniforms? A uniform is a uniform...

Sorry for being unclear, I meant a more "complex" API than what SFML currently provides, i.e. fully exploiting the different GLSL data types and not only setting a few float coordinates.

Regarding the way how they set uniforms, CrystalSpace and Torque3D seem to use overloads of the same function, and they also have a dedicated data type to store only uniforms.

What they all do that SFML doesn't is batch those sets together. SFML is quite... fat... in this regard. If you have to set many many uniforms every frame you end up with much more overhead than you would need because the setParameter() functions only function as a thin wrapper on top of GL and apply the changes immediately as opposed to say... when they are actually required i.e. when the shader is executed during drawing. If you retain the uniform state and set them all only when actually required, you could save all the unbinding and rebinding that is needed now.

That's a good point. As I see it, it's more a question of implementation, as this caching/batching could be done internally? Or do you think a simple API with setters gives too few control to the user?

In general, the use of more modern OpenGL might change the way how shaders are used in SFML... So even if the idea is currently to improve the SFML 2 API, it doesn't hurt if we already have a rough impression how this could look in the future, so we don't have unnecessarily huge changes.

you should also only redesign APIs after getting feedback from the people who use them (good luck finding these people). It might turn out that all of your assumptions were false and you are just wasting time trying to think of ways to prevent things that never happen.

You're probably exaggerating, but API design is not wasted time just because you may make mistakes, in the same way that thinking about  the correct data structures and algorithms is not premature optimization. It's true that it takes some time to come up with intuitive and error-robust interfaces, but it's usually worth the result.

At least, I know from my own experience that even small flaws like the Java Set interface not providing a lookup method can be really annoying

[binary1248]

That's a good point. As I see it, it's more a question of implementation, as this caching/batching could be done internally? Or do you think a simple API with setters gives too few control to the user?

Whether using SFML, any of those engines or raw GL, when the user sets a uniform they always mean the same thing: make sure the shader uses these values the next time they are run (keeping the command queue in mind). So it doesn't really matter when the GL functions are called. All that matters is that it takes place before the next draw command using the shader. Even if there was some explicit way for the user to specify when the upload takes place, they would have to eventually call it before they draw, and they would probably also do it right before they draw to maximize performance if they are unsure from where in their code the uniforms might be set. If the library takes care of this for them, all the better.

You're probably exaggerating, but API design is not wasted time just because you may make mistakes, in the same way that thinking about  the correct data structures and algorithms is not premature optimization. It's true that it takes some time to come up with intuitive and error-robust interfaces, but it's usually worth the result.

At least, I know from my own experience that even small flaws like the Java Set interface not providing a lookup method can be really annoying

Maybe I was exaggerating, but considering what would be a good API design without any real examples (i.e. SFML projects using sf::Shader) to dissect is just trying to predict how code would be written using sf::Shader and after the things I have seen, I am unconvinced that there is any predictability at all. I tend to be a form-follows-function kind of person, and as such I would ask myself what the current API lacks or doesn't sufficiently allow the programmer to achieve before asking myself how it would look best to them. Whether they have something they can use should take precedence over whether they can use it effectively. I don't see any major flaws in the current API, but then again I also don't have any metrics to compare the current and proposed API by. It was always good enough for my needs (not considering lack of support for more data types).

As many things in life, there is no single "right way" to do it. And I've been part of countless discussions as part of my studies, where progress was halted for months by back-and-forth arguments over whether to do it one way or another which were effectively the same in the end, only to be re-implemented the other way a few days later after creating example scenarios. Sometimes all that is needed is some good old pragmatism.

[Ixrec]

I guess it's worth mentioning that in my personal experience using shaders for simple 2D effects (I haven't done any 3D programming yet), I've never had any difficulties on the SFML side.  As binary said 95% of it was from screwing up my GLSL code.  Admittedly I am passing some integers as floats, but that hasn't come back to bite me thus far.

Maybe we should stick to setParameter() overloads for now since that's what everyone's used to, then change names when SFML 3 reimplements and redesigns everything to use modern OpenGL (that is the plan for SFML 3, right?).

[Nexus]

I don't see any major flaws in the current API, but then again I also don't have any metrics to compare the current and proposed API by. It was always good enough for my needs (not considering lack of support for more data types).

I guess it's worth mentioning that in my personal experience using shaders for simple 2D effects [...], I've never had any difficulties on the SFML side.

So far, there was small potential for ambiguity and different types in the API; this will change when we add more overloads. People will have to be a bit more careful that they're using the right types (see example above).

So let's solve the pragmatic issues
How would you provide methods to support matrices? setParameter(float matrix[9]) won't work because matrix is a pointer, and so we can't overload it with another size. How would you like to specify a matrix in the API, or don't you care that much?

What about matrices of NxM size? Even if they're not added immediately, it would be good to have an interface that basically allows them, so that they don't have to be added with duct tape later.

[Ixrec]

I'm not sure how binary feels but I still think setWhateverArray(name, values, length) and setWhateverMatrix(name, values, rows, columns) is the simplest and most intuitive way to go.

[Nexus]

Okay, that doesn't look bad... And arrays of matrices?

setParameterMatrixArray(name, values, rows, columns, length)

...I guess we can initially skip that. As I see it, the general opinion so far is to stay with setParameter(), and add further overloads for other GLSL data types.

We should also consider the naming of such functions. Like this?

setParameter
setParameterArray
setParameterMatrix
setParameterMatrixArray

Or omit the "Parameter" in the others? But then it's not clear that an array is simply a collection of parameters.

setParameter
setArray
setMatrix
setMatrixArray

I'm not sure whether matrices should be given special treatment, they're data types in GLSL like vectors and scalars. Whereas arrays are collections of parameters -- I think it's important to highlight that difference.

The alternative would be a dedicated matrix type, but I don't know if it could be used elsewhere:

setParameter(name, Matrix(values, rows, columns))
setParameter(name, Matrix<rows, columns>(values))

The advantage would be that we only have two method names -- setParameter and setParameterArray, and they behave consistently for every data type, i.e. no exception for matrices. This would simplify generic code on user side.

setParameter
setParameterArray

With "Uniform", it's already quite expressive:

setUniform
setUniformArray