sfRenderStates "constructors" from shaders and textures

[Fabien-Chouteau]

Hi @mgrojo, the C++ API has constructors to create a default RenderStates from a shader;

This is convenient when one uses RenderStates just to apply shaders.

I think it should be doable to have something similar in Ada:

function To_RenderState (Shader : sfShader_Ptr) return sfRenderStates 
is (blendMode => Sf.Graphics.BlendMode.sfBlendAlpha,
    transform => Identity,
    texture => null,
    shader => Shader);

[mgrojo]

Hi @Fabien-Chouteau,

Yes, the CSFML is sometimes minimal and only includes what is needed for bindings, but not general useful stuff for facilitating use of the library. In version 2.5.2 they had to add a function returning the default.

Constructors in the original C++ are always exported as create* functions in CSFML, so I'm hesitating between your naming, which is ideal per se, and createFromShader or createWithShader, which would be consistent with what CSFML does in other cases.

[mgrojo]

@Fabien-Chouteau, could you confirm it works for you? Then we can close the issue.

At the end I have chosen a create function with all the defaults as defined in the default constructor in C++, so one can have the same effect as in C++, but explicitly calling the conversion function, as expected in Ada. Additionally, I've created a default function to keep it aligned with CSFML after SFML/CSFML#180

[Fabien-Chouteau]

Thanks @mgrojo, I works!

I also realized that adding default values to the sfRenderStates record would also work:

   type sfRenderStates is record
      blendMode : aliased Sf.Graphics.BlendMode.sfBlendMode := Sf.Graphics.BlendMode.sfBlendAlpha;
      transform : aliased Sf.Graphics.Transform.sfTransform := Sf.Graphics.Transform.Identity;
      texture : sfTexture_Ptr := null;
      shader : sfShader_Ptr := null;
   end record;

  RS : aliased sfRenderStates := (shader => My_Shader, others => <>);

[mgrojo]

I tend to avoid the record initializations because I've seen sometimes they could lead to a performance issue. But since there are two access fields, I suppose there is already an initialization function for the record, so it might make sense to include the others. I don't know if having both styles is going to be too much, but I'll add this without removing the function, because I like that for consistency to other types and to the C++ class.

[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.