tsdl icon indicating copy to clipboard operation
tsdl copied to clipboard

implement SDL_Vertex and SDL_RenderGeometry

Open fstandaert opened this issue 10 months ago • 1 comments

This pull request implements the SDL_Vertex type and SDL_RenderGeometry function for TSDL. SDL_RenderGeometry renders an arbitrary list of vertices, by default in order but optionally in a user-specified order. It requires SDL version 2.0.18 or later and should not be merged until TSDL is ready for SDL 2.0.18.

fstandaert avatar Apr 25 '24 15:04 fstandaert

On the efficiency of CArray.of_list and a potential Bigarray version, I think that an additional binding to SDL_RenderGeometryRaw (https://wiki.libsdl.org/SDL2/SDL_RenderGeometryRaw) would make more sense. It is similar to SDL_RenderGeometry, but only uses two arrays of 32-bit floats and one array of SDL_Colors, which the wiki pages says can be represented by integers. It would therefore be more amenable to a Bigarray version.

I confess that I am not sure how SDL_RenderGeometryRaw works, but that wouldn't necessarily stop me from writing a binding to it. I will write the binding, hopefully soon but maybe when my school semester ends since finals are coming up for me. I will also go ahead and update the minimum version requirement.

Also, the color array is sensitive to endianness, and it may be justified to create a new abstract type for color arrays and present a cleaner interface. Doing it properly will take me some time, I think.

fstandaert avatar Apr 26 '24 20:04 fstandaert

Sorry for the delay. I have implemented SDL_RenderGeometryRaw and new module Color_ba for bigarrays representing SDL_Color. Corresponding tests are also included.

I am still unsure about the details for this particular addition. I will list my thoughts here.

To begin, the SDL version of RenderGeometryRaw provides arguments for 'stride' values, i.e., the number of bytes separating each xy coordinate/color/uv coordinate. These appear to serve the purpose of allowing SDL_RenderGeometryRaw to work on vertices that are not stored contiguously in memory. For instance, here is an implementation of SDL_RenderGeometry directly from SDL (https://github.com/libsdl-org/SDL/blob/main/src/render/SDL_render.c), lines 4403 to 4420.

int SDL_RenderGeometry(SDL_Renderer *renderer,
                       SDL_Texture *texture,
                       const SDL_Vertex *vertices, int num_vertices,
                       const int *indices, int num_indices)
{
    if (vertices) {
        const float *xy = &vertices->position.x;
        int xy_stride = sizeof(SDL_Vertex);
        const SDL_FColor *color = &vertices->color;
        int color_stride = sizeof(SDL_Vertex);
        const float *uv = &vertices->tex_coord.x;
        int uv_stride = sizeof(SDL_Vertex);
        int size_indices = 4;
        return SDL_RenderGeometryRaw(renderer, texture, xy, xy_stride, color, color_stride, uv, uv_stride, num_vertices, indices, num_indices, size_indices);
    } else {
        return SDL_InvalidParamError("vertices");
    }
}

Rather than separate arrays for xy values, colors, and uv values, a single array of SDL_Vertex is available. The addresses of the first xy point, color, and uv point are obtained, and stride lengths of SDL_Vertex are used.

To me, it seems unlikely OCaml program would do something similar. Instead, I just used a bigarray for each argument and hard-coded the stride lengths to the appropriate values.

One case where the stride values could potentially be allowed was in the indices argument, which in SDL is allowed to have 1-byte, 2-byte or 4-byte integers for indices. A corresponding size_indices argument is provided. For my version, I again hard-coded the indices as 4-byte integers, though in this case an OCaml program perhaps could work with 1-or-2 byte indices.

Finally, I omitted a non-bigarray implementation of render_geometry_raw entirely because I could not think of one more useful or natural than the bigarray implementation.

As for the new Color_ba, it is relatively straightforward. The concrete representation (int, Bigarray.int8_unsigned_elt) bigarray is exposed because elsewhere in Tsdl, that same representation is used, e.g., at get_palette_colors_ba. I was tempted to leave the type abstract because manipulating the raw color array might be error prone (especially given that it is sensitive to endianness, see https://wiki.libsdl.org/SDL2/SDL_Color). However, I chose consistency with the rest of Tsdl, at least for this first attempt.

My code accounts for endianness, and works properly on my little-endian machine. However, I do not have a big-endian machine available to test it.

fstandaert avatar Aug 24 '24 18:08 fstandaert

Thanks. I merged your patches but I massaged them in 8a0a03691. I got rid of the convenience Color_ba (the t in tsdl :-).

I also (a bit hastily) tweaked the binding to SDL_RenderGeometryRaw, the stride stuff is important if you get your data in memory buffers on which you make views via bigarray.

dbuenzli avatar Sep 12 '24 14:09 dbuenzli