sf::Shader -- API to set uniforms

[Ixrec]

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.

[Nexus]

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.

[binary1248]

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.

[Nexus]

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.

[MorleyDev]

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.

[Nexus]

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.

[MorleyDev]

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.

[Nexus]

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.

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.

[MorleyDev]

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 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.

[Ixrec]

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.

[Nexus]

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.

[wintertime]

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.

[binary1248]

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.

[davispuh]

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));

[therocode]

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?