v923z / micropython-ulab

a numpy-like fast vector module for micropython, circuitpython, and their derivatives
https://micropython-ulab.readthedocs.io/en/latest
MIT License
422 stars 116 forks source link

Add support for 24-bit RGB values (uint24) #565

Closed CedarGroveStudios closed 1 year ago

CedarGroveStudios commented 1 year ago

To better manage memory, it would be helpful to initialize an array with uint24 or uint32 for 24-bit RGB values.

v923z commented 1 year ago

As far as I know, there is no uint24 type in standard C, so we would have to implement all operators on that from scratch. While it is not impossible, it is definitely a significant undertaking. Here are also some comments on why adding a new type might be problematic: https://github.com/v923z/micropython-ulab/issues/160, https://github.com/v923z/micropython-ulab/issues/306#issuecomment-774664893, https://github.com/v923z/micropython-ulab/issues/364#issuecomment-819811972

Having said that, if you tell a bit more about your application, we might be able to find a solution that is cheap, in terms of both implementation, and resources (RAM, and flash), and not so disruptive. E.g., if you work with images, you most certainly want to treat the channels separately. Can't you then simply have three uint8 matrices to hold your data?

Another option is to dust off https://github.com/v923z/micropython-ulab/pull/327. With that, you wouldn't even have to reserve memory for your data, you could work directly on the image.

CedarGroveStudios commented 1 year ago

Ah, I see that it would be prohibitive to create a new type of uint24.

The application is an existing set of CircuitPython displayio.Palette classes for controlling brightness, chromakey filtering, and to wrap a palette object to allow slicing (displayio palette objects are not inherently sliceable nor inheritable). The existing workaround is to create an array with separate R,G,B dimensions then recombine when needed to reconstruct the palette:

self._ref_palette = numpy.zeros((len(self._source_palette), 3), dtype=numpy.uint8)

This is an acceptable workaround with a slight increase in overhead to reconstruct the RGB integer value for situations that didn't need separate color channels like chromakeying and the slicing wrapper.

https://github.com/v923z/micropython-ulab/pull/327 could be useful when ultimately dealing with the bitmap image, but at this time I'm focused on manipulating the palette object.

Thank you for your consideration. Let's close this for now.

v923z commented 1 year ago

@CedarGroveStudios Sorry, I somehow missed your comment here. If this is a circuitpython application, then I would actually re-consider. I'm not sure I would re-open this particular issue, but I would be interested in exploring how we could construct a transparent interface to circuitpython objects.

In the particular case that you mention above,

self._ref_palette = numpy.zeros((len(self._source_palette), 3), dtype=numpy.uint8)

you actually create (i.e., construct the header and reserve memory) an ndarray for your palette. It is also possible to simply take the buffer of the palette. That saves you the memory allocation.

@jepler Jeff, do you think an interface of some sort would be useful in the broader context?

jepler commented 1 year ago

I'm not sure that the addition makes sense, though I'm open to other viewpoints.

What I wish is that the circuitpython Palette object was memoryview()able and that we could promise the in-memory format. However, (A) it's not and (B) the RGB pixel data is interleaved with other data so the memoryview'd part would be very weird to expose to users. The palette data is an array of this C structure:

typedef struct {
    uint32_t rgb888;
    uint16_t rgb565;
    uint8_t luma;
    uint8_t hue;
    uint8_t chroma;
    bool transparent; // This may have additional bits added later for blending.
} _displayio_color_t;

I wonder if it would help if CircuitPython offered a way (perhaps a function in bitmaptools, if not directly on the Palette object) to bulk set a palette from an appropriate buffer. For instance, this hypothetical function would work with a uint8 array of shape (N, 3) or (N, 4) depending on the stride value passed in:

def bulk_set_palette(p: Palette, m: ReadableBuffer, *, stride=3) -> None: ...

and can do whatever it does behind the scenes to maintain those other fields based on an rgb888 value.

v923z commented 1 year ago

I see your point, but I am still tempted to argue in favour of an interface. If we could get a pointer to the data, and simply attach it to the ndarray, then you could do quite a few nifty things. E.g., you could invert all colours in a single line:

rgb_array = np.frombuffer(display_data)
rgb_array ^= 0xff
display_data.data_pointer = rgb_array.tobytes()

or something along those lines.

Having said that, I would really like to finish https://github.com/v923z/micropython-ulab/pull/327. There the idea was to access and work with e.g., camera data without allocating extra memory. If you think that the displayio stuff could be handled there, I would be happy to bring in the required tools. I just don't know if there is demand for manipulating the contents of a display, or anything like that. On the other hand, if such a tool existed, people would come up with ideas as to how to use it.

The block object mentioned in the cited PR requires nothing but a pointer to a function that can read out the data from whatever object the data are stored in. As far as I see, we would just need implement the inverse of this function as Dan suggested in https://github.com/v923z/micropython-ulab/pull/327#issuecomment-812687120. This would avoid the problem of having to expose a memoryview explicitly.