fix issue with background being shifted from text. only create bitmap…

[FoamyGuy]

… if it will get used.

I think these changes fix the walking text problem and avoid creating a bitmap until it's needed based on users selection of background color and current text+padding.

There is still probably improvement to be had by refactoring the bitmap creation into a function to avoid repetition. I can work on that if this version is working correctly.

@kmatch98 If you have a moment please try this out with display_text_background_color_padding.py and confirm if that seems to be working as expected now.

If these changes do look good I think they will populate over to the main repo PR if you accept them here.

[kmatch98]

@FoamyGuy I will take a look but may not be until Sunday. Also please considering adding the font file to the example package so that some of these examples will work without any changes.

[FoamyGuy]

@kmatch98 I've implemented the last few tweaks that we discussed. I think this one is good to go now.

I tested it out with the background color example again as well as a few other simple_text things that got broken by the anchored position issue. Everything seems to working as intended now.

[kmatch98]

@FoamyGuy I found a three issues to consider, mostly dealing with possible where the bitmap is added or removed from the group. One I have a fix for, the second I don't fully understand. I'll just write the information here, since I'm not savvy enough with git to edit your PR to merge these changes.

Both of these are in _updated_background_color:

1. Verify that the background bitmap is present before popping the first element. With the current code, this can cause an error if the background is updated to None twice in a row.

I added this if statement:

    def _update_background_color(self, new_color):

        if (new_color is None):
            self._background_palette.make_transparent(0)
            if self._added_background_tilegrid:
                self.pop(0)
                self._added_background_tilegrid = False

2. I tried changing the the text to nothing and it gave this error during the self.insert command. I think this has two issues. First, if the text is blank, then the logic should not show any background. How can you detect this inside of _update_background_color, or do you need to figure this out up in _create_background_box?

Note: The line numbers may have shifted since I made some edits. The "Group index out of range" error occurs when trying to insert into an empty group. That's unexpected to me, since I know that for lists, it is ok to insert an element to an empty list. I tried a simple experiment on the command line, and I think this is a bug in the Group library that errors when inserting an item into an empty Group? Maybe we should push this up as an issue in the Group label. In the meantime, you can check the length of the self. Group and if it's empty you can use append. Not so clean, but will work for now.

This was the error when changing text to blank text='':

Traceback (most recent call last):
  File "code.py", line 108, in <module>
  File "/lib/adafruit_display_text/label.py", line 329, in background_color
  File "/lib/adafruit_display_text/label.py", line 193, in _update_background_color
IndexError: Group index out of range

Here is a command line experiment that shows a bug where insert doesn't work with an empty Group:

import displayio
myGroup=displayio.Group()
myGroup2=displayio.Group()
myGroup.insert(0, myGroup2)

Here is the error that I received:

>>> myGroup.insert(0, myGroup2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: Group index out of range
>>> 

3. I don't see where the zero-sized background box is prevented from drawing. Should this be handled in _create_background_box, and send None to _updated_background_color if no background is required?

Overall it seems like this is getting complicated for a simple background, but I guess new features always add these complications. And we're still left with the open question about dealing with fixed size fonts....

[FoamyGuy]

1. Good catch. I'll push a commit today to add an if statement like yours

2. Weird, I'm not sure why the insert doesn't work. That does seem to be an issue with Group. I think for now we can work around it by checking length and appending instead of inserting if needed.

3. I think this if statement near the bottom of _update_text:

if (
    self._background_color
    and len(new_text) + self._padding_left + self._padding_right > 0
):

Should be causing the bitmap to not get added if there is no text and padding (i.e. if background would have been 0 size).

This could probably use some more testing though. I don't think I checked specifically setting the text to empty.

[kmatch98]

Item 3: Here are a couple of possible things that could arise.

_update_text: If the background size is <= 0, I think you also need to "pop" off any existing background box.

_update_background_color: I think similar zero-size checking logic is necessary when the background color is changed through _update_background_color, since this will also cause the background box to be created.

One options is that size can be checked inside of _create_background_box and then if the width or height is <= 0, then it won't create a tileGrid and returns None. Then _update_background_color can detect this None and delete the background from the Group.

One other approach is to remove any background box changes from _update_text and just call _update_background_box at the end of _update_text to prevent duplicating code.

[madgrizzle]

This stopped my program from constantly crashing.. it runs with about 20K ram free and when I went to display text, previously, it would crash. Hasn't crashed yet with this code.. knock on wood.

[kmatch98]

I reviewed your changes but I have a few more updates. I don't know the best way of merging both changes, so here is what I will do. I will merge your changes on this pull request, and then I'll merge my updates. I'll send you a note when I merge both and then you can review all these changes.