Skip to content
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

Add methods to get and set shader attribute locations #999

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

criptych
Copy link

@criptych criptych commented Nov 3, 2015

Implements #987 (forum post).

Fixed attribute locations may be assigned before loading the shader source, and/or the locations assigned by the shader compiler may be retrieved afterward.

Usage:

    // Vertex shader
    #version 330
    uniform mat4 projection;
    uniform mat4 modelview;
    in vec2 position;
    in vec2 texCoord;
    out vec2 v_position;
    out vec2 v_texCoord;
    void main() {
        gl_Position = projection * modelview * vec4(position, 0, 1);
        v_position = position;
        v_texCoord = texCoord;
    }
    // Assign locations before loading
    sf::Shader myShader;
    myShader.setAttribLocation("position", 0);
    myShader.setAttribLocation("texCoord", 1);
    myShader.loadFromFile("shaders/tilemap.vert", "shaders/tilemap.frag");
    glEnableVertexAttribArray(0);
    glEnableVertexAttribArray(1);
    glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 16, 0);
    glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 16, 8);
    // Retrieve locations after loading
    sf::Shader myShader;
    myShader.loadFromFile("shaders/tilemap.vert", "shaders/tilemap.frag");
    int loc_position = myShader.getAttribLocation("position");
    int loc_texCoord = myShader.getAttribLocation("texCoord");
    glEnableVertexAttribArray(loc_position);
    glEnableVertexAttribArray(loc_texCoord);
    glVertexAttribPointer(loc_position, 2, GL_FLOAT, GL_FALSE, 16, 0);
    glVertexAttribPointer(loc_texCoord, 2, GL_FLOAT, GL_FALSE, 16, 8);

@binary1248
Copy link
Member

This is a start, but I was already considering an API that allowed newer GLSL to be used with sfml-graphics rendering as well. Your code only benefits users who use sf::Shader as a lightweight GL object wrapper. It would be nicer if this feature could benefit sfml-graphics users (i.e. those who use sf::RenderTargets for drawing) allowing them to e.g. make use of newer GLSL versions in combination with their usual rendering.

@LaurentGomila
Copy link
Member

This is a start, but I was already considering an API that allowed newer GLSL to be used with sfml-graphics rendering as well.

Can you explain what you have in mind exactly? How could this new stuff be combined with sfml-graphics, and how would you modify the API to allow that?

@binary1248
Copy link
Member

Just like how you can bind a GLSL sampler to sf::Shader::CurrentTexture, you should be able to bind your position/texture/colour attributes to fixed attribute enums that a sf::RenderTarget can bind the vertex array to in order to render sf::Drawable objects. Something like sf::Shader::AttributePosition, sf::Shader::AttributeTexCoord and sf::Shader::AttributeColor. All that matters is that there is a common convention that is used for binding between the user's own GLSL attributes and the sf::RenderTarget. There would also be an overload for arbitrary binding as in @criptych's PR.

@criptych
Copy link
Author

criptych commented Nov 9, 2015

Actually, I'd thought about something like this too, but wasn't sure how to go about it. The thing is, SFML currently uses standard vertex attributes through glVertexPointer et al., and according to the OpenGL 2.1 docs, "Applications are not allowed to bind any of the standard OpenGL vertex attributes using [glBindAttribLocation], as they are bound automatically when needed." (Notwithstanding, apparently some cards do alias the standard attributes to known locations.) SFML itself would likely have to switch to using shaders internally in order to properly support this.

On a side note, I also found the following in the docs for 3.0+: "the location of the attribute specified in the shader's source [with layout(location)] takes precedence". My implementation as written doesn't account for this if an attribute location was set by the user (it assumes the set location is definitive), so I'll need to rethink that as well....

@binary1248
Copy link
Member

The thing is, SFML currently uses standard vertex attributes through glVertexPointer et al., and according to the OpenGL 2.1 docs, "Applications are not allowed to bind any of the standard OpenGL vertex attributes using [glBindAttribLocation], as they are bound automatically when needed." (Notwithstanding, apparently some cards do alias the standard attributes to known locations.) SFML itself would likely have to switch to using shaders internally in order to properly support this.

Not necessarily. There are 3 cases:

  • No shader bound as part of a RenderState -> Do things as usual.
  • Shader bound as part of a RenderState without explicit binding -> Do things as usual.
  • Shader bound as part of a RenderState with explicit binding -> Use glBindAttribLocation instead.

Causes `getAttribLocation` to retrieve the *actual* attribute location,
even if it has been overridden in shader source with `layout(location)`.
@criptych
Copy link
Author

criptych commented Nov 9, 2015

Updated PR; now allows for locations overridden in the shader.

Not necessarily. There are 3 cases:

  • No shader bound as part of a RenderState -> Do things as usual.
  • Shader bound as part of a RenderState without explicit binding -> Do things as usual.
  • Shader bound as part of a RenderState with explicit binding -> Use glBindAttribLocation instead.

Of course, sorry. I was only thinking about case 3. :)

Unfortunately, though, I still don't really see a way to do it the way you're suggesting:

you should be able to bind your position/texture/colour attributes to fixed attribute enums that a sf::RenderTarget can bind the vertex array to in order to render sf::Drawable objects

The standard attributes gl_Vertex etc. can only be used one-to-one with glVertexPointer etc. The attributes can't be bound (mentioned above) and the functions don't take a location parameter. So unless I'm misunderstanding, either you have to use the standard names in your shader (case 2); or you have to use custom names and the generic glVertexAttribPointer with a location.

I'll keep looking at this, though, and I'm open to suggestions.

@eXpl0it3r
Copy link
Member

@binary1248 will this be obsolete with the legacy shader changes you have in the pipeline?

@binary1248
Copy link
Member

Possibly.

@Bromeon
Copy link
Member

Bromeon commented Dec 7, 2019

@binary1248 I assume the situation has not changed since 2018?

@binary1248
Copy link
Member

@Bromeon It hasn't.

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

Successfully merging this pull request may close these issues.

None yet

6 participants