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

Support more types in sf::Shader::setParameter #538

Closed
LaurentGomila opened this Issue Feb 25, 2014 · 23 comments

Comments

Projects
None yet
8 participants
@LaurentGomila
Member

LaurentGomila commented Feb 25, 2014

Add overloads for:

  • 4x4 matrices (const float[16])
  • integers
  • booleans

To be discussed:

  • 3x3 raw matrices (const float[9])
  • arrays

@LaurentGomila LaurentGomila added this to the 2.2 milestone Feb 25, 2014

@Bromeon Bromeon self-assigned this Feb 25, 2014

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Feb 25, 2014

Member

Since we work with many overloads of the same function, we should avoid ambiguous pairs. For example, setParameter(int) and setParameter(float) would be ambiguous in certain circumstances. Even worse: setParameter(float[16]) and setParameter(float[9]) are identical (setParameter(float*)).

The strategy is to try to avoid these conflicts if possible (drop less important types if needed), and if not, we should think about a different API (postponed to SFML 3 if we have to break API compatibility).

Member

LaurentGomila commented Feb 25, 2014

Since we work with many overloads of the same function, we should avoid ambiguous pairs. For example, setParameter(int) and setParameter(float) would be ambiguous in certain circumstances. Even worse: setParameter(float[16]) and setParameter(float[9]) are identical (setParameter(float*)).

The strategy is to try to avoid these conflicts if possible (drop less important types if needed), and if not, we should think about a different API (postponed to SFML 3 if we have to break API compatibility).

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Feb 25, 2014

Member

Integers and booleans
Would there only be support for scalars or also for GLSL vectors of 2, 3 and 4 dimensions?
What about unsigned integers?

3x3 and 4x4 matrices
The question is whether matrices need to be passed as pointer to raw arrays, or if reusing sf::Transform would be okay. The 3x3 case would then be covered, but sf::Transform currently offers no constructor from 4x4 arrays (but a conversion to them). It is reasonable to provide a conversion from a float* pointer to an array rather than from 16 separate float values, also for consistency with getMatrix(). Either a constructor or a setMatrix() method might be appropriate.

One point to think about is whether the additional functionality in sf::Transform would also be useful in other scenarios, such as loading from OpenGL or different matrix classes.

Arrays
Since GLSL arrays can have arbitrary length, a signature such as

 void setParameterArray(const std::string& name, float* array, std::size_t size);

would be standing to reason. But again the question is, what types should be supported? GLSL arrays can be of any basic type (even vectors or matrices), so the number of overloads will quickly explode. Templates would be an option, but there might still be an internal case differentiation.

In general, for SFML 2 I suggest to stay as close to the current API as possible. Other function names such as setParameterInt might be worth considering for SFML 3, but currently they would look strange. Using setParameterArray for arrays on the other side shows a separation between GLSL basic types and arrays.

See also: GLSL Data Types

Member

Bromeon commented Feb 25, 2014

Integers and booleans
Would there only be support for scalars or also for GLSL vectors of 2, 3 and 4 dimensions?
What about unsigned integers?

3x3 and 4x4 matrices
The question is whether matrices need to be passed as pointer to raw arrays, or if reusing sf::Transform would be okay. The 3x3 case would then be covered, but sf::Transform currently offers no constructor from 4x4 arrays (but a conversion to them). It is reasonable to provide a conversion from a float* pointer to an array rather than from 16 separate float values, also for consistency with getMatrix(). Either a constructor or a setMatrix() method might be appropriate.

One point to think about is whether the additional functionality in sf::Transform would also be useful in other scenarios, such as loading from OpenGL or different matrix classes.

Arrays
Since GLSL arrays can have arbitrary length, a signature such as

 void setParameterArray(const std::string& name, float* array, std::size_t size);

would be standing to reason. But again the question is, what types should be supported? GLSL arrays can be of any basic type (even vectors or matrices), so the number of overloads will quickly explode. Templates would be an option, but there might still be an internal case differentiation.

In general, for SFML 2 I suggest to stay as close to the current API as possible. Other function names such as setParameterInt might be worth considering for SFML 3, but currently they would look strange. Using setParameterArray for arrays on the other side shows a separation between GLSL basic types and arrays.

See also: GLSL Data Types

@criptych

This comment has been minimized.

Show comment
Hide comment
@criptych

criptych Feb 25, 2014

"useful in other scenarios"
Support for 4x4 Transforms, even in 2D, would be handy for e.g. depth effects, especially in conjunction with Shaders: depth testing, parallax, DOF blur....

criptych commented Feb 25, 2014

"useful in other scenarios"
Support for 4x4 Transforms, even in 2D, would be handy for e.g. depth effects, especially in conjunction with Shaders: depth testing, parallax, DOF blur....

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Feb 26, 2014

Member

Then extending sf::Transform sounds reasonable. The new interaction with raw arrays would not only be applicable in other use cases, but it solves the ambiguity problem of float[9] and float[16].

What we should consider:

  • Are there common cases where 3x3 matrices are used in raw form (i.e. as an array of 9 elements)? Once we add a constructor/function to sf::Transform taking a single float* parameter for raw 4x4 matrices, we can't go back.

  • Should sf::Transform provide a constructor or a set function (as a counterpart to getMatrix()), or both?

    explicit        Transform(const float* matrix);
    void            setMatrix(const float* matrix);
    const float*    getMatrix() const; // exists already
Member

Bromeon commented Feb 26, 2014

Then extending sf::Transform sounds reasonable. The new interaction with raw arrays would not only be applicable in other use cases, but it solves the ambiguity problem of float[9] and float[16].

What we should consider:

  • Are there common cases where 3x3 matrices are used in raw form (i.e. as an array of 9 elements)? Once we add a constructor/function to sf::Transform taking a single float* parameter for raw 4x4 matrices, we can't go back.

  • Should sf::Transform provide a constructor or a set function (as a counterpart to getMatrix()), or both?

    explicit        Transform(const float* matrix);
    void            setMatrix(const float* matrix);
    const float*    getMatrix() const; // exists already
@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Feb 26, 2014

Member

Adding support for 3D to sf::Transform is not straight-forward. Passing a 4x4 matrix to its constructor would be useless, it internally never uses the extra matrix elements. Only getMatrix() would be able to access them, but then what's the point of using a sf::Transform if you just set and get the 16 elements ;)

To make it useful, we would have to add a transformPoint(sf::Vector3f) function for example. Or even transformPoint(sf::Vector3f, float w). But this starts to become too 3D-ish for a 2D graphics module ;)

We would also have to modify getInverse() and possibly other functions, to take in account the currently unused matrix elements.

It would also not help much regarding the initial issue: sf::Transform will always be passed as a 4x4 matrix to the shader. The only benefit of this modification would be to be able to define those 7 matrix elements that were previously not accessible with sf::Transform. But it wouldn't help with 3x3 matrices.

My conclusion is that we should stick to the initial problem (sf::Shader) and leave sf::Transform as it is. Simply using a more verbose syntax would solve all kind of issues:

Shader::setMatrix3x3(const float[9]);
Shader::setMatrix3x3(sf::Transform);

Shader::setMatrix4x4(const float[16]);
Shader::setMatrix4x4(sf::Transform);

Shader::setInteger(int);
Shader::setFloat(float);
Shader::setBool(bool);

Shader::setFloatArray(const float*, std::size_t);
Shader::setMatrix3x3Array(const Transform*, std::size_t);
...

We can define everything that we need without any problem.

Member

LaurentGomila commented Feb 26, 2014

Adding support for 3D to sf::Transform is not straight-forward. Passing a 4x4 matrix to its constructor would be useless, it internally never uses the extra matrix elements. Only getMatrix() would be able to access them, but then what's the point of using a sf::Transform if you just set and get the 16 elements ;)

To make it useful, we would have to add a transformPoint(sf::Vector3f) function for example. Or even transformPoint(sf::Vector3f, float w). But this starts to become too 3D-ish for a 2D graphics module ;)

We would also have to modify getInverse() and possibly other functions, to take in account the currently unused matrix elements.

It would also not help much regarding the initial issue: sf::Transform will always be passed as a 4x4 matrix to the shader. The only benefit of this modification would be to be able to define those 7 matrix elements that were previously not accessible with sf::Transform. But it wouldn't help with 3x3 matrices.

My conclusion is that we should stick to the initial problem (sf::Shader) and leave sf::Transform as it is. Simply using a more verbose syntax would solve all kind of issues:

Shader::setMatrix3x3(const float[9]);
Shader::setMatrix3x3(sf::Transform);

Shader::setMatrix4x4(const float[16]);
Shader::setMatrix4x4(sf::Transform);

Shader::setInteger(int);
Shader::setFloat(float);
Shader::setBool(bool);

Shader::setFloatArray(const float*, std::size_t);
Shader::setMatrix3x3Array(const Transform*, std::size_t);
...

We can define everything that we need without any problem.

@criptych

This comment has been minimized.

Show comment
Hide comment
@criptych

criptych Feb 26, 2014

Are there common cases where 3x3 matrices are used in raw form (i.e. as
an array of 9 elements)? Once we add a constructor/function to
sf::Transform taking a single float* parameter, we can't go back.
I can't think of any, offhand, but someone might.

Should sf::Transform provide a constructor or a set function (as a
counterpart to getMatrix()), or both?

explicit Transform(const float* matrix);
void setMatrix(const float* matrix);
const float* getMatrix() const; // exists already
I would say both, and implement the constructor in terms of the setter.
Maybe even an overload for operator[], although that could open the way for
other issues.

criptych commented Feb 26, 2014

Are there common cases where 3x3 matrices are used in raw form (i.e. as
an array of 9 elements)? Once we add a constructor/function to
sf::Transform taking a single float* parameter, we can't go back.
I can't think of any, offhand, but someone might.

Should sf::Transform provide a constructor or a set function (as a
counterpart to getMatrix()), or both?

explicit Transform(const float* matrix);
void setMatrix(const float* matrix);
const float* getMatrix() const; // exists already
I would say both, and implement the constructor in terms of the setter.
Maybe even an overload for operator[], although that could open the way for
other issues.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Feb 26, 2014

Member

You're right Laurent, I didn't think of that. So let's leave sf::Transform for the moment.

The more explicit syntax would be nice for a new API, but what should happen with the existing setParameter() functions? It's weird if for some types you have to call setParameter(float) and for others setInt(int), that's why I thought of a different approach for now. Or did you think of deprecating the current API?

What are the alternatives? One way is to provide small wrapper types, similar to the already existing Shader::CurrentTextureType nested structure.

Shader::Matrix3x3(const float* matrix);
Shader::Matrix4x4(const float* matrix);

Also, it would be nice if we didn't have to duplicate the whole set of functions just to support arrays. Just to show the required number of methods, considering potential future additions:

  • float, int, unsigned int, bool (double?): Overloads for 1-4 scalar parameters = 4*(4..5) = 16..20
  • float (also other types?): Overloads for 2D and 3D vectors = (1..5)*2 = 2..10
  • Color, Texture, Matrix3x3, Matrix4x4, Transform = 5
  • At the moment: 10 functions.
  • Support for GLSL basic types: 23..35 functions.
  • Everything with arrays: 46..70 functions.

That's a bit many in my opinion. A template for arrays would be nice, but it would require types that are unambiguous. Maybe one far day SFML also supports other GLSL compound types (structs), and at the latest then a more generic and type-inferring API is crucial.

Member

Bromeon commented Feb 26, 2014

You're right Laurent, I didn't think of that. So let's leave sf::Transform for the moment.

The more explicit syntax would be nice for a new API, but what should happen with the existing setParameter() functions? It's weird if for some types you have to call setParameter(float) and for others setInt(int), that's why I thought of a different approach for now. Or did you think of deprecating the current API?

What are the alternatives? One way is to provide small wrapper types, similar to the already existing Shader::CurrentTextureType nested structure.

Shader::Matrix3x3(const float* matrix);
Shader::Matrix4x4(const float* matrix);

Also, it would be nice if we didn't have to duplicate the whole set of functions just to support arrays. Just to show the required number of methods, considering potential future additions:

  • float, int, unsigned int, bool (double?): Overloads for 1-4 scalar parameters = 4*(4..5) = 16..20
  • float (also other types?): Overloads for 2D and 3D vectors = (1..5)*2 = 2..10
  • Color, Texture, Matrix3x3, Matrix4x4, Transform = 5
  • At the moment: 10 functions.
  • Support for GLSL basic types: 23..35 functions.
  • Everything with arrays: 46..70 functions.

That's a bit many in my opinion. A template for arrays would be nice, but it would require types that are unambiguous. Maybe one far day SFML also supports other GLSL compound types (structs), and at the latest then a more generic and type-inferring API is crucial.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Feb 26, 2014

Member

What are the alternatives?

I have no idea. I don't want to deprecate the current API, and I'm not sure that using wrapper types would be great here.

Member

LaurentGomila commented Feb 26, 2014

What are the alternatives?

I have no idea. I don't want to deprecate the current API, and I'm not sure that using wrapper types would be great here.

@germinolegrand

This comment has been minimized.

Show comment
Hide comment
@germinolegrand

germinolegrand Feb 26, 2014

C++ has some magic features to solve that, it's called template :-).

Le 26/02/2014 14:51, Bromeon a écrit :

You're right Laurent, I didn't think of that. So let's leave
|sf::Transform| for the moment.

The more explicit syntax would be nice for a new API, but what should
happen with the existing |setParameter()| functions? It's weird if for
some types you have to call |setParameter(float)| and for others
|setInt(int)|, that's why I thought of a different approach for now.
Or did you think of deprecating the current API?

What are the alternatives? One way is to provide small wrapper types,
similar to the already existing |Shader::CurrentTextureType| nested
structure.

Shader::Matrix3x3(const float* matrix);
Shader::Matrix4x4(const float* matrix);

Also, it would be nice if we didn't have to duplicate the whole set of
functions just to support arrays. Just to show the required number of
methods:

  • float, int, unsigned int, bool: Overloads for 1-4 scalar
    parameters = 4*4 = 16
  • float (also other types?): Overloads for 2D and 3D vectors =
    (1..4)*2 = 2..8
  • Color, Texture, Matrix3x3, Matrix4x4, Transform = 5
  • Totally: 23..27 functions.
  • Everything with arrays: 46..54 functions.

That's a bit many in my opinion. A template for arrays would be nice,
but it would require types that are unambiguous. Maybe one far day
SFML also supports other GLSL compound types (structs), and at the
latest then a more generic API is crucial.


Reply to this email directly or view it on GitHub
#538 (comment).

germinolegrand commented Feb 26, 2014

C++ has some magic features to solve that, it's called template :-).

Le 26/02/2014 14:51, Bromeon a écrit :

You're right Laurent, I didn't think of that. So let's leave
|sf::Transform| for the moment.

The more explicit syntax would be nice for a new API, but what should
happen with the existing |setParameter()| functions? It's weird if for
some types you have to call |setParameter(float)| and for others
|setInt(int)|, that's why I thought of a different approach for now.
Or did you think of deprecating the current API?

What are the alternatives? One way is to provide small wrapper types,
similar to the already existing |Shader::CurrentTextureType| nested
structure.

Shader::Matrix3x3(const float* matrix);
Shader::Matrix4x4(const float* matrix);

Also, it would be nice if we didn't have to duplicate the whole set of
functions just to support arrays. Just to show the required number of
methods:

  • float, int, unsigned int, bool: Overloads for 1-4 scalar
    parameters = 4*4 = 16
  • float (also other types?): Overloads for 2D and 3D vectors =
    (1..4)*2 = 2..8
  • Color, Texture, Matrix3x3, Matrix4x4, Transform = 5
  • Totally: 23..27 functions.
  • Everything with arrays: 46..54 functions.

That's a bit many in my opinion. A template for arrays would be nice,
but it would require types that are unambiguous. Maybe one far day
SFML also supports other GLSL compound types (structs), and at the
latest then a more generic API is crucial.


Reply to this email directly or view it on GitHub
#538 (comment).

@criptych

This comment has been minimized.

Show comment
Hide comment
@criptych

criptych Feb 26, 2014

what's the point of using a sf::Transform if you just set and get the 16 elements ;)
Could be very handy for drawing, via RenderStates or Shaders. I've already
been thinking of adding 3D translation (at least) to my local copy, before
this topic came up, for the purposes I mentioned.

To be useful, we would have to add a transformPoint(sf::Vector3f) function for example. Or even transformPoint(sf::Vector3f, float w). But this starts to become too 3D-ish for a 2D graphics module ;)

We would also have to modify getInverse() and possibly other functions, to take in account the currently unused matrix elements.
Perhaps it could be split off into another module, or an extension? 4x4
inverse isn't terribly difficult, but more expensive than 3x3 of course.

As you say, though, we should probably leave this ticket to the Shader
issue, and, if you don't have any objections, open a new one to discuss
Transforms.

criptych commented Feb 26, 2014

what's the point of using a sf::Transform if you just set and get the 16 elements ;)
Could be very handy for drawing, via RenderStates or Shaders. I've already
been thinking of adding 3D translation (at least) to my local copy, before
this topic came up, for the purposes I mentioned.

To be useful, we would have to add a transformPoint(sf::Vector3f) function for example. Or even transformPoint(sf::Vector3f, float w). But this starts to become too 3D-ish for a 2D graphics module ;)

We would also have to modify getInverse() and possibly other functions, to take in account the currently unused matrix elements.
Perhaps it could be split off into another module, or an extension? 4x4
inverse isn't terribly difficult, but more expensive than 3x3 of course.

As you say, though, we should probably leave this ticket to the Shader
issue, and, if you don't have any objections, open a new one to discuss
Transforms.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Feb 26, 2014

Member

I have no idea. I don't want to deprecate the current API, and I'm not sure that using wrapper types would be great here.

I see the following options:

  1. Deprecate setParameter and introduce non-overloaded methods such as setFloat
  2. Introduce additional non-overloaded methods such as setInt, setMatrix3x3 -> inconsistent API
  3. Only introduce non-overloaded methods where there are ambiguities (Matrix3x3, Matrix4x3) -> partially inconsistent API
  4. Overload setParameter further, use wrappers à la Shader::Matrix3x3 for ambiguous types
  5. Use other parameter to differ between overloads, e.g. setParameter(const float* matrix, size_t dimension) or even different types (tags) setParameter(const float* matrix, Matrix3x3)

The disadvantages of option 4 are slightly more verbose syntax for matrices and the possibility to pass wrong types when being careless. The advantage is conformance with the current API and type inference (which would allow the use of templates and decrease the number of required methods tremendously for GLSL arrays/structs).

Other ideas and arguments are welcome!

As you say, though, we should probably leave this ticket to the Shader issue, and, if you don't have any objections, open a new one to discuss Transforms.

Please use the forum for discussions, issues are for concrete feature requests and confirmed bugs. But in general, features asking for 3D support are unlikely to be accepted ;)

Member

Bromeon commented Feb 26, 2014

I have no idea. I don't want to deprecate the current API, and I'm not sure that using wrapper types would be great here.

I see the following options:

  1. Deprecate setParameter and introduce non-overloaded methods such as setFloat
  2. Introduce additional non-overloaded methods such as setInt, setMatrix3x3 -> inconsistent API
  3. Only introduce non-overloaded methods where there are ambiguities (Matrix3x3, Matrix4x3) -> partially inconsistent API
  4. Overload setParameter further, use wrappers à la Shader::Matrix3x3 for ambiguous types
  5. Use other parameter to differ between overloads, e.g. setParameter(const float* matrix, size_t dimension) or even different types (tags) setParameter(const float* matrix, Matrix3x3)

The disadvantages of option 4 are slightly more verbose syntax for matrices and the possibility to pass wrong types when being careless. The advantage is conformance with the current API and type inference (which would allow the use of templates and decrease the number of required methods tremendously for GLSL arrays/structs).

Other ideas and arguments are welcome!

As you say, though, we should probably leave this ticket to the Shader issue, and, if you don't have any objections, open a new one to discuss Transforms.

Please use the forum for discussions, issues are for concrete feature requests and confirmed bugs. But in general, features asking for 3D support are unlikely to be accepted ;)

@Lee-R

This comment has been minimized.

Show comment
Hide comment
@Lee-R

Lee-R Feb 27, 2014

'ello.

Regarding the ambiguous overloads. It seems as though everyone is forgetting about references. Check the program output at the bottom of the following page.

https://ideone.com/VI9Rgw

Lee-R commented Feb 27, 2014

'ello.

Regarding the ambiguous overloads. It seems as though everyone is forgetting about references. Check the program output at the bottom of the following page.

https://ideone.com/VI9Rgw

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Feb 27, 2014

Member

That's right, however the problem with references and pointers to arrays is that they're not very flexible: They require a static array of exactly that size. If the user has another data structure (std::vector, std::array, pointer to dynamic memory, other matrix class), he can't directly pass the data to the shader.

Considering that raw arrays are completely superseded by the std::array class in C++11, that's a rather big restriction...

Member

Bromeon commented Feb 27, 2014

That's right, however the problem with references and pointers to arrays is that they're not very flexible: They require a static array of exactly that size. If the user has another data structure (std::vector, std::array, pointer to dynamic memory, other matrix class), he can't directly pass the data to the shader.

Considering that raw arrays are completely superseded by the std::array class in C++11, that's a rather big restriction...

@Lee-R

This comment has been minimized.

Show comment
Hide comment
@Lee-R

Lee-R Feb 27, 2014

There was a pointer overload that covers the case you're describing. I did forget one this one though:

void setParameter(const float*, int size, int count) {
    std::cout << "float[" << size << "][" << count << "]\n";
}

Anyway, this is all completely irrelevant if the previous discussion on 'float[9] float[16]' style API isn't being considered any longer.

Lee-R commented Feb 27, 2014

There was a pointer overload that covers the case you're describing. I did forget one this one though:

void setParameter(const float*, int size, int count) {
    std::cout << "float[" << size << "][" << count << "]\n";
}

Anyway, this is all completely irrelevant if the previous discussion on 'float[9] float[16]' style API isn't being considered any longer.

@criptych

This comment has been minimized.

Show comment
Hide comment
@criptych

criptych Feb 28, 2014

features asking for 3D support are unlikely to be accepted ;)

Fair enough, but at least having access to the remaining fields would make it flexible enough for users to "roll their own."

criptych commented Feb 28, 2014

features asking for 3D support are unlikely to be accepted ;)

Fair enough, but at least having access to the remaining fields would make it flexible enough for users to "roll their own."

@kenpower

This comment has been minimized.

Show comment
Hide comment
@kenpower

kenpower Mar 10, 2014

if, for whatever reason, setParameter for 4x4Matrices are not going to be supported in sf::Shader in v2.2, then at least can we have

int sf::Shader::getProgram(){return m_shaderProgram;} 

so we can and roll our own using

glUniformMatrix4fv();

?

kenpower commented Mar 10, 2014

if, for whatever reason, setParameter for 4x4Matrices are not going to be supported in sf::Shader in v2.2, then at least can we have

int sf::Shader::getProgram(){return m_shaderProgram;} 

so we can and roll our own using

glUniformMatrix4fv();

?

@kenpower

This comment has been minimized.

Show comment
Hide comment
@kenpower

kenpower Mar 10, 2014

For matrices, how about specifying the matrix size as a parameter (as in Bromeon's 5th option above);

setParameter(const float* matrix, size_t dimension);

this will be the way arrays will be done eventually, so why not similar behaviour for matrices. It will cut down on the number of overloads.

kenpower commented Mar 10, 2014

For matrices, how about specifying the matrix size as a parameter (as in Bromeon's 5th option above);

setParameter(const float* matrix, size_t dimension);

this will be the way arrays will be done eventually, so why not similar behaviour for matrices. It will cut down on the number of overloads.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Mar 23, 2014

Member

The problem with

 void setParameter(const float* matrix, std::size_t dimension);

is that the size_t doesn't make it very clear that there are only two valid values (unless we allow also other dimensions than 3 and 4, and possibly even specify both dimensions, aka matNxM in GLSL).

The question is also, what would an ideal API (in SFML 3) look like? It should be a good compromise between expressiveness, type-safety and genericity. I also think setParameter should be renamed, at least to something clearer like setUniform or directly setFloat & Co. But the latter has again the problem that it can't be used in generic code (and only to support arrays every function would need to be duplicated, which is far from ideal).

Member

Bromeon commented Mar 23, 2014

The problem with

 void setParameter(const float* matrix, std::size_t dimension);

is that the size_t doesn't make it very clear that there are only two valid values (unless we allow also other dimensions than 3 and 4, and possibly even specify both dimensions, aka matNxM in GLSL).

The question is also, what would an ideal API (in SFML 3) look like? It should be a good compromise between expressiveness, type-safety and genericity. I also think setParameter should be renamed, at least to something clearer like setUniform or directly setFloat & Co. But the latter has again the problem that it can't be used in generic code (and only to support arrays every function would need to be duplicated, which is far from ideal).

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Mar 29, 2014

Member

I somehow have the feeling that an integration into SFML 2.2 will become difficult like this :D

Maybe this should be discussed in the forum, as it's more frequented than this tracker?

Member

Bromeon commented Mar 29, 2014

I somehow have the feeling that an integration into SFML 2.2 will become difficult like this :D

Maybe this should be discussed in the forum, as it's more frequented than this tracker?

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Mar 29, 2014

Member

Yep, don't hesitate to open a thread on the forum if you need more feedback.

Member

LaurentGomila commented Mar 29, 2014

Yep, don't hesitate to open a thread on the forum if you need more feedback.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Mar 29, 2014

Member

Ok. Further discussion on the topic please here:
http://en.sfml-dev.org/forums/index.php?topic=14802

Member

Bromeon commented Mar 29, 2014

Ok. Further discussion on the topic please here:
http://en.sfml-dev.org/forums/index.php?topic=14802

@Bromeon Bromeon modified the milestones: 2.x, 2.2 Apr 6, 2014

@eXpl0it3r eXpl0it3r removed this from the 2.x milestone Nov 13, 2014

@LaurentGomila LaurentGomila removed this from the 2.x milestone Nov 13, 2014

@binary1248 binary1248 added this to the 2.4 milestone Mar 29, 2015

@Bromeon Bromeon self-assigned this Jul 14, 2015

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Nov 2, 2015

Member

Since #983 is merged, this can be closed, right?

Member

LaurentGomila commented Nov 2, 2015

Since #983 is merged, this can be closed, right?

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Nov 2, 2015

Member

Yes, totally.

Member

Bromeon commented Nov 2, 2015

Yes, totally.

@Bromeon Bromeon closed this Nov 2, 2015

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