Overv / Open.GL

The source code and content of Open.GL.
http://open.gl/
1.07k stars 259 forks source link

[Suggestion] Format GLSL code so that the shader macro you added is used properly #60

Closed jungletek closed 7 years ago

jungletek commented 7 years ago

A while ago you modified the example code so that the GLSL code went from looking like this:

const GLchar* vertexSource =
    "#version 150 core\n"
    "in vec2 position;"
    "void main()"
    "{"
    "   gl_Position = vec4(position, 0.0, 1.0);"
    "}";

to this:

const GLchar* vertexSource = R"glsl(
    #version 150 core
    in vec2 position;
    void main()
    {
        gl_Position = vec4(position, 0.0, 1.0);
    }
)glsl";

Okay, certainly an improvement, easier to type, etc. But at some point you also added that 'Shader Macro' #define, yet for whatever reason you didn't modify the format of the shader code such that said define was actually being used. I hope you'll agree that the following format is superior to the prior two and will update the example source files appropriately, as I found it more readable and easier to understand than the previous two valid but unwieldy formattings:

const GLchar* vertexSource = GLSL(
    in vec2 position;
    void main()
    {
        gl_Position = vec4( position, 0.0, 1.0 );
    }
);

As you can see, we can omit the #version line, and the syntax is even simpler and easier to remember(!). It may not be desirable for some edge-cases where you want to mix GLSL versions (is that even a good idea?), but for newbies it cleans things up a bit, IMHO.

Additionally, and not really related: I've gone through your tutorials and found them helpful, and I used SDL to handle window management, the GL context, etc., rather than SFML/GLFW. Accordingly, I have working, compilable versions using SDL2 and SDL2_image that I'd be happy to send your way if you wanted to add them to the repo. There may be a few slightly-differently named vars, and my preferred formatting is slightly different, but you should be able to see the relevant SDL-bits with a quick diff if you were so inclined. Let me know if you're interested, or if it's trivial enough that you don't want to bother.

Overv commented 7 years ago

Yeah, the macro was not supposed to be in there anymore since I switched everything over to C++11 raw string literal syntax. I think it's better to avoid macros in C++ and make it explicit that the shader code is using GLSL version 1.50 core.

jungletek commented 7 years ago

Fair enough, it's your project, and I understand the logic behind your decision.

Thanks for getting to my tickets quickly. Take care :)