New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New API to set shader uniforms #983

Merged
merged 1 commit into from Nov 1, 2015

Conversation

Projects
None yet
5 participants
@Bromeon
Member

Bromeon commented Oct 5, 2015

Link to forum thread.

This pull request implements #538 and introduces a new API to set uniform variables in sf::Shader. The old setParameter() overloads are deprecated in favor of new setUniform() ones.

The sf::Glsl namespace is added to SFML. It contains type definitions for types that can be passed to Shader::setUniform(). Most types are typedefs to existing SFML types (such as sf::Vector2<T>), there are however a couple of new ones:

sf::Glsl::Mat3  // 3x3 matrix
sf::Glsl::Mat4  // 4x4 matrix
sf::Glsl::Vec4  // 4D float vector
sf::Glsl::Ivec4 // 4D int vector
sf::Glsl::Bvec4 // 4D bool vector

I provided implicit conversions from sf::Transform and sf::Color to these types where appropriate. Where necessary for ambiguity reasons, an explicit conversion must be used:

sf::Color color;
shader.setUniform("color", sf::Glsl::Vec4(color)); // pass as 4D vector
shader.setUniform("color", color); // same call, implicit conversion

sf::Transform transform;
shader.setUniform("matrix", sf::Glsl::Mat3(transform)); // pass as 3x3
shader.setUniform("matrix", sf::Glsl::Mat4(transform)); // pass as 4x4

In addition to basic GLSL types, arrays are now supported as well. They can be specified with Shader::setUniformArray() that takes a pointer to the first element and the length:

sf::Glsl::Vec2 positions[5] = ...;
shader.setUniformArray("positions", positions, 5);

Design rationale

After extensive thinking about the design (see 2 forum threads), I came to the conclusion that setUniform() overloads are more appropriate than named functions setUniformFloat(), for the following reasons:

  • The call is simpler and we don't force the user to type redundant information.
  • We allow polymorphism (a user can write a template to pass uniforms of any type, without knowing the function names).
  • The number of provided methods is smaller. We can work with conversions where possible. For example, we don't need to overload setUniformMat3() for sf::Glsl::Mat3, float* and sf::Transform separately, one method taking sf::Glsl::Mat3 is enough.
  • It is still possible to enforce type safety at compile time and at runtime. The former can be achieved with a wrapper function that explicitly requires the type to be specified.
  • There is no way around dedicated types anyway, otherwise we can't support GLSL arrays (or only in a very low-level way).

Try it out

A self-contained archive containing a C++ file, two GLSL shader files and an image can be downloaded temporarily from my homepage: www.bromeon.ch/sfml/feature-shader-uniforms.zip

@Bromeon Bromeon added this to the 2.4 milestone Oct 5, 2015

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Oct 5, 2015

Member

👍 for finalizing this task.

I have two comments so far:

  1. What about replacing all those functors in Shader.cpp with a single RAII structure similar to TextureSaver, which would save/restore the program binding with its constructor/destructor? This way you can keep the code that sets uniforms in each setUniform function, and you can keep things a little simpler.
  2. I would create a Glsl.inl file to keep Glsl.hpp clean -- especially since all this stuff is private code.
Member

LaurentGomila commented Oct 5, 2015

👍 for finalizing this task.

I have two comments so far:

  1. What about replacing all those functors in Shader.cpp with a single RAII structure similar to TextureSaver, which would save/restore the program binding with its constructor/destructor? This way you can keep the code that sets uniforms in each setUniform function, and you can keep things a little simpler.
  2. I would create a Glsl.inl file to keep Glsl.hpp clean -- especially since all this stuff is private code.
@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 5, 2015

Member

Like this? 😃

Member

Bromeon commented Oct 5, 2015

Like this? 😃

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 5, 2015

Member

Of course, the moment I wrote that, I realized that I could as well have outsourced the entire class definitions, since a declaration is enough for typedef...

So it should be even cleaner now.

Member

Bromeon commented Oct 5, 2015

Of course, the moment I wrote that, I realized that I could as well have outsourced the entire class definitions, since a declaration is enough for typedef...

So it should be even cleaner now.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Oct 5, 2015

Member

I haven't got time to check everything in details but I would have included the .inl at the end of the .hpp (*) file from outside both sf and priv namespaces, and added those in the .inl file so that when reading this file we know without looking at any other file in which namespace things are declared.

(*) if I remember correctly it's file to define a typedef to an incomplete type if it's full definition is available when using it so in this case it should work I believe.

Member

mantognini commented Oct 5, 2015

I haven't got time to check everything in details but I would have included the .inl at the end of the .hpp (*) file from outside both sf and priv namespaces, and added those in the .inl file so that when reading this file we know without looking at any other file in which namespace things are declared.

(*) if I remember correctly it's file to define a typedef to an incomplete type if it's full definition is available when using it so in this case it should work I believe.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 5, 2015

Member

I haven't got time to check everything in details but I would have included the .inl at the end of the .hpp (*) file from outside both sf and priv namespaces, and added those in the .inl file so that when reading this file we know without looking at any other file in which namespace things are declared.

I would have done it like this too (see Thor), but I looked at Rect.inl and adhered to that style... Laurent? ;)

(*) if I remember correctly it's file to define a typedef to an incomplete type if it's full definition is available when using it so in this case it should work I believe.

Yes, where "using" depends on how you're accessing the type -- it's a matter of whether the type is complete at that time or not. For example, you can declare pointers and references to incomplete types.

Member

Bromeon commented Oct 5, 2015

I haven't got time to check everything in details but I would have included the .inl at the end of the .hpp (*) file from outside both sf and priv namespaces, and added those in the .inl file so that when reading this file we know without looking at any other file in which namespace things are declared.

I would have done it like this too (see Thor), but I looked at Rect.inl and adhered to that style... Laurent? ;)

(*) if I remember correctly it's file to define a typedef to an incomplete type if it's full definition is available when using it so in this case it should work I believe.

Yes, where "using" depends on how you're accessing the type -- it's a matter of whether the type is complete at that time or not. For example, you can declare pointers and references to incomplete types.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Oct 5, 2015

Member

I would have done it like this too (see Thor), but I looked at Rect.inl and adhered to that style... Laurent? ;)

Good point. Also note that in Thread.inl, the priv namespace is added there.

But I'd say we should avoid this style, at least from now on. What @SFML/sfml think?

Member

mantognini commented Oct 5, 2015

I would have done it like this too (see Thor), but I looked at Rect.inl and adhered to that style... Laurent? ;)

Good point. Also note that in Thread.inl, the priv namespace is added there.

But I'd say we should avoid this style, at least from now on. What @SFML/sfml think?

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Oct 6, 2015

Member

Also note that in Thread.inl, the priv namespace is added there.

In Thread.inl, the namespace priv surrounds local private stuff, not the class definition.

I would have done it like this too (see Thor), but I looked at Rect.inl and adhered to that style... Laurent?

Do you really need to ask if you should do like in all other files? ;)

Regarding the modifications: looks perfect now 👍

Member

LaurentGomila commented Oct 6, 2015

Also note that in Thread.inl, the priv namespace is added there.

In Thread.inl, the namespace priv surrounds local private stuff, not the class definition.

I would have done it like this too (see Thor), but I looked at Rect.inl and adhered to that style... Laurent?

Do you really need to ask if you should do like in all other files? ;)

Regarding the modifications: looks perfect now 👍

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 6, 2015

Member

Do you really need to ask if you should do like in all other files? ;)

No, I already did it like in all other files ;) the question is rather:

But I'd say we should avoid this style, at least from now on. What does @SFML/sfml think?

But it's a different question, so let's discuss it elsewhere and not pollute this pull request any further....


Regarding the modifications: looks perfect now 👍

Okay, cool :)

By the way, for those who haven't seen it yet, I attached a small self-contained example to my initial post, so everybody can test it.

Member

Bromeon commented Oct 6, 2015

Do you really need to ask if you should do like in all other files? ;)

No, I already did it like in all other files ;) the question is rather:

But I'd say we should avoid this style, at least from now on. What does @SFML/sfml think?

But it's a different question, so let's discuss it elsewhere and not pollute this pull request any further....


Regarding the modifications: looks perfect now 👍

Okay, cool :)

By the way, for those who haven't seen it yet, I attached a small self-contained example to my initial post, so everybody can test it.

/// <algorithm> and MSVC's annoying 4996 warning in header
///
////////////////////////////////////////////////////////////
void copyMatrix(const float* source, std::size_t elements, float* dest);

This comment has been minimized.

@binary1248

binary1248 Oct 8, 2015

Member

Is there a reason for this parameter ordering? I find following the same ordering as std::memcpy to be more "intuitive" and probably easier for people to remember.

@binary1248

binary1248 Oct 8, 2015

Member

Is there a reason for this parameter ordering? I find following the same ordering as std::memcpy to be more "intuitive" and probably easier for people to remember.

This comment has been minimized.

@Bromeon

Bromeon Oct 8, 2015

Member

The function is identical to C++11 std::copy_n<float*>(). But maybe you're right, it's also more consistent with initializeMatrix() above...

@Bromeon

Bromeon Oct 8, 2015

Member

The function is identical to C++11 std::copy_n<float*>(). But maybe you're right, it's also more consistent with initializeMatrix() above...

Show outdated Hide outdated include/SFML/Graphics/Glsl.inl Outdated
Show outdated Hide outdated src/SFML/Graphics/Shader.cpp Outdated
Show outdated Hide outdated src/SFML/Graphics/Shader.cpp Outdated
@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 12, 2015

Member

I applied the suggestions and some other changes.

  • Within UniformBinder, I left out ensureGlContext(), because as Laurent pointed out, it has already been called at that point. I don't like "better safe than sorry" reasoning, we need to be clear about our logic. I emphasized this with a comment.
  • Renamed the internal functions to copyMatrix(). All overloads start with the source parameter, as that's more idiomatic C++ (STL algorithms).
  • Added a few lines of code to define all setUniform(), setUniformArray() and setParameter() overloads for OpenGL ES, instead of hundreds of lines of empty implementations.
  • Squashed the commits.
Member

Bromeon commented Oct 12, 2015

I applied the suggestions and some other changes.

  • Within UniformBinder, I left out ensureGlContext(), because as Laurent pointed out, it has already been called at that point. I don't like "better safe than sorry" reasoning, we need to be clear about our logic. I emphasized this with a comment.
  • Renamed the internal functions to copyMatrix(). All overloads start with the source parameter, as that's more idiomatic C++ (STL algorithms).
  • Added a few lines of code to define all setUniform(), setUniformArray() and setParameter() overloads for OpenGL ES, instead of hundreds of lines of empty implementations.
  • Squashed the commits.
@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Oct 12, 2015

Member

Within UniformBinder, I left out ensureGlContext(), because as Laurent pointed out, it has already been called at that point.

It still needs to be called (it was there before your modifications, it was for a good reason ;)), in case setUniform is called in a thread that has no active GL context. That's how all GL resources work: they call ensureGlContext() in every function that makes an OpenGL call, even if their constructor already did it, because things could happen in different threads.

Added a few lines of code to define all setUniform(), setUniformArray() and setParameter() overloads for OpenGL ES, instead of hundreds of lines of empty implementations.

Although it reduces the number of lines to type for you, it clutters the public header. I prefer hiding such details, even if it involves typing more useless code for us. It's not as if it was going to change often.

Member

LaurentGomila commented Oct 12, 2015

Within UniformBinder, I left out ensureGlContext(), because as Laurent pointed out, it has already been called at that point.

It still needs to be called (it was there before your modifications, it was for a good reason ;)), in case setUniform is called in a thread that has no active GL context. That's how all GL resources work: they call ensureGlContext() in every function that makes an OpenGL call, even if their constructor already did it, because things could happen in different threads.

Added a few lines of code to define all setUniform(), setUniformArray() and setParameter() overloads for OpenGL ES, instead of hundreds of lines of empty implementations.

Although it reduces the number of lines to type for you, it clutters the public header. I prefer hiding such details, even if it involves typing more useless code for us. It's not as if it was going to change often.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 12, 2015

Member

It still needs to be called

Okay, I misunderstood you. Thanks for the explanation! 😀

Although it reduces the number of lines to type for you, it clutters the public header. I prefer hiding such details, even if it involves typing more useless code for us.

It makes me sad how we have all kinds of powerful abstractions in C++, but resort to silly boilerplate code 😟 are those 15 lines in the header a real problem? I'm not worried about typing (my IDE is powerful enough), but about code readability. Having a .cpp file of which the half are just empty dummy implementations is awful... But I can change it of course, if keeping things out of the header is the priority.

Member

Bromeon commented Oct 12, 2015

It still needs to be called

Okay, I misunderstood you. Thanks for the explanation! 😀

Although it reduces the number of lines to type for you, it clutters the public header. I prefer hiding such details, even if it involves typing more useless code for us.

It makes me sad how we have all kinds of powerful abstractions in C++, but resort to silly boilerplate code 😟 are those 15 lines in the header a real problem? I'm not worried about typing (my IDE is powerful enough), but about code readability. Having a .cpp file of which the half are just empty dummy implementations is awful... But I can change it of course, if keeping things out of the header is the priority.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Oct 12, 2015

Member

It makes me sad how we have all kinds of powerful abstractions in C++, but resort to silly boilerplate code

Don't forget that it is not true code, just a quick and dirty placeholder until we switch to OpenGL ES 2 and have a proper implementation of shaders for all platforms.

are those 15 lines in the header a real problem?

I like keeping public headers clean.

code readability

Nobody's supposed to "read" this code, it's empty.

Having a .cpp file of which the half are just empty dummy implementations is awful...

Would you feel better if we created a separate cpp file for the empty implementation, and leave the original Shader.cpp clean?

Member

LaurentGomila commented Oct 12, 2015

It makes me sad how we have all kinds of powerful abstractions in C++, but resort to silly boilerplate code

Don't forget that it is not true code, just a quick and dirty placeholder until we switch to OpenGL ES 2 and have a proper implementation of shaders for all platforms.

are those 15 lines in the header a real problem?

I like keeping public headers clean.

code readability

Nobody's supposed to "read" this code, it's empty.

Having a .cpp file of which the half are just empty dummy implementations is awful...

Would you feel better if we created a separate cpp file for the empty implementation, and leave the original Shader.cpp clean?

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 12, 2015

Member

I like keeping public headers clean.

I don't see headers so much as the public interface, because they're in fact not. The documentation should be the primary source to learn about the API, headers are more tailored to developers/"power users" than average users. And as a developer, it may be more intuitive to see the same thing handled with less code, but it depends of course on the person.

Still, I totally understand your point, and generally I agree that it's a good idea to let .hpp files give a clean and concise overview of the class.

Would you feel better if we created a separate cpp file for the empty implementation, and leave the original Shader.cpp clean?

I thought about that too, but I don't think it's worth the special case for the configuration script. Handling it in code would require headers, and that's probably only more confusing. I'll just add the empty functions to the bottom of Shader.cpp 😉

Member

Bromeon commented Oct 12, 2015

I like keeping public headers clean.

I don't see headers so much as the public interface, because they're in fact not. The documentation should be the primary source to learn about the API, headers are more tailored to developers/"power users" than average users. And as a developer, it may be more intuitive to see the same thing handled with less code, but it depends of course on the person.

Still, I totally understand your point, and generally I agree that it's a good idea to let .hpp files give a clean and concise overview of the class.

Would you feel better if we created a separate cpp file for the empty implementation, and leave the original Shader.cpp clean?

I thought about that too, but I don't think it's worth the special case for the configuration script. Handling it in code would require headers, and that's probably only more confusing. I'll just add the empty functions to the bottom of Shader.cpp 😉

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Oct 12, 2015

Member

Sooner or later, the backend implementations using the underlying GL objects will probably have to be split anyway since supporting multiple specifications (GL vs GLES) and multiple versions of the same specification (2.1 vs e.g. 4.5 which supports DSA) within a single file will become a complete mess. This is also one of the reasons why I favoured keeping UniformBinder isolated in the .cpp file: when using DSA it becomes completely unnecessary.

Member

binary1248 commented Oct 12, 2015

Sooner or later, the backend implementations using the underlying GL objects will probably have to be split anyway since supporting multiple specifications (GL vs GLES) and multiple versions of the same specification (2.1 vs e.g. 4.5 which supports DSA) within a single file will become a complete mess. This is also one of the reasons why I favoured keeping UniformBinder isolated in the .cpp file: when using DSA it becomes completely unnecessary.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 12, 2015

Member

@binary1248 Okay, good to know. I think we should keep such things in a separate future PR though, there's also some optimization potential.

I applied the changes as discussed above: reintroduced ensureGlContext() and all the empty definitions for OpenGL ES.


Something to keep in mind: the API currently provides an implicit conversion from sf::Color to sf::Glsl::Vec4. While being convenient, this disables future conversions from colors to other GLSL types, most notably sf::Glsl::Ivec4 (or a potential sf::Glsl::Uvec4 that may be supported one day). Even though multiple conversions can coexist, the call setUniform(string, color) will be ambiguous, breaking code that relies on the implicit conversion.

It would be possible to add explicit conversions from sf::Color to other GLSL types. This however would not allow things like

sf::Glsl::Ivec4 colors[3] =
{
    sf::Color::Yellow,
    sf::Color::Cyan,
    sf::Color::Red,
};

So the question really boils down to whether the syntax

shader.setUniform("color", sf::Glsl::Vec4(color));

is such a big issue, that we desperately need

shader.setUniform("color", color);

at the cost of future limitations as outlined above.

To demonstrate the difference, I added another commit that implements implicit conversions from sf::Color to both sf::Glsl::Vec4 and sf::Glsl::Ivec4. I don't think it's inherently bad if the user explicitly specifies whether he needs a conversion to float or integer 4D vectors. It would also be more consistent to sf::Transform and sf::Glsl::Mat3/4:

sf::Color color;
shader.setUniform("color", sf::Glsl::Vec4(color));
shader.setUniform("color", sf::Glsl::Ivec4(color));

sf::Transform transform;
shader.setUniform("transform", sf::Glsl::Mat3(transform));
shader.setUniform("transform", sf::Glsl::Mat4(transform));

What do you think? Or is it unrealistic that people use other GLSL types than vec4 for colors?

Member

Bromeon commented Oct 12, 2015

@binary1248 Okay, good to know. I think we should keep such things in a separate future PR though, there's also some optimization potential.

I applied the changes as discussed above: reintroduced ensureGlContext() and all the empty definitions for OpenGL ES.


Something to keep in mind: the API currently provides an implicit conversion from sf::Color to sf::Glsl::Vec4. While being convenient, this disables future conversions from colors to other GLSL types, most notably sf::Glsl::Ivec4 (or a potential sf::Glsl::Uvec4 that may be supported one day). Even though multiple conversions can coexist, the call setUniform(string, color) will be ambiguous, breaking code that relies on the implicit conversion.

It would be possible to add explicit conversions from sf::Color to other GLSL types. This however would not allow things like

sf::Glsl::Ivec4 colors[3] =
{
    sf::Color::Yellow,
    sf::Color::Cyan,
    sf::Color::Red,
};

So the question really boils down to whether the syntax

shader.setUniform("color", sf::Glsl::Vec4(color));

is such a big issue, that we desperately need

shader.setUniform("color", color);

at the cost of future limitations as outlined above.

To demonstrate the difference, I added another commit that implements implicit conversions from sf::Color to both sf::Glsl::Vec4 and sf::Glsl::Ivec4. I don't think it's inherently bad if the user explicitly specifies whether he needs a conversion to float or integer 4D vectors. It would also be more consistent to sf::Transform and sf::Glsl::Mat3/4:

sf::Color color;
shader.setUniform("color", sf::Glsl::Vec4(color));
shader.setUniform("color", sf::Glsl::Ivec4(color));

sf::Transform transform;
shader.setUniform("transform", sf::Glsl::Mat3(transform));
shader.setUniform("transform", sf::Glsl::Mat4(transform));

What do you think? Or is it unrealistic that people use other GLSL types than vec4 for colors?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Oct 12, 2015

Member

Or is it unrealistic that people use other GLSL types than vec4 for colors?

In an sfml-graphics only context (not sf::Context 😉), it is probably unrealistic since users would not care about how colour data flows through the shader stages. They just accept the fact that colours are vec4s, even more so since SFML still supports legacy GL which provides gl_Color which is a vec4 itself. Just look at the example shaders.

If we are to stay somewhat future-proof, we should probably allow for other types as well. Already now, shaders can be used in a limited way as GL wrappers although I don't know whether people would actually bother using sf::Color in that case. In really advanced use cases, reducing the size of the colour attribute if you know your destination framebuffer only supports e.g. a 32-bit RGBA format (like SFML does with its FBO RenderTextures) might provide some sort of performance improvement.

Member

binary1248 commented Oct 12, 2015

Or is it unrealistic that people use other GLSL types than vec4 for colors?

In an sfml-graphics only context (not sf::Context 😉), it is probably unrealistic since users would not care about how colour data flows through the shader stages. They just accept the fact that colours are vec4s, even more so since SFML still supports legacy GL which provides gl_Color which is a vec4 itself. Just look at the example shaders.

If we are to stay somewhat future-proof, we should probably allow for other types as well. Already now, shaders can be used in a limited way as GL wrappers although I don't know whether people would actually bother using sf::Color in that case. In really advanced use cases, reducing the size of the colour attribute if you know your destination framebuffer only supports e.g. a 32-bit RGBA format (like SFML does with its FBO RenderTextures) might provide some sort of performance improvement.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 14, 2015

Member

Okay. So should I merge the two commits?
Other feedback?

Member

Bromeon commented Oct 14, 2015

Okay. So should I merge the two commits?
Other feedback?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Oct 17, 2015

Member

So should I merge the two commits?

Sure.

I guess this looks fine now. Like I said, there is optimization potential, but I'll leave that for another time in the (hopefully not to distant) future. 😉

👍

Member

binary1248 commented Oct 17, 2015

So should I merge the two commits?

Sure.

I guess this looks fine now. Like I said, there is optimization potential, but I'll leave that for another time in the (hopefully not to distant) future. 😉

👍

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Oct 21, 2015

Member

Now that the corresponding PR has been merged, I introduced the SFML_DEPRECATED macro, and used SFML_DOXYGEN (from #961) to hide implementation details.

I also improved the documentation in several parts, and provided a usage example.

Member

Bromeon commented Oct 21, 2015

Now that the corresponding PR has been merged, I introduced the SFML_DEPRECATED macro, and used SFML_DOXYGEN (from #961) to hide implementation details.

I also improved the documentation in several parts, and provided a usage example.

Show outdated Hide outdated src/SFML/Graphics/Shader.cpp Outdated
Added new methods to set uniforms in sf::Shader
Implements a new design for the shader uniform API.
* Added Shader::setUniform() and Shader::setUniformArray() overloads for the following types:
  -> scalars: float, int, bool
  -> vectors: 2D, 3D, 4D
  -> matrices: 3x3, 4x4
  -> arrays of basic types
  -> samplers (sf::Texture)
  -> conversions for SFML types (sf::Transform, sf::Color)
* Added sf::Glsl namespace with GLSL-equivalent types
* Deprecated Shader::setParameter() overloads

Other related changes:
* Refactored sf::Shader internals to avoid code duplication
* Improved documentation
* Added SFML_DEPRECATED macro to Doxyfile
* Defined _SCL_SECURE_NO_WARNINGS to disable std::copy() warnings on MSVC
@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Oct 25, 2015

Member

Anything left? Final reviews?

Member

eXpl0it3r commented Oct 25, 2015

Anything left? Final reviews?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Oct 28, 2015

Member

Looks good to me. I can't spot anything that I would want to comment on right now, but from what I can tell, this should serve as a good starting point for future work anyway. 👍

Member

binary1248 commented Oct 28, 2015

Looks good to me. I can't spot anything that I would want to comment on right now, but from what I can tell, this should serve as a good starting point for future work anyway. 👍

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Oct 29, 2015

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Oct 29, 2015

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

@eXpl0it3r eXpl0it3r merged commit 9c5c750 into master Nov 1, 2015

@eXpl0it3r eXpl0it3r deleted the feature/shader_uniforms branch Nov 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment