flyx / OpenGLAda

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

The GL.Objects.Shaders documentation on the web page doesn't mention Initialize_Id #107

Closed esthermations closed 6 years ago

esthermations commented 6 years ago

Problem page: https://flyx.github.io/OpenGLAda/gl-objects-shaders.html

This caused me a bit of a headache to track down. As far as I can tell by looking at the examples in this repo, both shaders and GL programs need to be initialised by calling Initialize_Id on them before use, but that page of the GitHub site doesn't mention this at all. It's not in the text nor the code example.

It has this (shortened):

declare
   Vertex_Shader   : GL.Objects.Shaders.Shader (Kind => GL.Objects.Shaders.Vertex_Shader);
   Fragment_Shader : GL.Objects.Shaders.Shader (Kind => GL.Objects.Shaders.Fragment_Shader);
   Program         : GL.Objects.Programs.Program;
begin
   -- load and compile shaders
   GL.Files.Load_Shader_Source_From_File (Vertex_Shader, "vertex.glsl");
   GL.Files.Load_Shader_Source_From_File (Fragment_Shader, "fragment.glsl");
   ...

This exact code causes a constraint error in Raw_Id when it's called by Load_Shader_Source_From_File. I suppose that's because the ID defaults to -1 and its type is UInt, but I don't know specifically.

The example code should be amended to at least have something like this:

declare
   ...
begin
   -- initialise
   Vertex_Shader.Initialize_Id;
   Fragment_Shader.Initialize_Id;
   Program.Initialize_Id;

   -- load and compile shaders
   GL.Files.Load_Shader_Source_From_File (Vertex_Shader, "vertex.glsl");
   GL.Files.Load_Shader_Source_From_File (Fragment_Shader, "fragment.glsl");
   ...

The actual text on that page also describes a step-by-step process for using shaders, and it completely fails to mention Initialize_Id, so I think that should be updated too.

As far as I can tell that GitHub site doesn't have a public repo, so I'm putting this issue here.

rogermc2 commented 6 years ago

I don't understand what "caused me a bit of a headache to track down"? Have you looked at using examples/common/src/program_loader? For example:

 Render_Graphic_Program := Program_Loader.Program_From
        ((Src ("src/shaders/vertex_shader.glsl", Vertex_Shader),
         Src ("src/shaders/fragment_shader.glsl", Fragment_Shader)));
esthermations commented 6 years ago

Those examples are fine and work correctly. The problem is that the official documentation (linked at the top of this issue) provides an example and explanation that causes the program to crash with a constraint error on a UInt nested two function calls into the library's private implementation.

In my opinion, that should be fixed.

rogermc2 commented 6 years ago

Agreed. Thanks for the explanation.

flyx commented 6 years ago

Initialize_Id is documented on the page of the parent package, GL.Objects. I agree that this call should definitely be part of the example code.

By the way, the website's public repo is the gh-pages branch of this repository. You are welcome to create a pull request (I won't be very active between the years).

esthermations commented 6 years ago

Ah, I didn't find it. I'll make a pull request shortly.

flyx commented 6 years ago

Fixed by PR.