adding bitmap property to TileGrid

[FoamyGuy]

Allows the Bitmap to be changed so that you can swap a tilegrid from one source Bitmap to another.

Tested successfully with this code:

import time
import adafruit_imageload
import displayio
import board

display = board.DISPLAY
main_group = displayio.Group()

first_bitmap, first_palette = adafruit_imageload.load("bmps/test_bmp_1.bmp")
second_bitmap, second_palette = adafruit_imageload.load("bmps/test_bmp_2.bmp")

first_palette.make_transparent(0)
second_palette.make_transparent(0)

tilegrid = displayio.TileGrid(bitmap=first_bitmap, pixel_shader=first_palette,
                              width=4, height=4,
                              tile_width=16, tile_height=16, default_tile=4)
# fill the tiles
tilegrid[0] = 0
tilegrid[3] = 2
tilegrid[12] = 6
tilegrid[15] = 8
for x in range(2):
    tilegrid[x+1, 0] = 1
    tilegrid[x+1, 3] = 7
for y in range(2):
    tilegrid[0, y+1] = 3
    tilegrid[3, y+1] = 5

# show on the display
main_group.append(tilegrid)
display.show(main_group)

while True:
    tilegrid.bitmap = second_bitmap
    tilegrid.pixel_shader = second_palette
    time.sleep(2)
    tilegrid.bitmap = first_bitmap
    tilegrid.pixel_shader = first_palette
    time.sleep(2)

When this test code run it looks like this:

changing_bitmap_on_tilegrid.mp4

I used these bmp files for testing:

test_bmps.zip

While working on this I noticed an extra large amount of whitespace in this line:

{ MP_ROM_QSTR(MP_QSTR_pixel_shader), MP_ROM_PTR(&displayio_tilegrid_pixel_shader_obj) },

and removed it as part of the PR to make the spacing match all of the lines around it.

[FoamyGuy]

It looks like this pushed a few of the M0 boards over the limit for some translations. I'm not sure of a way to squeeze it further, maybe shorten the error message?

Is it possible and worth it to disable displayio entirely on these builds? For other M0 devices like CPX there is a separate build for displayio on the Gizmo display.

[FoamyGuy]

I did find how to disable displayio and turned it off for the builds that got too big. But am open other ideas as well if there is a better solution that allows them to keep it.

[dhalbert]

I did find how to disable displayio and turned it off for the builds that got too big. But am open other ideas as well if there is a better solution that allows them to keep it.

These are workhorse boards, and it would be nice to have displayio on them. Try

CIRCUITPY_ONEWIREIO = 0

and if that's not enough, also

CIRCUITPY_RAINBOWIO = 0

though I'd prefer to keep the latter.

[jepler]

This'll be handy, but if we don't get the details right at first it'll vex users.

[jepler]

This needs additional checking, similar to what is done now during the constructor in shared-bindings.

    if (bitmap_width % tile_width != 0) {
        mp_raise_ValueError(translate("Tile width must exactly divide bitmap width"));
    }
    if (bitmap_height % tile_height != 0) {
        mp_raise_ValueError(translate("Tile height must exactly divide bitmap height"));
    }

in fact, the old and new bitmaps may need to be exactly the same resolution?

As for dealing with the type of the thing, consider factoring out this block from the TileGrid constructor in shared-bindings, and re-using its message as-is:

    mp_obj_t native = mp_obj_cast_to_native_base(bitmap, &displayio_shape_type);
    if (native != MP_OBJ_NULL) {
        displayio_shape_t *bmp = MP_OBJ_TO_PTR(native);
        bitmap_width = bmp->width;
        bitmap_height = bmp->height;
    } else if (mp_obj_is_type(bitmap, &displayio_bitmap_type)) {
        displayio_bitmap_t *bmp = MP_OBJ_TO_PTR(bitmap);
        native = bitmap;
        bitmap_width = bmp->width;
        bitmap_height = bmp->height;
    } else if (mp_obj_is_type(bitmap, &displayio_ondiskbitmap_type)) {
        displayio_ondiskbitmap_t *bmp = MP_OBJ_TO_PTR(bitmap);
        native = bitmap;
        bitmap_width = bmp->width;
        bitmap_height = bmp->height;
    } else {
        mp_raise_TypeError_varg(translate("unsupported %q type"), MP_QSTR_bitmap);
    }

[dhalbert]

  if (bitmap_width % tile_width != 0) {
        mp_raise_ValueError(translate("Tile width must exactly divide bitmap width"));
    }
    if (bitmap_height % tile_height != 0) {
        mp_raise_ValueError(translate("Tile height must exactly divide bitmap height"));
    }

Error messages add bytes, so . Maybe "Tile aspect ratio must be integral" for both? If they have to be equal, then that's shorter also.

[FoamyGuy]

I've had some success factoring this into a new function and using it from the bitmap setter. But there is still work to do I think.

Right now this code is repeated in the constructor, ideally we could use the new function from the constructor to avoid the repetition. But I'm not sure how to do it that way yet, it needs to return some of the values so they can be used afterward inside the constructor. I'll keep working on it later.

[FoamyGuy]

Disabling onewire did provide enough space on the builds that were failing, though I tried it before some of the other recent changes. There are hopefully some further improvements to be made by removing some of the currently duplicated code as well. I'll re-check the builds that failed prior once the property is sorted out.

[dhalbert]

And we are not disabling onewire completely; it is still available in busio. It's just the new onewireio module that was meant to be a smooth transition from busio.onewire to onewire is turned off.

[FoamyGuy]

I attempted to refactor the type and "size must be multiple of tile size" check into it's own function. I was able to get a function that did complete those checks successfully but I couldn't figure out how to use the function from inside of make_new()

The main problem I ran into was being unable have this function "return" the bitmap sizes, tile sizes, and Shape, Bitmap, or OndiskBitmap object. I made a few attempts storing things in an array or using pointers to "return" those values in order to use them in make_new() but couldn't get it working. And it seems to me the 3 possible types of bitmap make it tricky even if I could work out the way to get bitmap size and tile size out of the function for use inside of make_new().

Since I couldn't get it working for make_new() there was still a good chunk of code duplication so I removed that extra function and have gone toward just doing the needed checks inside the bitmap setter. It's now also enforcing the bitmap to be the same size as the previous one which negates the need to check for equal divisibility by tile size.

I tested a bit with different sized bitmaps and it doesn't cause it to crash (assuming divisibility by tile size still) but the resulting portion displayed is not correct. With a 1x1 full bitmap sized Tilegrid changing from 48x48 to 96x96 resulted in only the top left quarter of the bigger bitmap to be shown. It may be possible to get it working with different sized bitmaps, but I didn't chase it any further. Same size covers all of the uses I have in mind for this.

[FoamyGuy]

The latest commit reverts the locale change noted above.

I also noticed that there was a redundant type check inside the bitmap setter which had it's own different error message. I removed the extra one and left the one with the more concise error message (same as make_new).

The space savings from removing the redundant check and it's different error message happen to be just enough to allow the feather_m0_supersized build with ru language to complete successfully:

[dhalbert]

@FoamyGuy Do you think this is ready to merge?

[FoamyGuy]

@FoamyGuy Do you think this is ready to merge?

I do believe so. After the latest round of changes I re-tested the functionality to make sure it's still working as intended, and I also tried with differently sized bitmaps as well as invalid types to ensure that the enforcement of both is working properly.

As far as I can tell it's good to go.

[dhalbert]

This looks OK by me! @jepler do you want to review again?

[jepler]

Thanks.