-
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 (https://github.com/SFML/SFML/issues/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.
-
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...
-
What API would you like to use?
-
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.
-
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.
-
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 (https://www.opengl.org/sdk/docs/man/html/glUniform.xhtml) 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.
-
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.
-
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 :)
-
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 (http://www.opengl.org/wiki/Data_Type_%28GLSL%29) 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 (https://github.com/ehsan/ogre/blob/069a43c4c4fcb5264c995fca65a28acd3154b230/RenderSystems/GL/src/GLSL/src/OgreGLSLLinkProgram.cpp#L256)
Irrlicht (https://github.com/zaki/irrlicht/blob/11ea0a762552d69a45e6217c7ac5d24741a02e8f/source/Irrlicht/COpenGLSLMaterialRenderer.cpp#L552)
Crystal Space (https://github.com/crystalspace/CS/blob/3029384a26f1f16d0fa7b05142c2d8aaf5102726/plugins/video/render3d/shader/shaderplugins/glshader_glsl/glshader_glslprogram.cpp#L48)
Torque 3D (https://github.com/GarageGames/Torque3D/blob/aa090daf03640705f5f759ce6abbc6987e7e8e0d/Engine/source/gfx/gl/gfxGLShader.cpp#L622)
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 (http://www.opengl.org/wiki/Interface_Block_%28GLSL%29) in combination with uniform buffer objects (http://www.opengl.org/wiki/Uniform_Buffer_Object) and shader storage buffer objects (http://www.opengl.org/wiki/Shader_Storage_Buffer_Object). 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.
-
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 (http://www.crystalspace3d.org/docs/online/api/classcsShaderVariable.html) and Torque3D (https://github.com/GarageGames/Torque3D/blob/master/Engine/source/gfx/gl/gfxGLShader.h#L117-L137) 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 ;)
-
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.
-
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?).
-
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.
-
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.
-
Okay, that doesn't look bad... And arrays of matrices? :D
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
-
This may be because I'm not an OpenGL guru, but the idea of giving Arrays a special function and not Matrices seems incredibly puzzling, regardless of what those terms technically mean in GLSL. I think that would only be a good idea if our goal was to support every possible kind of parameter you could ever conceivably send to a shader (including things like arrays of matrices of booleans). Similarly, I think a dedicated SFML Matrix type (or a hierarchy of ShaderParameter types/enums) would be a good idea if that was the goal, but for simply extending setParameter(), it's overkill. Maybe this will be the goal in SFML 3, but judging from the OP post here it doesn't seem like it is right now.
Let's just add more overloads in the most obvious/straightforward way possible for now, then by the time SFML 3 development starts we'll know if people really need more features than this or not.
Also, setParameterArray > setArray. The blindingly obvious name is probably the best name for now.
-
This may be because I'm not an OpenGL guru, but the idea of giving Arrays a special function and not Matrices seems incredibly puzzling, regardless of what those terms technically mean in GLSL.
It makes sense because in GLSL, there are basic data types (scalars, vectors, matrices among others) and compounds (arrays, structs) which are collections of basic types.
So, an array is not similar to a matrix in any way -- a vector however is. Matrices and vectors are fixed-size data types (2-4 dimensions), arrays are not. You can have arrays of matrices like of any other data type. So, giving matrices special treatment seems arbitrary.
I think that would only be a good idea if our goal was to support every possible kind of parameter you could ever conceivably send to a shader (including things like arrays of matrices of booleans).
I don't see the need to add a direct overload for such types, but that's exactly why I'm in favor of generic solutions -- let's say we have a
template <typename T>
void setParameterArray(const std::string& name, const T* array, std::size_t length);
method, then that works for any T, including matrices of bools. Of course, internally there will still be some dispatching necessary, but it's far less code than providing every possible combination, and it's also less confusing in the documentation.
-
Isn't sf::Transform already a 4x4 matrix? If you add template parameters it can be used to represent all the other sizes as well and setParameter could call the correct glUniform based on its size.
template <unsigned int Rows, unsigned int Columns>
class Transform
{
...
}
template <unsigned int Rows, unsigned int Columns>
void setParameter(const Transform<Rows, Columns>& transform)
Also about arrays... you must not forget that arrays of structs exist as well as structs of arrays. In the former case one must set each member of an element by telling GL to do so with the array subscript included in the uniform name and in the latter case one can set the whole array using glUniform*v on the member name. I have no idea how setParameter would need to be implemented to automatically support uploading whole structures, if it is even possible, and whether GL would be happy with it.
Also, I would prefer the methods being called setUniform. In every version of GLSL since the start they have been called uniforms, and one can assume that if the user already wrote the shader they would understand what setUniform would do.
-
I'd like to keep sf::Transform as it is, because it is currently used for 2D rendering and has a lot of functionality besides wrapping a matrix (operator overloads, inversion, applying direct translation/rotation/scale...). In places besides sf::Shader, template arguments would only complicate the type without being useful.
I'm rather thinking about a small wrapper
sf::Matrix<N, M> // or
sf::Shader::Matrix<N, M>
that contains no actual functionality. I don't know however if introducing such a type in the main namespace is a good idea, when it's only used for shaders (it would probably lead to confusion with sf::Transform)... And setting a uniform with this type is a bit more verbose, but if genericity is a concern, I see no other way.
About arrays of structs and structs of arrays, I don't think SFML will support them directly in the initial sf::Shader enhancement; but if we ever want to add support, then everything but a generic solution is insane. I have not thought deeply about it, however I could imagine that a type sf::Shader::Struct or similar could then be used, which would store a (name, type, value) mapping. That's already quite advanced, but it's good to keep it in mind for later.
-
I'm in favour of the Matrix wrapper. Maybe not a whole Matrix class, but giving the matrix overload a strong typing.
Requiring the developer to pass naked pointers to the Shader API smells like Primitive Obsession to me. Wrapping that low level concept in some kind of higher level container seems the most user friendly and what I'd expect from a modern C++ API.
-
The question is then whether the user always ends up writing
float* rawPointer = ...;
shader.setParameter("name", sf::Matrix<float, 3, 3>(rawPointer));
or if he can actually use this matrix type meaningfully. If so, that would certainly be nice, because he could also use it as return type (which is not possible with a raw array) and store it elsewhere.
-
SFML has so far only implemented the mathematical operations and structures it needed. The lack of a magnitude with regards to sf::Vector jumps to mind. This does potentially reopen up a bigger design discussion on how much Mathematical operation support should SFML provide.
Using it as nothing more than prettying up the pointer and reducing the number of parameters on a shader function seems like a waste. More appropriate would perhaps if it stored the matrix array internally and provided read/write access to that storage.
From there, SFML could take the current Vector approach, namely store it's relevant fields and provide the simplest operations needed, and developers who need more can write functions and overloading operators to provide that.
Or SFML could provide a more full-featured matrix implementation, but that then raises the question "Where is the dot product function on sf::Vector?". Considering the complexity of matrices, a simple addition would be comparable to a complex vector operation. An inconsistency and a can of worms it may not be the best time to open.
-
Yes, such a Matrix class would be a pure wrapper, with constructors and a few accessor functions.
I'm sure there will be people requesting matrix algebra, but it's just not meaningful if we start implementing it in places where SFML doesn't need it for its own implementation. Considering that even the simplest operations like matrix multiplication are non-trivial to implement efficiently, people who require such functionality should definitely look for a linear algebra library like Eigen, Boost.uBLAS or others (http://scicomp.stackexchange.com/a/353).
The documentation would then also state that the purpose of the class is a type-safe adapter, not computation. In case we use a nested template sf::Shader::Matrix, that will be less of a concern anyway.
-
Agreed. sf::Shader::Matrix definitely conveys that this matrix concept is related to the Shader and not the core Mathematical Matrix concept. I'd assume sf::Matrix was a full-fledged Matrix implementation comparable to the one provided by XNA (http://msdn.microsoft.com/en-us/library/microsoft.xna.framework.matrix.aspx) and be required to look at the documentation to ensure I wasn't missing something and figure out it's much more limited use-cases.
With sf::Shader::Matrix I'd be much more likely to assume was just an adapter without going into the documentation (I regard requiring less documentation visits as being a very good indicator of quality of design).
Though the implementation detail of whether Matrix stores data by-value is a question. Being able to return a sf::Shader::Matrix from a function would simplify integration with the more fully-featured Matrix implementations provided by a maths library.
As an aside, it's a shame SFML 2 can't drop the non-C++11 support. Because std::array would be a useful C++ standard structure for the previous array discussion.
-
Should we add sf::Vector4<T> while we're at it? I'm fine with using wrapper classes as long as we do it consistently.
I would prefer the Matrix type stores its values, partly because that's least likely to surprise people, partly because it seems more flexible, and partly because that's what the Vector types do.
I'm unsure about hiding Matrix inside the Shader namespace because the Vectors don't do that, and the Vectors already have the "where are the linear algebra functions?" problem. Would having Matrices too really make it any worse? Not to mention sf::Transform essentially is a Matrix with a couple of those lin alg functions defined.
-
As an aside, it's a shame SFML 2 can't drop the non-C++11 support. Because std::array would be a useful C++ standard structure for the previous array discussion.
I think the signature
template <typename T>
void setParameterArray(const std::string& name, const T* array, std::size_t length);
should be okay, then the user can have a std::vector, a std::array, a raw array or anything else that stores its elements contiguously.
Should we add sf::Vector4<T> while we're at it? I'm fine with using wrapper classes as long as we do it consistently.
I see your point of consistency, but 4D vectors currently don't have any use case outside the shader.
I would prefer the Matrix type stores its values, partly because that's least likely to surprise people, partly because it seems more flexible, and partly because that's what the Vector types do.
Right, storing the values would allow the user to pass around matrix objects with value semantics.
I'm unsure about hiding Matrix inside the Shader namespace because the Vectors don't do that
I'm not sure whether we should orient ourselves too much on the vectors. Their primary purpose is to act as a coordinate abstraction in rendering (2D) and audio (3D), sf::Shader::setParameter only provides overloads taking them for convenience. We do not have a sf::Vector<N>, for example.
What's more important to me is keeping in mind that there may be further uniform types that could have their own wrappers -- now there is already sf::Shader::CurrentTextureType, and one possibility is to define something similar to sf::Shader::Matrix for GLSL structs/arrays in the future. It makes sense if that is logically aggregated within sf::Shader.
and the Vectors already have the "where are the linear algebra functions?" problem. Would having Matrices too really make it any worse? Not to mention sf::Transform essentially is a Matrix with a couple of those lin alg functions defined.
At the moment, SFML provides more or less exactly the linear algebra functions that it needs itself in the public API. sf::Transform is specialized to 2D rendering with one size, and provides several convenience functions related to 2D transforms. I don't see why SFML should provide a generic, general-purpose matrix class if there are libraries that do it in a much better way.
-
I think adding a matrix class would be the start to adding more and more of a math library and IMO thats only wasting time. In the end people will prefer a real math library like glm anyway and possibly get problems converting to the SFML type.
Maybe for SFML3 consider removing the vector and rectangle types and just including glm to streamline the library? Its easy to use as its a header only library and fits well to GLSL.
-
I still don't know...
I agree that SFML shouldn't try to solve the problems that other dedicated libraries probably already do much better. However for me creating a special sf::Shader::Matrix class seems more contrived than it has to be.
The pragmatist in me has me thinking:
Given that the user will be using e.g. Eigen to solve all their linear algebra needs, they will store some Eigen specific data in each of their objects that needs it.
class SomeClass
{
...
private:
Eigen::Matrix4d m_matrix;
}
And now the user wants to send that matrix to the GL shader:
theShader.setParamater("theMatrix", sf::Shader::Matrix( ... grab data from Eigen matrix ... ));
Wouldn't it be simpler to just allow directly passing the data array to the setParamater() method? In this case some other way of specifying the size of the data that is passed would be needed, and even with a templated sf::Shader::Matrix, the user would still be able to erroneously pass an array of a wrong size and not notice.
Because the user would use the LA library in their own data structures, sf::Shader::Matrix would only be used as part of the setParameter() API. I can't imagine anywhere where they would need to be returned, short of something coming from SFML itself and this also makes little sense in the case of uniforms (unless you want to add getParameter support as well).
Type safety can only be guaranteed if you enforce that the user store SFML types in their objects or if you support conversion from each LA implementation into the SFML type (which is impossible to maintain). As soon as the user is required to manually convert from their stored type into the SFML type through a generic interface, errors can happen. This also has the side effect that depending on how the compiler optimizes and how often this conversion has to be done, there will be some amount of overhead.
-
I still don't know...
I agree that SFML shouldn't try to solve the problems that other dedicated libraries probably already do much better. However for me creating a special sf::Shader::Matrix class seems more contrived than it has to be.
The pragmatist in me has me thinking:
Given that the user will be using e.g. Eigen to solve all their linear algebra needs, they will store some Eigen specific data in each of their objects that needs it.
class SomeClass
{
...
private:
Eigen::Matrix4d m_matrix;
}
And now the user wants to send that matrix to the GL shader:
theShader.setParamater("theMatrix", sf::Shader::Matrix( ... grab data from Eigen matrix ... ));
Wouldn't it be simpler to just allow directly passing the data array to the setParamater() method? In this case some other way of specifying the size of the data that is passed would be needed, and even with a templated sf::Shader::Matrix, the user would still be able to erroneously pass an array of a wrong size and not notice.
Because the user would use the LA library in their own data structures, sf::Shader::Matrix would only be used as part of the setParameter() API. I can't imagine anywhere where they would need to be returned, short of something coming from SFML itself and this also makes little sense in the case of uniforms (unless you want to add getParameter support as well).
I've kinda similar problem. I'm mostly using OpenGL and so I'm also using GLM rather than SFML types. Why? Because I need linear algebra (for 3D) and SFML doesn't provide that properly. Now due that, sometimes I've to convert between GLM and SFML types which isn't pleasant. Also sf::Shader doesn't support uniform blocks so I implemented it myself with direct OpenGL and there I can pass GLM types directly to glBufferData but I can't do that with SFML types.
So for SFML to be usable for everyone, it should define not classes as types, but struct types which have same layout which can be directly used by OpenGL and a lot of libraries already use same layout so they all can be used interchangeably with proper casts. I could pass same GLM types to SFML as I would do for OpenGL. Then similarly like GLM does, SFML could provide some helper Vector/Matrix classes which would operate and work with those types. So you could do like
// Using native SFML classes/types
sf::Matrix::Data matrix = sf::Shader::Matrix.data();
theShader.setParamater("theMatrix", matrix);
or
glm::mat4 matrix; // with GLM
theShader.setParamater("theMatrix", (sf::Matrix::Data)glm::value_ptr(matrix));
-
If SFML defines some maths functionality like its vectors and the proposed matrix, there will always be a fuzzy border between how much functionality should be provided and not.
It will also cause issues in the cases where users want maths functionality and are forced to copy over values between their own maths stuff and SFML's maths stuff. This is not a smooth way of interfacing.
In my opinion, a nice solution would be to deprecate all the sf::Vectors and make it use GLM. The drawback is obviously that it is another dependency, but in my opinion it is worth it. It is also headers only so the dependency is light.
This would give all users access to any kind of linear algebra maths they would need out of the box. It would also mean perfect integration between SFML and the user's maths code. Furthermore it would contain ready types for all GLSL uniforms since GLM is based on GLSL. It would also be no maintenance for the SFML developers since the responsibility for the maths stuff is at the GLM crew.
Has this been considered before? Are there more drawbacks than it being a dependency? Or is that a big drawback enough?
-
Are there more drawbacks than it being a dependency? Or is that a big drawback enough?
I am not sure how much it would affect it, but you would also have to deal with changing over SFML's bindings to the GLM types. And since I haven't taken a look at GLM I don't know how much of an issue converting to the bindings will be.
-
Presumably a switch over to GLM would have to wait for SFML 3?
And if C++11 required is on the cards for SFML 3, most of the current SFML typedefs for integral types could be replaced with the cstdint (http://www.cplusplus.com/reference/cstdint/) definitions (std::uint32_t, std::uint16_t, std::int16_t etc.)
-
I looked up some other C++ libraries that wrap OpenGL shaders, and these are the two best-looking parameter binding interfaces I found:
http://oglplus.org/oglplus/html/classoglplus_1_1Uniform.html
https://github.com/Overv/OOGL/blob/master/include/GL/GL/Program.hpp
I saw several other libraries and classes out there, but most of them support significantly fewer overloads/permutations than we want, so they wouldn't be good references.
Some trends I noticed that are relevant to the previous discussion:
- Everyone treats matrices as a special case. The non-template interfaces have overloads for matrices of specific sizes, and the template interfaces have a separate function for matrices.
- I don't think anybody supports arrays of matrices. However, the types in oglplus' interface are painfully obtuse so I can't be sure.
- Everyone seems to offer a pointer/size interface for parameter arrays, even if they also provide overloads taking std::vectors or other containers.
- Everyone seems to have vector and matrix classes for all the sizes they support. I never saw a pointer/width/height interface for matrices. Admittedly, they are wrapping *all* of OpenGL, and the two I linked above appear to support non-trivial math on their vector/matrix classes.
- And of course, "setUniform" and variants thereof are by far the most popular names for these functions.
Hope this helps.
P.S. Aside from adding yet another dependency, would there be any real downside to incorporating glm in SFML 3? (presumably with some typedefs so users don't need to explicitly use the glm namespace)
-
P.S. Aside from adding yet another dependency, would there be any real downside to incorporating glm in SFML 3? (presumably with some typedefs so users don't need to explicitly use the glm namespace)
Some things that come into my mind:
- Code style does not fit into SFML (typedefs are definitely no solution, as you will face the real names somewhere sometime).
- Added dependency -- GLM is header-only, so this is cheap, but it still has to be maintained
- What do Eigen users do? They still have to convert.
- SFML itself does not really benefit from it, instead it's for use cases for the user which are probably not in SFML's scope.
I'm not against this per se, but SFML does not need as much algebra as the user, and what's needed is provided.
I actually like binary1248's idea: What if the data types of SFML would be binary-compatible to other libraries' data types? They are likely not to change in the future at all, so it's rather safe to cast between them. Maybe the types are even already compatible, I haven't checked this.
-
I am sorry for hijacking this thread, had no idea where should I post.
Anyway, I just upgraded to the current SFML version from git (revision 1217699fe0) and noticed that
fooShader.setUniform("someFloatVal", someIntVal);
behaves differently than the previous version:
fooShader.setParameter("someFloatVal", someIntVal);
I mean, if our shader has
uniform float someFloatVal;
//...
the new version stops working.
In order to fix that I had to add an explicit cast from int to float:
fooShader.setUniform("someFloatVal", (float) someIntVal);
Is this a bug?
-
It's not a bug, if you pass an integer it maps it to an integer uniform.
-
thanks for the response.
weird, the previous version works fine in my case even if I pass an integer.
-
You can post in the Help/Graphics forum.
That's not weird, there was no int overload of the deprecated setParameter(). It's about time that people use the proper C++ types :P