set_color @ pygame.transform.threshold format is BGR instead of RGB if the image does not have an alpha channel.

[Kyuchumimo]

Expected behavior:
RGB format for set_color @ pygame.transform.threshold

Screenshots
test.png:

test2.png:

test3.png:

test4.png:

Test code

import pygame
pygame.init()

image = pygame.image.load('test.png')
pygame.transform.threshold(image, image, (255,0,255), (0,0,0,0), (255,0,0), 1, None, True)
pygame.image.save(image, 'test2.png')

image = pygame.image.load('test.png')
pygame.transform.threshold(image, image, (255,0,255), (0,0,0,0), (0,255,0), 1, None, True)
pygame.image.save(image, 'test3.png')

image = pygame.image.load('test.png')
pygame.transform.threshold(image, image, (255,0,255), (0,0,0,0), (0,0,255), 1, None, True)
pygame.image.save(image, 'test4.png')

pygame.quit()

[Fantastair]

I examined the implementation of threshold, and the primary issue lies within the _set_at_pixels() function in src_c/transform.c. This function modifies the pixels of dest_surf.

/* _set_at_pixels sets the pixel to the_color.

    x - x pos in the SDL_Surface pixels.
    y - y pos in the SDL_Surface pixels.
    format - of the SDL_Surface pixels.
    pitch - of the SDL_Surface.
    the_color - to set in the pixels at this position.
*/
static PG_INLINE void
_set_at_pixels(int x, int y, Uint8 *pixels, PG_PixelFormat *format,
               int surf_pitch, Uint32 the_color)
{
    Uint8 *byte_buf;

    switch (PG_FORMAT_BytesPerPixel(format)) {
        case 1:
            *((Uint8 *)pixels + y * surf_pitch + x) = (Uint8)the_color;
            break;
        case 2:
            *((Uint16 *)(pixels + y * surf_pitch) + x) = (Uint16)the_color;
            break;
        case 3:
            byte_buf = (Uint8 *)(pixels + y * surf_pitch) + x * 3;
#if (SDL_BYTEORDER == SDL_LIL_ENDIAN)
            *(byte_buf + (format->Rshift >> 3)) = (Uint8)(the_color >> 16);
            *(byte_buf + (format->Gshift >> 3)) = (Uint8)(the_color >> 8);
            *(byte_buf + (format->Bshift >> 3)) = (Uint8)the_color;
#else
            *(byte_buf + 2 - (format->Rshift >> 3)) = (Uint8)(the_color >> 16);
            *(byte_buf + 2 - (format->Gshift >> 3)) = (Uint8)(the_color >> 8);
            *(byte_buf + 2 - (format->Bshift >> 3)) = (Uint8)the_color;
#endif
            break;
        default: /* case 4: */
            *((Uint32 *)(pixels + y * surf_pitch) + x) = the_color;
            break;
    }
}

When processing RGB data, this code distinguishes between big-endian and little-endian formats. This is likely the direct cause of RGB being converted to BGR.

Color data of other lengths are assigned directly. I suspect that 1-, 2-, and 4-byte data are aligned and can be handled automatically by the CPU, while only 3-byte data requires manual order adjustment.

[Starbuck5]

I think the error is that "the_color" on the 24 bit path is assumed to RGB, when actually it's in the native pixelformat of the surface. Therefore it shouldn't be adjusting order based on the pixelformat at all, it should just be storing.

import pygame
pygame.init()

screen = pygame.display.set_mode((500,500))

image = pygame.image.load('test.png')
image = image.convert()
print(image, image.get_shifts()) # <Surface(128x128x32)> (16, 8, 0, 0) # (RGB)

If you open a window and convert it swaps the byte order from BGR to RGB, and then the functions work as expected.

So I think this only impacts BGR 24 bit surfaces. Therefore it seems reasonable to fix-- often things like this have been around so long that it seems safer to keep the behavior until a breaking change. But this seems niche enough, a single variant of a single format is just wrong.