Skip to content

Fast copy from ulab to displayio.Bitmap #4414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dglaude opened this issue Mar 15, 2021 · 3 comments · Fixed by #4403
Closed

Fast copy from ulab to displayio.Bitmap #4414

dglaude opened this issue Mar 15, 2021 · 3 comments · Fixed by #4403
Labels
displayio enhancement ulab Related to the ulab module
Milestone

Comments

@dglaude
Copy link

dglaude commented Mar 15, 2021

Playing with ulab, I found it was costly to copy from an array to a display.bitmap.

ulab can build a buffer: ulab.array.tobytes

Here is the current fastest way I was able to copy:

image_bitmap = displayio.Bitmap(24, 32, number_of_colors)
inta=ulab.array((npframe-min_t)*factor,dtype=ulab.int8)
index = 0
for h in range(24):
    for w in range(32):
        image_bitmap[h, w] = inta[index] 
        index+=1

@v923z
Copy link

v923z commented Mar 15, 2021

Basically, what would be needed is a way of passing the bytes of an ndarray directly into the bitmap. One option would be the tobytes array method, if displayio could accept a buffer. https://micropython-ulab.readthedocs.io/en/latest/ulab-ndarray.html?highlight=tobytes#tobytes

The module that has to be modified is most probably this: https://github.com/adafruit/circuitpython/blob/main/shared-bindings/displayio/Bitmap.c

The bitmap constructor is defined here

void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t width,
uint32_t height, uint32_t bits_per_value) {
uint32_t row_width = width * bits_per_value;
// align to size_t
uint8_t align_bits = 8 * sizeof(size_t);
if (row_width % align_bits != 0) {
self->stride = (row_width / align_bits + 1);
} else {
self->stride = row_width / align_bits;
}
self->width = width;
self->height = height;
self->data = m_malloc(self->stride * height * sizeof(size_t), false);
self->read_only = false;
self->bits_per_value = bits_per_value;

so, I believe, one could simply add a buffer keyword to

STATIC mp_obj_t displayio_bitmap_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
, and set the bytes of the ndarray to self->data, without allocating any memory.

@kmatch98
Copy link

I'm interested in helping out on this and stretching my skills a bit to try to understand better how the bitmap structure is formed and modified. Sorry I didn't chime in during today's CircuitPython call, but I'm not very well versed in the terminology y'all use, so I have to do some digging to be partially intelligible when discussing this. Here goes:

As best I understand, to save memory the bitmap structure uses the minimum number of bits necessary to hold the maximum palette indices (value_count in put parameter to displayio_bitmap_make_new(). For that reason, each pixel could be represented by somewhere between 1 and 32 bits (either 1-8, 16 or 32 bits).

So, the ndarray will somehow need to know the bits-per-value of the bitmap, the width, height, and stride.

In looking at the
ulab documentation, it looks like ndarrays can be decoded as 8, 16 or 32 bits. (I'm uncertain if < 8 bits can be handled directly in ndarray.)

I've heard y'all talk about using "buffer protocols" to handle this, but I'm not very familiar with this, and @v923z you mention adding a buffer keyword into the bitmap constructor, and @jepler mentioned mp_get_buffer on the CircuitPython call today. In a brief reading about buffers, it seems like it's a chunk of memory that you can access in different ways. Now I see why you are suggesting this. Can y'all point to an example where a similar thing like this is done?

@tannewt tannewt added displayio ulab Related to the ulab module labels Mar 15, 2021
@tannewt tannewt added this to the Long term milestone Mar 15, 2021
@jepler jepler linked a pull request Mar 16, 2021 that will close this issue
@v923z
Copy link

v923z commented Mar 16, 2021

So, the ndarray will somehow need to know the bits-per-value of the bitmap, the width, height, and stride.

No, that should definitely be done on the bitmap's side.

In looking at the
ulab documentation, it looks like ndarrays can be decoded as 8, 16 or 32 bits. (I'm uncertain if < 8 bits can be handled directly in ndarray.)

Bits can't be stored even in numpy. Booleans are just uint8 with a special flag.

I've heard y'all talk about using "buffer protocols" to handle this, but I'm not very familiar with this, and @v923z you mention adding a buffer keyword into the bitmap constructor, and @jepler mentioned mp_get_buffer on the CircuitPython call today. In a brief reading about buffers, it seems like it's a chunk of memory that you can access in different ways. Now I see why you are suggesting this. Can y'all point to an example where a similar thing like this is done?

I can only speak for ulab, but here are two examples: https://github.com/v923z/micropython-ulab/blob/714a3b872747332de9600fda4be07bbcc5f99626/code/ulab_create.c#L693-L705, and the reverse https://github.com/v923z/micropython-ulab/blob/714a3b872747332de9600fda4be07bbcc5f99626/code/ndarray.c#L1595-L1606

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
displayio enhancement ulab Related to the ulab module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants