flyx / OpenGLAda

Thick Ada binding for OpenGL and GLFW
flyx.github.io/OpenGLAda/
MIT License
95 stars 13 forks source link

GL.Objects.Shaders.Source doesn't like being called twice on the same list item. #18

Closed rogermc2 closed 7 years ago

rogermc2 commented 7 years ago

Attached is a program that I've simplified as much as possible to demonstrate that GL.Objects.Shaders.Source doesn't like being called twice on the same list item. When the procedure Show_All_Shader_Program_Data contains both calls to GL.Objects.Shaders.Source, the program terminates with:

/OpenGL_Projects/GLFW Tutorial/Test/test

version 400

in vec3 vp; void main() { gl_Position = vec4(vp, 1.0); } Leaving Show_All_Shader_Program_Data. An exceptiom occurred in Tutorial_1. raised PROGRAM_ERROR : my_buffers.adb:49 finalize/adjust raised exception raised PROGRAM_ERROR : finalize/adjust raised exception [2016-12-07 18:24:41] process exited with status 1, elapsed time: 02.14s

When the procedure Show_All_Shader_Program_Data contains only one call to GL.Objects.Shaders.Source, the program terminates successfully. Test.zip

flyx commented 7 years ago

I downloaded and compiled your project on OSX with GLFW3. It builds an executable test, which seems to run fine (and does not output anything on the command line). Can you specify exactly how I am supposed to compile it to reproduce the error?

rogermc2 commented 7 years ago

Apologies. I downloaded the program with the offensive line commented out to show that it works with Source only being called once but forgot to add the following instruction: Uncommenting line 17 of Utilities.adb should cause the problem: 17 Put_Line(GL.Objects.Shaders.Source(GL.Objects.Shaders.Lists.Element(List_Cursor)));

rogermc2 commented 7 years ago

The same problem seems to happen with Active_Subroutine_Uniforms

flyx commented 7 years ago

The issue is that you try to access resources after the windows is being closed. Without a Window, there is no OpenGL context, and without OpenGL context, OpenGL objects (like GL.Objects.Program) do not work.

OpenGLAda's automatic reference counting assumes that objects are going out of scope before the OpenGL context is closed. This assumption is violated by your code. It is working with the line commented out because of race conditions (subroutine returns before OpenGL context has finished unloading).

It may be a design bug in OpenGLAda that it is possible to cause this behavior, but I don't see a possibility for a workaround or a better error message. Anyway, to fix this behavior, just make sure all your GL.Objects instances go out of scope before the window is closed.

rogermc2 commented 7 years ago

Thanks flyx. I thought it was something like that but didn't realize that closing the window was causing the problem. Now that I understand what's going on, hopefully I can figure out how to guard against it.

rogermc2 commented 7 years ago

Following further testing, I found the same problem occurring prior to closing the context. Including line 83 of my_buffers.adb in the attached program produced the following on execution:

Leaving Show_All_Shader_Program_Data. Draw returned an invalid operation error: raised GL.ERRORS.INVALID_OPERATION_ERROR : gl-raise_exception_on_opengl_error.adb:23

The same result also occurred when I placed the call to Show_All_Shader_Program_Data at lines 30 or 37 of display.adb.

So it seems that something is not being finalized properly when Show_All_Shader_Program_Data terminates? Test.zip

rogermc2 commented 7 years ago

I am actually having a similar problem with my main project (Tutorial_2). Show_All_Shader_Program_Data is called prior to drawing and seems to run correctly. On execution, the OpenGL window appears and drawing occurs correctly. However, when I close the OpenGL window I get: An exceptiom occurred in Tutorial_2. raised PROGRAM_ERROR : buffers_and_shaders.adb:83 finalize/adjust raised exception This occurs with only ONE call to Shaders.Source in Show_All_Shader_Program_Data! (buffers_and_shaders.adb corresponds to my_buffers.adb of my test program.) I'm wondering if I should use GL.Objects.Programs.Program.Delete_Id somewhere?

flyx commented 7 years ago

The Show_All_Shader_Data error may be a different problem and I need to investigate.

As for your second error: Whenever you get finalize/adjust raised exception, the problem occurs when one of your GL.Object variables goes out of scope after the context is destroyed. This may be any subprogram that takes such a value as parameter or any declare scope where such a value is declared.

If you have such variables that go out of scope too late, calling Delete_Id on them before destroying the context will fix the problem. I may come up with a fix that will make this unnecessary, but for the time being, you have to check the lifetimes of your object variables and make sure they will be deleted before the context is destroyed.

rogermc2 commented 7 years ago

I think I understand you explanation. However, I am baffled that the finalize/adjust raised exception on window closure seems to be caused by the Show_All_Shader_Program_Data instruction: Shader_Source := To_Unbounded_String(Shaders.Source(Shaders.Lists.Element(List_Cursor))); even though Show_All_Shader_Program_Data exits well before window closure. I can't see what GL.Object variable is involved that isn't involved without this instruction. I assume that all the Show_All_Shader_Program_Data local variables are properly finalized/adjusted by standard Ada runtime mechanisms. The value returned by Shaders.Source(Shaders.Lists.Element(List_Cursor) is just a standard string. So, is Shaders.Source internally generating some GL.Object variable that doesn't get deleted? Or perhaps incrementing some count on some such GL.Object variable? If so how can I determine what it is so that I can call Delete_Id on it.

I thought perhaps that the Show_All_Shader_Program_Data local variable Shaders_List : Shaders.Lists.List := Programs.Attached_Shaders(aProgram); might be an GL.Object variable but then realized that this gets generated irrespective of calling Shaders.Source.

flyx commented 7 years ago

Shaders.Lists.Element returns a GL.Objects.Object (namely GL.Objects.Shaders.Shader). This might be the culprit.

And yes, the Shaders_List may indeed be part of the problem, because it contains a list of objects that will be finalized when it goes out of scope.

I have little time to investigate further, but you can yourself with gdb step through the code to see what's happening. Of course I will think of a proper solution for the problem, but currently, I lack a good idea how to solve it.

rogermc2 commented 7 years ago

Thanks flyx, I'll have a go at using gdb on it but previously I've not had much success with it due to the level of complexity involved. However, hopefully, your clues may point me in the right direction.

rogermc2 commented 7 years ago

Following your advice, I traced GL.Objects.Shaders.Lists.Element(List_Cursor) with gdb and found that it generates a shader object so I declared and set a shader variable to GL.Objects.Shaders.Lists.Element(List_Cursor) This seemed to be a step in the right direction as the test program than ran without problem and without needing to run Delete_ID on the shader variable. I then tried to declare a second shader variable and set it to the same Element. However, this resulted in: Draw returned an invalid operation error: raised GL.ERRORS.INVALID_OPERATION_ERROR : gl-raise_exception_on_opengl_error.adb:23 The attached program should run OK only if you delete the lines containing shader2.

So it seems that Shaders.Source(Shaders.Lists.Element(List_Cursor) generates a non-visible shader object that doesn't get deleted. I don't suppose there's any way of applying Delete_ID to this invisible object?

However, from a practical point of view, this problem can be overcome by declaring a shader variable and setting it to Shaders.Lists.Element(List_Cursor).

I've yet to check what happens when I try to step through the Shaders_List which I will look into next.

Test.zip

rogermc2 commented 7 years ago

The attached program works OK but I will check it further with more than two shaders. Test.zip

rogermc2 commented 7 years ago

The attached program with four shaders works OK. So I now have a work-around for my original problem caused by Shaders.Lists.Element(List_Cursor) in Shaders.Source(Shaders.Lists.Element(List_Cursor)) generating an "invisible" shader object. So I suppose I should close this Issue; but it would be nice if "invisible" objects could be automatically finalized prior to going out of scope. Thanks for your advice. With this experience behind me, hopefully I can also fix my main project; if not I'll probably raise a separate issue. Test.zip

flyx commented 7 years ago

This is actually a real OpenGLAda bug and I will push a proper fix shortly.

The problem is that Attached_Shaders returns a list of reference-counted shader objects created from IDs gotten from the OpenGL API. When they go out of scope, OpenGLAda deletes these shader object in OpenGL because the reference count gets 0. This is an error! The IDs have been queried from OpenGL and are probably used elsewhere.

The solution for this problem will fix the error you get while the window is open. However, this will not solve the problem that objects must be deleted before the context goes out of scope.

rogermc2 commented 7 years ago

I downloaded OpenGLAda-master and rebuilt but my test program which worked yesterday failed with: Leaving Show_All_Shader_Program_Data. An exceptiom occurred in Tutorial_1. raised PROGRAM_ERROR : my_buffers.adb:49 finalize/adjust raised exception raised PROGRAM_ERROR : finalize/adjust raised exception [2016-12-13 07:03:15] process exited with status 1, elapsed time: 03.41s

I wonder if I downloaded the right thing. It is dated Yesterday 10.17 AM (I presume local time here) which doesn't make much sense. Time here at the moment today is 7.08 AM I'll try some debugging later.

Test.zip

flyx commented 7 years ago

Well all your shader variables in Load_Buffer live longer than the context, since that is closed in Draw. Those are likely to cause the error.

rogermc2 commented 7 years ago

Note that my latest test programs don't call Draw. But your "likely cause" regarding context is the culprit. With your latest OpenGLAda, calling Delete_Id against each of my seven OPenGL objects before leaving Load_Buffer fixes the problem. So, all is well! Thanks.

Test.zip

onox commented 7 years ago

@rogermc2 FYI On Github you can have code listings with syntax highlighting by writing 3 backticks and then the name of the programming language ("```ada", "```glsl", etc.). End the code block with 3 backticks again. Then you get something like below:

#version 400
in vec3 vp;
void main()
{
    gl_Position = vec4(vp, 1.0);
}