flyx / OpenGLAda

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

Transform Feedback functions #111

Closed rogermc2 closed 6 years ago

rogermc2 commented 6 years ago

I am working on a project involving transform feedback functions and notice that so far only three are implemented in OpenGLAda:

function Transform_Feedback_Buffer_Mode (Subject : Program) return Buffer_Mode;
function Transform_Feedback_Varyings (Subject : Program) return Size;
function Transform_Feedback_Varying_Max_Length (Subject : Program) return Size;

I'll implement other functions that I need in my project but I'm not sure if they'll be good enough to introduce into the OpenGLAda public code. What enum name should I use for GL_Interleaved_Attribs and GL_Separate_Attribs. Currently I'm using Transform_Buffer_Mode.

flyx commented 6 years ago

I think using Transform_Feedback_Buffer_Mode is more clear (after all, this is about transform feedback, not merely transforms).

Wrapping glTransformFeedbackVaryings seems to be a challenge since it takes a list of Strings. I would map the varyings parameter to a single String. That String shall contain all names, concatenated with commas (a character not allowed as a varying name). The implementation should search all commas, replace them by Character'Val (0) and append one, then you have a list of zero-terminated strings ready for OpenGL processing. You only need to throw the access Character values into an array, which you can then use to call the OpenGL function.

rogermc2 commented 6 years ago

Thanks, currently I've come across a few more implementations that I need. As there are, say half a dozen, I am thinking of putting them into a separate branch so as to submit them as a separate PR. Is this a better idea then submitting them as part of the example I am currently working on which should be my next PR example? Thanks very much for the advice re glTransformFeedbackVaryings which should help me greatly. Very much appreciated. This probably indicates that I should provide the transform feedback related as a separate PR to get them right first.

flyx commented 6 years ago

I would rather have changes to OpenGLAda core in a separate PR.

rogermc2 commented 6 years ago

OK. Will do.

rogermc2 commented 6 years ago

With regard to. the enum name for GL_Interleaved_Attribs and GL_Separate_Attribs, I find that I have actually called it just Buffer_Mode in gl-objects-programs.ads.

type Buffer_Mode is (Interleaved_Attribs, Separate_Attribs);

Is this OK or do you prefer 'Transform_Feedback_Buffer_Mode`?

rogermc2 commented 6 years ago

I've currently implemented glTransformFeedbackVaryings as shown below.

  1. Does it seem OK?
  2. Even if OK would it be better to implement it as you suggest?
   type Varyings_Array is array (UInt range <>) of  Ada.Strings.Unbounded.Unbounded_String;
  procedure Transform_Feedback_Varyings  (Object :  Program; Count : Integer; 
                                          Varyings : Varyings_Array;  
                                          Mode : Buffer_Mode) is
      use Interfaces.C.Strings;
      use Ada.Strings.Unbounded;
      C_Varyings   : chars_ptr_array (1 .. Interfaces.C.size_t (Count));
      Vary_Ptr     : chars_ptr;
   begin
      for index in Varyings'Range loop
         Vary_Ptr :=  New_String (To_String (Varyings (index)));
         C_Varyings (Interfaces.C.size_t (index)) := Vary_Ptr;
      end loop;

      API.Transform_Feedback_Varyings
        (Object.Reference.GL_Id, Size (Count), C_Varyings, Mode);
            Raise_Exception_On_OpenGL_Error;
   end Transform_Feedback_Varyings;
flyx commented 6 years ago

The problem with Unbounded_String is that it uses heap allocations for every string and is therefore not really on the level of performance we want for an OpenGL wrapper. It is also typically a pain for the user to use.

Regarding your implementation, you don't need the Count variable, since that is available as attribute of Varyings. Also, you seem to assume that Varyings starts at 1, since you assign the C string generated from Varyings (index) to C_Varyings (index) which is only correct if Varyings starts at 1.

Finally, there is a Raise_Exception_On_OpenGL_Error; missing after the API call.

I would definitely prefer an implementation that works without Unbounded_String.

rogermc2 commented 6 years ago

Would an implementation based on Indefinite_Doubly_Linked_Lists be OK?

flyx commented 6 years ago

No, the Indefinite containers all need to allocate memory each time an element is added (because they cannot allocate a reasonably large array beforehand as the simple containers do). Moreover, doubly linked lists perform poorly compared to vectors unless your use-case is primarily removing and adding elements at front or in the middle.

We need a trick like the one I described if we do not want to harm performance.

rogermc2 commented 6 years ago

OK, I suspected there might be some such problem. I'll see what I can do with your suggested method and try to find some nice "trick".

flyx commented 6 years ago

I could implement that myself, but I have increasingly little time for this project :/.

rogermc2 commented 6 years ago

This replies firstly to your earlier message. OK, I suspected that there might such a problem. Now that I think a bit more about it, making the user interface a list of comma separated phrases seems an excellent idea which I should be able to implement without too much trouble. Hopefully, there will be no need to implement it yourself as I should be able provide a suitable implementation. Your recommendations are well appreciated. Its a great pity that you have little time for this project. An even greater pity that a suitable assistant has n't appeared. I think OpenGLAda is an excellent port to an excellent programming language.

rogermc2 commented 6 years ago

Transform_Feedback_Varyings implemented. Awaiting completion of Transform Feedback example for more comprehensive testing.