oglplus::Context initialization is misleading

[vif]

To the best of my knowledge:

• oglplus does not handle context creation
• all of the oglplus::Context functions are static

The usage of oglplus::Context as seen in the examples, leads to a feeling of "locality", and confusion as to when to initialize the oglplus::Context object.

I feel that calling the static functions directly would be a more accurate representation of what is actually happening behind the scenes.

Also initializing a oglplus::Context locally in Visual Studio, and calling it's different members (at least the ones I have tried thus far) leads to Visual Studio complaining about an "unused local variable" upon compilation.

[matus-chochlik]

oglplus does not handle context creation

This is true and intentional (since GL context creation is platform-specific) and the documentation clearly says so.

all of the oglplus::Context functions are static

Also true and natural, and there is nothing preventing calling the Context functions as static functions. The reason why I in the examples do:

oglplus::Context gl;
gl.Whatever();

is purely a matter of my 'taste' because I like the code visually looking this way. There isn't however anything preventing you from doing this, if you dislike the above:

typedef oglplus::Context GL;
GL::Whatever();

As for the MSVC warning, there is nothing in the C++ standard saying that calling static member functions through the dot operator of an object is "wrong" or something that the compiler should be issuing warnings about. IMHO what the compiler should do is to silently optimize the code, so that the this variable is not used or even instantiated in most cases.

[vif]

As somebody who is starting out learning OpengGL this is quite confusing. In my opinion, any OpenGL functionality that does something to an object should be encapsulated by a class, while something that changes state should be encapsulated by a namespace. As an example, consider Bindings and Unbindings. With the encapsulation/naming as it currently is, one would expect a Bind to just 'activate' the object, rather than changing a state that indicates that the object is the active one. i.e. with the current encapsulation/naming a naive user is led to believe you can have multiple (of the same) object bound at the same time. I think having a Bind function that accepts a pointer to a object that it binds would be more accurate, since we are doing something with the object reference, rather than doing something to the object.

I am lazy and I don't want to keep track of GLint references, and call glGetError after every function call, so this library is awesome, but I think what I mentioned is something to consider. Thank you.

[matus-chochlik]

I see your points, however with the Context class implemented as it is you can treat it both ways as a namespace: oglplus::Context::ClearColor(...) and as a monostate object: gl.ClearColor(...) so you have two options instead of one.

As for binding, I'm planning an alternative API (that will be part of Context) doing something similar as you suggest but I have to think it through so that it 'works' correctly also with the DSA objects and the Bound<Object> template.

[matus-chochlik]

I've added an alternative API for object binding, so you can now to the following:

typedef Context GL;
Program prog;
Texture tex;

// binding / using
GL::Bind(Texture::Target::_2D, tex);
GL::Use(prog);

// getting reference to the currently bound/used object
GL::Current(Texture::Target::_2D).WrapS(TextureWrap::Repeat);
GL::Current<Program>().Validate();

// binding and getting the reference (more efficient)
// NOTE that you don't have to specify the target for each
// of the texture functions now.
GL::Bound(Texture::Target::_2D, tex)
    .WrapS(TextureWrap::Repeat)
    .WrapT(TextureWrap::Repeat)
    .MinFilter(...)
    .MagFilter(...)
    .Image2D(...)
    .GenerateMipmap();

Many examples have also been updated to show this feature.

[vif]

Awesome! Going to incorporate this into my code.

[matus-chochlik]

OK, but please be aware that this is a new feature and may yet possibly be subject to some minor changes in the future.

[vif]

Hmm, I think that the bind calls should be on each object. i.e. so you can do something like oglplus::VertexArray::Bind(foo); which would be consistent with the unbind functionality oglplus::VertexArray::Unbind();

[matus-chochlik]

Doing oglplus::VertexArray::Bind(foo) is IMHO redundant, since you already are saying that you are binding a VAO from the type of foo, and this would get even more awkward with textures & co: oglplus::Texture::Bind(Texture::Target::_2D, tex) where you would specify that you are binding a texture in three different ways in a single call.

Regarding the Unbind function I plan some changes to that too, because it actually is not really consistent. On some types of objects (Texture, Framebuffer, TransformFeedback) you can bind a default (= zero). On other types (Buffer, Renderbuffer,...) binding zero means 'binding nothing' / unbinding. I have some ideas but I have to think them through.

[vif]

Well, talking about VertexArray, it is my understand that in the GPU memory there is a register (or something similar) which holds the reference ID of the currently bound VertexArray object. For me at the very least, saying oglplus::VertexArray::Bind makes sense because for me the VertexArray register is the static object that oglplus::VertexArray is abstracting, and telling that object to Bind or UnBind makes sense.

[matus-chochlik]

Yes, but the register holding the handle to the current vertex array (or most other bindable objects) is part of the currently bound GL context. So this is where the binding is done in the new API. Individual objects already have Bind member functions.

As for the solution for the BindDefault/Unbind functions I'm currently in favour of doing the following:

There already are the DefaultTransformFeedback and DefaultFramebuffer classes and in addition to those implement a NoBuffer (or NilBuffer), NoRenderbuffer, NoVertexArray, DefaultTexture, etc. all of which would have a Bind(...) member function. So the binding of the default and named object or the nil objects (unbinding) would all be done in the same way:

Context gl;
DefaultFramebuffer dfb;
Framebuffer fbo;
gl.Bind(fbo); // bind named framebuffer object
...
gl.Bind(dfb); // bind default framebuffer

NoVertexArray nil_vao;
VertexArray vao;
gl.Bind(vao); // bind a named VAO
...
gl.Bind(nil_vao); // bind nil VAO (= unbind)

[vif]

That sounds much more transparent (so long as the defaults are easy to find).

The issue with the singular "smart" bind, is that if you mess up the types by doing something like:

Context gl;
NoVertexArray dfb;  // <- WRONG TYPE
Framebuffer fbo;
gl.Bind(fbo); // bind named framebuffer object
...
gl.Bind(dfb); // bind default framebuffer

is that you basically have no idea what is being bound, sure you can check the types manually, or debug and try to guess what's wrong but I feel that is gets rid of a lot of type safety.

vs if you do something like this

Context gl;
NoVertexArray dfb;  // <- WRONG TYPE
Framebuffer fbo;
gl.Bind<Framebuffer>(fbo); // bind named framebuffer object
...
gl.Bind<Framebuffer>(dfb); // bind default framebuffer

or something explicit, even though it is slightly redundant, the compiler should be able to catch the error, and you will see that you are doing something wrong.

[matus-chochlik]

Actually the new API allows you to do exactly that, i.e. if you wish you can do:

gl.Bind<Framebuffer>(x);

and this would fail to compile if x was not a Framebuffer.

[vif]

Well that's awesome then! I wish there was a way to force explicit types though, or make the implicit bind explicit (e.g. separate function called SmartBind or something), but that's a personal preference. On Apr 16, 2014 12:58 PM, "Matus Chochlik" notifications@github.com wrote:

Actually the new API allows you to do exactly that, i.e. if you wish you can do:

gl.Bind(x);

and this would fail to compile if x was not a Framebuffer.

— Reply to this email directly or view it on GitHubhttps://github.com/matus-chochlik/oglplus/issues/58#issuecomment-40624033 .