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

Versioned shader doesn't work when setup from source #6235

Open
memo opened this Issue Feb 5, 2019 · 6 comments

Comments

Projects
None yet
3 participants
@memo
Copy link
Member

memo commented Feb 5, 2019

I posted this on the forum, but I now think it's a bug, so I'm filing here.
https://forum.openframeworks.cc/t/shader-setupshaderfromsource-not-working/29432/4

If I load my #versioned 150 shaders from file, it works fine. If I setup from source via stringify or R", it compiles & links fine, but I see nothing. I dump the strings to the console to compare, and they look fine. I couldn't track down the issue. (Actually it seems to be displaying something, maybe it's related to the default uniforms like modelViewProjectionMatrix etc not being passed correctly or something?)

I did try:

  1. manually adding shader.setUniformMatrix4f("modelViewProjectionMatrix", ofGetCurrentMatrix(OF_MATRIX_PROJECTION) * ofGetCurrentMatrix(OF_MATRIX_MODELVIEW));
    but this made no difference at all to either shader.
  2. renaming the uniform modelViewProjectionMatrix to mvp and setting it manually to ofGetCurrentMatrix(OF_MATRIX_PROJECTION) * ofGetCurrentMatrix(OF_MATRIX_MODELVIEW)). In this case shader0 (loaded from file) still showed the correct content, but was offset by half (excluding the plane's position. is this a bug in core or user error on my side?) However shader1 is still not showing anything.

This is openFrameworks 0.10.1 on ubuntu 18.04.1 with Qt Creator 4.6.2

// vert shader
#version 150

uniform mat4 modelViewProjectionMatrix;
in vec4 position;
in vec2 texcoord;
out vec2 varyingtexcoord;

void main(){
    varyingtexcoord = texcoord;
    gl_Position = modelViewProjectionMatrix * position;
}
// frag shader
#version 150

uniform sampler2DRect tex0;
in vec2 varyingtexcoord;
out vec4 outputColor;

void main()
{
    outputColor = texture(tex0, varyingtexcoord);
}
// main.cpp
#include "ofMain.h"

#define STRINGIFY(A) string("#version 150\n")+#A

class ofApp : public ofBaseApp{
public:
    ofShader shader0;   // loaded from file <-- WORKS!
    ofShader shader1;   // setup from (identical) source <-- COMPILES, BUT NO OUTPUT!
    ofVideoGrabber cam;
    ofPlanePrimitive quad;

    //--------------------------------------------------------------
    // vertex shader source
#ifdef STRINGIFY
    string vertSource = STRINGIFY(
            uniform mat4 modelViewProjectionMatrix;
            in vec4 position;
            in vec2 texcoord;
            out vec2 varyingtexcoord;

            void main(){
                varyingtexcoord = texcoord;
                gl_Position = modelViewProjectionMatrix * position;
            }
     );

    //--------------------------------------------------------------
    // fragment shader source
    string fragSource = STRINGIFY(
            uniform sampler2DRect tex0;
            in vec2 varyingtexcoord;
            out vec4 outputColor;

            void main()
            {
                outputColor = texture(tex0, varyingtexcoord);
            }

     );
#else
    string vertSource = R"(
            #version 150

            uniform mat4 modelViewProjectionMatrix;
            in vec4 position;
            in vec2 texcoord;
            out vec2 varyingtexcoord;

            void main(){
                varyingtexcoord = texcoord;
                gl_Position = modelViewProjectionMatrix * position;
            }

      )";

    //--------------------------------------------------------------
    // fragment shader source
    string fragSource = R"(
            #version 150

            uniform sampler2DRect tex0;
            in vec2 varyingtexcoord;
            out vec4 outputColor;

            void main()
            {
                outputColor = texture(tex0, varyingtexcoord);
            }
     )";
#endif


    //--------------------------------------------------------------
    void setup(){
        ofBackground(50);
        cam.setup(320, 240);

        quad.set(cam.getWidth(), -cam.getHeight(), 2, 2); // flip vertically
        quad.setPosition(cam.getWidth()/2, cam.getHeight()/2, 0);
        quad.mapTexCoords(1, 1, cam.getWidth(), cam.getHeight());

        // setup shader0 from files
        shader0.load("pass");

        // setup shader1 from source
        ofLogNotice() << "//=== vertSource \n" << vertSource;
        ofLogNotice() << "//=== fragSource \n" << fragSource;
        shader1.setupShaderFromSource(GL_VERTEX_SHADER, vertSource);
        shader1.setupShaderFromSource(GL_FRAGMENT_SHADER, fragSource);
        shader1.linkProgram();
    }

    //--------------------------------------------------------------
    void drawShader(ofShader& s) {
        s.begin();
        s.setUniformTexture("tex0", cam.getTexture(), 0);
        quad.draw();
        s.end();
    }

    //--------------------------------------------------------------
    void draw(){
        cam.update();

        drawShader(shader0);

        ofPushMatrix();
        ofTranslate(0, cam.getHeight());
        drawShader(shader1);
        ofPopMatrix();
    }
};

int main( ){
    ofGLFWWindowSettings settings;
    settings.setGLVersion(3, 2);
    settings.setSize(1024, 768);
    ofCreateWindow(settings);
    ofRunApp(new ofApp());
}
@arturoc

This comment has been minimized.

Copy link
Member

arturoc commented Feb 5, 2019

when loading shaders from source in gl 3+ there's an extra step that needs to be done to bind the default attribute locations by calling bindDefaults()

This is done so in case you don't want to use the default OF attribtues and uniforms for position, normal, color, matrices... you can set your own. when loading we do it by default but if you are using the more manual setupShaderFromSource we understand that you want to explicitly indicate every step.

This could be a parameter to linkProgram though or even a second linkProgram method like linkProgramNoDefaults()

@memo

This comment has been minimized.

Copy link
Member Author

memo commented Feb 5, 2019

when loading shaders from source in gl 3+ there's an extra step that needs to be done to bind the default attribute locations by calling bindDefaults()

doh, I spend almost a day trying to debug a problem, turns out it was because the shader was setupFromSource :)

This is done so in case you don't want to use the default OF attribtues and uniforms for position, normal, color, matrices... you can set your own. when loading we do it by default but if you are using the

more manual setupShaderFromSource we understand that you want to explicitly indicate every step.
yes this makes a lot of sense.

This could be a parameter to linkProgram though or even a second linkProgram method like linkProgramNoDefaults()

yes making this more transparent could help. an argument to linkProgram sounds good.
One question though, is this a property of the shader? It could also be an argument to shader::bind() as well? (or bindWithDefaults()) etc?

@memo memo closed this Feb 6, 2019

@prabir78

This comment has been minimized.

Copy link

prabir78 commented Feb 6, 2019

Hii

@arturoc

This comment has been minimized.

Copy link
Member

arturoc commented Feb 10, 2019

@memo sorry didn't saw your last message before. This is something that needs to happen before linking so it couldn't be an argument to bind.

I'll look into deprecating bindDefaults and add a second version of link without defaults or a parameter...

going to reopen this so i can keep track of the issue

@arturoc arturoc reopened this Feb 10, 2019

@memo

This comment has been minimized.

Copy link
Member Author

memo commented Feb 16, 2019

This is something that needs to happen before linking so it couldn't be an argument to bind.

ah of course, because I guess you need to prefix headers with relevant uniforms etc.?

@arturoc

This comment has been minimized.

Copy link
Member

arturoc commented Feb 16, 2019

it's just how opengl works, attributes get a fixed location when linking. if you don't provide one before, at link time the linker assigns each attribute a location that can't be changed afterwards

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