subprograms with C string arguments

[Fabien-Chouteau]

Hi @mgrojo, I was trying to use shaders functions such as Sf.Graphics.Shader.createFromFile or Sf.Graphics.Shader.createFromFile and got into problems. The issue is that those functions expect C strings (a.k.a. pointers) and the Ada String type is not that.

I see two complementary solutions to that:

1. Change the specification to use the type Interfaces.C.Strings .chars_ptr, and let the users do the conversion.
2. Add an Ada String friendly version of the function that does the conversion for the users.

   function createFromFile
           (vertexShaderFilename : chars_ptr;
            geometryShaderFilename : chars_ptr;
            fragmentShaderFilename : chars_ptr) return sfShader_Ptr;

   function createFromFile
           (vertexShaderFilename : String;
            geometryShaderFilename : String;
            fragmentShaderFilename : String) return sfShader_Ptr;


   --  In the body
   function createFromFile
           (vertexShaderFilename : String;
            geometryShaderFilename : String;
            fragmentShaderFilename : String) return sfShader_Ptr
   is
      C_vertexShaderFilename   : chars_ptr := New_String (vertexShaderFilename);
      C_geometryShaderFilename : chars_ptr := New_String (geometryShaderFilename);
      C_fragmentShaderFilename : chars_ptr := New_String (fragmentShaderFilename);
      Result : sfShader_Ptr;
   begin
      Result := createFromFole (C_vertexShaderFilename, C_geometryShaderFilename, C_fragmentShaderFilename);

      Free (C_vertexShaderFilename);
      Free (C_geometryShaderFilename);
      Free (C_fragmentShaderFilename);
   end createFromFile;

Thanks for all the great work :)

[mgrojo]

Thanks for reporting.

Option 2 was the intention. I probably forgot to move the import to the body and implement the usual conversion in the function body. The bad thing is that these kinds of errors are not detected by the linking phase. I wonder if there would be more erroneous cases, there are many functions I've not tested myself. I'll try to detect that and fix all of them.

[mgrojo]

I've fixed the code in the master branch. All the subprograms with a string parameter in this package were affected, but the remaining ones look good. @Fabien-Chouteau, could you confirm it works for you? Then we can close the issue.

[Fabien-Chouteau]

Thanks @mgrojo , it works but not in all cases.

At least for the some of the shaders function, passing NULL pointer has a special meaning that is different from an empty string.

For instance when I do:

         This.Glow_Shader := createFromMemory
           ("",
            "",
            ASFML_Sim.Window.Shaders.Glow);

There's an error because empty string is not a valid shader.

One option would be to not convert the string and just pass null NULL when the argument is an empty Ada string. E.g.

      C_vertexShader   : chars_ptr :=
        (if vertexShader'Length /= 0
         then New_String (vertexShader)
         else Null_Ptr);

But then I don't know if there are cases where you want to pass an actual empty string and not a NULL pointer...

[mgrojo]

I thought CSFML would treat both cases equivalently, but it calls different C++ methods depending on being NULL or not, and I suppose SFML does not expect them to be empty in any case.

https://github.com/SFML/CSFML/blob/a3657e112a7c3204f583f898e369b3f0c4044c37/src/SFML/Graphics/Shader.cpp#L87

Let's suppose that there's no need to pass "" where NULL is meaningful and do it as you propose. I think it makes sense.

[mgrojo]

Implemented for the functions mentioning NULL for strings. Let me know if there is still any problem.

[mgrojo]

@Fabien-Chouteau I plan to publish a new release, and apparently this is fixed. Feel free to comment and/or reopen this if there is something still missing.