Port Texture to C code #3330
Port Texture to C code #3330
Conversation
commit 813f384 Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:43:45 2025 +0100 Add interface for _sdl2.video classes commit 702958c Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:38:01 2025 +0100 Add interface for _sdl2.video classes commit 081b032 Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:33:20 2025 +0100 Add interface for _sdl2.video classes commit c9ffd1c Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:25:10 2025 +0100 Add interface for _sdl2.video classes
commit 813f384 Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:43:45 2025 +0100 Add interface for _sdl2.video classes commit 702958c Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:38:01 2025 +0100 Add interface for _sdl2.video classes commit 081b032 Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:33:20 2025 +0100 Add interface for _sdl2.video classes commit c9ffd1c Author: Josip Komljenović <[email protected]> Date: Wed Jan 29 15:25:10 2025 +0100 Add interface for _sdl2.video classes
| if (!pg_TwoFloatsFromObj(originobj, &origin.x, &origin.y)) { | ||
| return RAISE(PyExc_ValueError, "origin must be a point or None"); | ||
| } | ||
| originptr = &origin; |
There was a problem hiding this comment.
You could pass &origin into SDL_RenderCopyExF directly rather than needing this extra variable. (I see the Cython has it, maybe it's a weird Cython thing or the person who wrote it wasn't as experienced with pointers).
There was a problem hiding this comment.
It is used because &origin cannot be NULL, but in some cases SDL_RenderCopyExF expects the center argument to be NULL
| new_texture->texture = | ||
| SDL_CreateTextureFromSurface(renderer->renderer, surf); | ||
| if (!new_texture->texture) { | ||
| return RAISE(pgExc_SDLError, SDL_GetError()); |
There was a problem hiding this comment.
I think new_texture should be freed before the RAISE?
There was a problem hiding this comment.
Good catch, rewrote the function a bit so free isn't needed in current version, but otherwise you are correct, can you check again
src_c/render.c
Outdated
| vertices[0].color.r = (int)r_mod * p1_mod[0]; | ||
| vertices[0].color.g = (int)g_mod * p1_mod[1]; | ||
| vertices[0].color.b = (int)b_mod * p1_mod[2]; | ||
| vertices[0].color.a = (int)a_mod * p1_mod[3]; | ||
| vertices[1].color.r = (int)r_mod * p2_mod[0]; | ||
| vertices[1].color.g = (int)g_mod * p2_mod[1]; | ||
| vertices[1].color.b = (int)b_mod * p2_mod[2]; | ||
| vertices[1].color.a = (int)a_mod * p2_mod[3]; | ||
| vertices[2].color.r = (int)r_mod * p3_mod[0]; | ||
| vertices[2].color.g = (int)g_mod * p3_mod[1]; | ||
| vertices[2].color.b = (int)b_mod * p3_mod[2]; | ||
| vertices[2].color.a = (int)a_mod * p3_mod[3]; |
There was a problem hiding this comment.
Casting (to int) happens before the multiplication, so (int)r_mod will be 0 unless that channel has 255 as a color mod.
There was a problem hiding this comment.
Fixed a bit and moved repetitive lines to macro (at the start of the file), can you check if the current version is correct
| FLIP_HORIZONTAL as FLIP_HORIZONTAL, | ||
| FLIP_NONE as FLIP_NONE, | ||
| FLIP_VERTICAL as FLIP_VERTICAL, |
There was a problem hiding this comment.
Do we need to expose these constants? As far as I can tell, these are always handled by keyword arguments like flip_x and flip_y
There was a problem hiding this comment.
I can remove them, used just as a possible improvement for the future if we change flip x/y arguments with the single flip that would accept something like FLIP_HORIZONTAL | FLIP_VERTICAL
This PR converts pygame._sdl2.video.Texture to C code. It has the implementation of all the Texture methods and attributes.