Remove unnecessary checks for API handles about fastuidraw HOT 10 CLOSED

Extra checks are not needed for zero API handles in a function like the destructor for the class "fastuidraw::gl::Program".

from fastuidraw.

Hi!

Thanks for the heads up.

The extra check is not for the sake of the GP API really. The issue is that if a GLSL program is set but never assembled, then there is a chance there was no GL context. In truth, as noted in the TODO, for those GL object things which do GL things in their dtor, the thing that needs to be done is that the GL calls need to be sent to a "GL worker" which issues GL calls with the guarantee that a GL context(of the correct GL shader group) is current. However, I freely admit, that getting that done is at the bottom of the long TODO list I still have.

from fastuidraw.

Thanks for your constructive feedback.

But it seems that I understand the technical constraints you are trying to explain a bit not good enough at the moment. How do you think about information like "A value of 0 for program will be silently ignored." in the documentation for the function "glDeleteProgram"?

Would it be better to keep this issue open until it will be fixed anyhow?

from fastuidraw.

The issue should be closed.

The check for m_name is ~Program (and for that manner ~Shader) is to handle the case where the Program (or Shader) object was created, but they never made it getting the underlying GL calls done on them. The Program and Shader object types can be constructed WITHOUT a GL context. The creation of the GL object referenced by m_name is delayed until it is needed. The point of the check is a (not very robust) check if a GL context is available.

The real issue is that dtor for objects that call GL things need a GL context, and the GL context worker has not yet been made.

from fastuidraw.

The point of the check is a (not very robust) check if a GL context is available.

I find that this special check should be omitted if a description for an application programming interface like OpenGL gives you such information.

The Program and Shader object types can be constructed WITHOUT a GL context.

Will the corresponding API handles be usually initialised with the value "zero" by the mentioned C++ class implementations?

from fastuidraw.

Sighs.

I think we will end this (very silly discussion) with the following example of what the check is mean to handle (though not robustly):

m_foo = FASTUIDRAWnew fastuidraw::gl::Program(args);

CreateAndBindGLContext()
if(!context_fail)
{
//do stuff
UnBindAndDestroyContext().
}

in the above code, the gl::Program is made before the Context is even made and it is deleted after the context is destroyed. The purpose of the check is to skip calling a GL function when the last reference goes out of scope and then the underlying gl::Program has its dtor called.

Now, the example is artificial and not likely to come up. The -correct- thing to do, which is literally at the bottom of my TODO, is to create a "GLDoStuff" object that is the one to issue GL calls when a GL context is current.. and if there is no GL context, to do nothing (GL resources are destroyed when a GL context is deleted).

Lets close this discussion now. If you do not understand the issue at this point, then you need to really read and USE the code.

from fastuidraw.

I suggest to reconsider the consequences when this software tries to pass a "zero" as an API handle for specific OpenGL functions.
Is this implementation detail worth for another look?

from fastuidraw.

Sighs. According the the GL spec it is harmless to pass 0. What is not likely harmless is calling a GL function without a GL context. Also harmless: not calling a GL function if the value is 0. There is zero performance gain from removing the check. The check handles (though not robustly) the example above.

Please, if you have something useful to share, do so. This however is useless.

from fastuidraw.

There is zero performance gain from removing the check.

I imagine that every extra check in the source code has got a higher cost than "zero".

I assume that you put too many expectations in questionable "sanity checks". I guess that a safer software design could avoid some development concerns here.

Can an increased use of smart pointers help to improve the discussed software situation a bit more?

from fastuidraw.

gl::Program is a reference counted object, it is handled via refernece_counted_ptr<> which is a smart pointer implementation. The dtor for a reference counted object needs to get called when the last reference goes out of scope. The check is there as a stub-check for the case if the object was created before a GL context was made current and destroyed after the GL context is current. As I said repeatedly, the right thing to do is to have a GL context "do stuff" thingy which would execute GL calls. The gl::Program would have a reference to that GLContextDoStuff object, send the command to it, and the GLContextDoStuff would have the "brains" to decide to execute the GL call, defer it for later, or never execute it. Implementing this is at the bottom of my TODO.

from fastuidraw.