Bitmap zero - Creates 1-pixel bitmap during initialization of tilegrid for background of label

[kmatch98]

Resolving zero-sized bitmap identified in issue #58.

To include a colored background for text, a single bitmap (in a TileGrid) is used as the first element of the display group. The first item is this background bitmap, and the remaining items are TileGrid elements, each with one character of text.

To overcome the zero-sized bitmap, I updated the initial instancing of the TileGrid to include a non-zero sized bitmap (single pixel).

Note: This initial instance in the group is a place holder that is later updated to the correct size after all the text is created. An alternate approach could be to create all the text TileGrids, then create the bitmap and insert it into the first position of the group. Also, do we need to check for a zero-sized box_width and box_height before setting the bitmap background?

Also, I updated the anchored_position getter and setter to prevent movement of the label when the text was updated (#60). I repaired an off-by-one error due to a difference in int() that truncates the decimals and changed it to round().

[FoamyGuy]

The anchored_position parts look good to me now. I will test them out a bit today as well.

I was thinking about the _create_background_box() 0 size issue a bit more. Instead of making a new property _background_present what if we use an if statement inside of _update_text() here like this:

if len(new_text) + self._padding_left + self._padding_right > 0:
    self[0] = self._create_background_box(lines, y_offset)

That way we will skip making the background all together if there is nothing to draw.

[FoamyGuy]

After testing this out a little bit it does seem to resolve the text getting moved problem.

I also tried out an if statement like above, however I also noticed that there are issues when trying to change the background color to None. So I think it needs some more tweaking still.

I think this solution can also help solve the recently the recently created #61 because I think we can get it to avoid creating/drawing the bitmap if it's not needed.

[FoamyGuy]

I never did get it working quite right in all scenarios and my attempts started to feel a bit over-complicated. I will come back to it a bit later tonight, or tomorrow with fresh eyes.

[FoamyGuy]

I have a branch here: https://github.com/FoamyGuy/Adafruit_CircuitPython_Display_Text/blob/remove_0_width_bitmap/adafruit_display_text/label.py that I've been using to work on this. This is working for changing background colors, and setting background color to None. I think it should avoid usage of 0 size bitmaps. But there is still some problem with it because the background box size is incorrect. Also there are lots of debugging prints in it right now.

You can check out this branch and poke around a bit if you like. I will come back to it tomorrow to try to resolve the remaining issues with it.

[kevinjwalters]

How does the bitmap background differ from setting the background before the bitmap feature? I set it to blue in the application with screenshow visible at top of https://learn.adafruit.com/clue-sensor-plotter-circuitpython/circuitpython-sensor-plotter and that was before the bitmap?

[FoamyGuy]

@kevinjwalters the code you linked is using the default font terminalio.FONT which was always working working correctly.

The issue was with custom BDF Fonts. On those the background color was not covering the whole label, but instead each individual glyph. See #49 for more info about that issue. The bitmap background was introduced to allow backgrounds to work with custom fonts.

[kevinjwalters]

@FoamyGuy Thanks, I had been puzzling over this one! Perhaps it's only for proportionally spaced ones too based on that ticket?

[kmatch98]

@FoamyGuy I reviewed the code and ran this version that you are working on (FoamyGuy@51e13b2).

Sizing: The background sizing looks right to me, I tried it with 0 padding and with +4 padding.

Logic and function organization: I agree with how you added a state variable self._added_background_tilegrid to keep track of the presence of the background. With this addition, you can now delete the initial creation of the bitmap (line 116 if self._background_color:...). The work of adding/removing the background can now fully fall to the combined efforts of _update_background_color and _create_background_box functions.

Please consider that if the background gets switched to None to just pop the background tileGrid out of the Group, and set self._added_background_tilegrid = False. That will reduce the memory usage by a little bit and I suspect it will put a little less work on the display to deal with the transparent bitmap. Also, the same "remove bitmap" function should be used if the width or height of the background is somehow <= 0.

Overall, adding this state variable adds one more state variable, but I think it will help resolve the issues that were raised, including the zero-sized bitmap and the option to reduce memory usage without any bitmap.

Once you read these comments, let me know if you want another checkout of the code, or if you want me to make any of these changes. Keep up the good work, and thanks for everyone fo finding these issues!

---

I have a branch here: https://github.com/FoamyGuy/Adafruit_CircuitPython_Display_Text/blob/remove_0_width_bitmap/adafruit_display_text/label.py that I've been using to work on this. This is working for changing background colors, and setting background color to None. I think it should avoid usage of 0 size bitmaps. But there is still some problem with it because the background box size is incorrect. Also there are lots of debugging prints in it right now.

You can check out this branch and poke around a bit if you like. I will come back to it tomorrow to try to resolve the remaining issues with it.

[FoamyGuy]

Thanks @kmatch98. That all sounds good to me.

My intention is to work on finishing this up later today. If you have some time today you can work on some of those last few tweaks mentioned if you want. But if not no worries.

Once I get to it I'll push to that same branch with an open PR on your fork. That way when it's ready you can merge that PR and everything should show up here I believe.

[kmatch98]

@FoamyGuy I’ll be tied up today until tonight but let me know if there are any loose ends to deal with then.

[FoamyGuy]

This looks good to me now. I tested on pyportal with https://learn.adafruit.com/pyportal-weather-station/code-pyportal-with-circuitpython as well as a modified display_text_background_color_padding.py to test out different padding values and setting text to empty.

I also tested with this CLUE project: https://learn.adafruit.com/adafruit-clue/clue-temperature-and-humidity-monitor which had a problem from the recent anchor point issue.

As far as I can tell everything is working as intended now, and all of these examples are successful.

[makermelissa]

Tested on Raspberry Pi and FT232H and there is noticeable improvement over the current version.

[kmatch98]

Thank you for your support on this!

…

On Jul 8, 2020, at 8:15 PM, Melissa LeBlanc-Williams ***@***.***> wrote:  Merged #59 into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.