Grand Unified Pixel Iteration

[Starbuck5]

Pygame-ce has a lot of code that directly accesses surface pixel data. Most relevant modules: surface, transform, draw, mask, image, pixelarray.

This code is extremely duplicative and seems to have many issues. For example, transform.c: average_surfaces has a TODO that it doesn't handle palettized surfaces, and this hasn't been resolved in at least 9 years. In another example, both SDL2 and SDL3 have pixelformats we may broadly lack support for, like SDL_PIXELFORMAT_ARGB2101010 -- 2 bits A, 10 bits R/G/B. This pixelformat is 4 bytes per pixel however, so I bet our SIMD code and the majority of our transform code would eat it up and produce fully wrong results. Also, our code does not support 12 or 15 bit surfaces. Also, our code seems to assume all 8 bit surfaces are palettized. I could go on.

But Starbuck, why would we let the user create those surfaces? We could keep it simple and support a subset.

• We don't control all surface creation sites, SDL_image can bring in any surface at any time. Also we want to support as many SDL features as possible.

I think we can have a solution that vastly increases the correctness of our codebase, makes pixel routines easier to write, and is faster than some of the stuff we have now.

Here's an example of a read/writer interface. It would provide the ability to read and decode pixels into a stack allocated buffer, then the calling code would mutate the buffer (apply whatever transformation), and the read/writer will encode and store those pixels.

#include <SDL.h>

#define PG_PIXEL_CHUNK_CAP 256

typedef struct PG_RGBAReadWriter PG_RGBAReadWriter;
typedef bool (*PG_RGBAReadWriterAdvanceFn)(PG_RGBAReadWriter *rw);

#define PG_FLAG_LINE_ALIGNED 1 << 0

struct PG_RGBAReadWriter {
    bool line_aligned;
    int cur_x, cur_y;
    // {more internal stuff probably}
    
    PG_RGBAReadWriterAdvanceFn _advance; /* set once by _init */

    // Public
    int buf_n;
    SDL_Color buf[PG_PIXEL_CHUNK_CAP];
};

/* Create once here, it sets up a backend, then you can iterate through using
 * PG_RGBAReadWriter_next. Handles src->RGBA, RGBA->dst, src/dst memory overlap.
 * Returns false on failure. */
bool
PG_RGBAReadWriter_init(PG_RGBAReadWriter *rw, SDL_Surface *src,
                        SDL_Surface *dst, uint32_t flags);

/* true: rw->buf[0..rw->buf_n) holds src pixels as SDL_Color, mutate in
 * place. false: all pixels processed and committed to dst. */
static inline bool
PG_RGBAReadWriter_next(PG_RGBAReadWriter *rw)
{
    return rw->_advance(rw);
}

bool
grayscale(SDL_Surface *src, SDL_Surface *dst)
{
    PG_RGBAReadWriter rw;
    if (!PG_RGBAReadWriter_init(&rw, src, dst, 0)) {
        return false;
    }

    while (PG_RGBAReadWriter_next(&rw)) {
        for (int i = 0; i < rw.buf_n; i++) {
            SDL_Color *c = &rw.buf[i];

            /* RGBA to GRAY formula used by OpenCV */
            Uint8 gray = (Uint8)((((76 * c->r) + 255) >> 8) +
                                  (((150 * c->g) + 255) >> 8) +
                                  (((29 * c->b) + 255) >> 8));
            c->r = c->g = c->b = gray;
            /* c->a left untouched */
        }
    }

    return true;
}

Considerations:

• Some operations are full surface, some operate on regions
• Some have complex movement or grab surrounding pixels
• Some operations only need the mapped pixel Uint32s extracted or written, some need RGBA
• Some are just read, some are just write
• Some operations are positional (they care about the position, some are not)
  • Grayscale does not, for instance, each pixel is completely indepedent
• Read/writers have potential to safely handle when 2 surfaces have overlapping pixels (subsurfaces or frombuffer), rather than corrupting things
• Reader or writer backends have the potential to have cool optimizations to encode/decode pixels in parallel using shifts for known pixelformats (which would be faster than a bunch of Map/GetRGBA calls, and could probably autovectorize)

I attempted to go through pixel accesses in pygame-ce by module, and then attempted to categorize them:
all_pixels.txt

[Starbuck5]

I used some 🤖 to draft a larger set of ideas for what the interface could look like: PIXEL_API_MOCKUP.py

For some reason, GH would not let me upload it as a c file, but it would accept is a py. But it's really a header file 😄