Bitmap dirty improvements #4432

jepler · 2021-03-18T14:42:14Z

In #4430, @kmatch98 raised an issue where calling displayio_bitmap_set_dirty_area with an "empty area" could in fact dirty some pixels.

This made me start looking in the code for ways to make the C APIs more resistant to misuse. To that end, I

Added more area helpers (canon, copy_coords and area_empty)
- canon swaps the coordinates of an area if necessary to make it "canonical"
- copy_coords copies the x1/y1/x2/y2 coordinates of one area (but not its 'next' pointer) to another
- empty_area is a predicate that returns true if the area is empty, false otherwise
Improve displayio_area_union so that it handles the union of empty areas properly (if one area is empty, the result is the other area's coords)
make displayio_bitmap_set_dirty_area use displayio_area routines
- taking in an area structure also improves code size a bit, at the expense of requiring temporary variables in callers
the area computed in this way can also be used in some of the bitmap routines to remove redundant code & calculations (see fill_region)
Moved the responsibility for read-only checking to displayio_bitmap_set_dirty_area
converted rotozoom to use set_dirty_area + write_pixel instead of bitmap_set_pixel

The adafruit_feather_rp2040 build got 216 bytes smaller with this change.

I did not TEST this beyond making sure it built, so I'm creating the PR in draft state.

.. and simplify the implmentation of displayio_area_union This _slightly_ changes the behavior of displayio_area_union: Formerly, if one of the areas was empty, its coordinates were still used in the min/max calculations. Now, if one of the areas is empty, the result gets the other area's coords In particular, taking the union of the empty area with coords (0,0,0,0) with the non-empty area (x1,y1,x2,y2) would give the area (0,0,x2,y2) before, and (x1,y1,x2,y2) after the change.

This routine will be used to simplify code that deals with ranges of bitmap coordinates.

.. simplifying code in the process. For instance, now fill_region uses area routines to order and constrain its coordinates. Happily, this change also frees a modest amount of code space.

…ty_area This is a modest code savings, but more importantly it reduces boilerplate in bitmap-modifying routines. Callers need only ensure they call displayio_bitmap_set_dirty_area in advance of the bitmap modifications they perform. (note that this assumes that no bitmap operation can enter background tasks. If an operation COULD enter background tasks, it MUST re-dirty the area it touches when it exits, simply by a fresh call to set_dirty_area with the same area as before)

jepler · 2021-03-18T23:43:04Z

@kmatch98 @FoamyGuy with my other work items I don't think I'll find the time to test this particularly soon. If one of you has the chance to put it through its paces it would be appreciated. Otherwise, I'll try to get back to it next week!

There's bound to be at least one problem introduced by this, I'm just curious what it is 😬

kmatch98 · 2021-03-18T23:59:42Z

@jepler I'll spend some time on this the next couple of days. Looks like your paying back a lot of debt (mostly withdrawn by me recently!). I'll do a code look and then some real trials to look for any weirdness.

FoamyGuy · 2021-03-19T02:17:05Z

Thanks for the improvements @jepler. I will test this out tomorrow

FoamyGuy · 2021-03-19T23:25:05Z

There seems to be an issue with the way that the progress_bar library is working. Running the progress_bar simpletest I am getting this mostly filled bar with lines in it:

The function responsible for updating the progress bars bitmap is here: https://github.com/adafruit/Adafruit_CircuitPython_ProgressBar/blob/816674747e2ea63c799a3322bbd81e53c7046dff/adafruit_progressbar.py#L94

Same user code and library work as expected on CircuitPython 6.2.0-beta.4

jepler · 2021-03-20T00:12:38Z

That's very "interesting" 😁 , clearly it needs more testing and less simply feeling confident it must be right :) I'll try to find time to dig in to it next week.

kmatch98 · 2021-03-20T01:09:03Z

I replicated @FoamyGuy observations with some bitmap pixel writes. It looks "timing" related because if I write pixels relatively slow it looks ok, but if I write them fast, then it is hit or miss. I suspect this is related to the refresh cycles and if they hit during the middle of the drawing. Will dig in more and report back.

kmatch98 · 2021-03-20T01:33:12Z

It was a < vs > issue in displayio_area_canon in shared-module/displayio/__init__.c.

Here's my branch with this one minor tweak: https://github.com/kmatch98/circuitpython/tree/bitmap-dirty-improvements

…thub.com/kmatch98/circuitpython into bitmap-dirty-improvements

jepler · 2021-03-20T01:42:10Z

Thanks @kmatch98 I pulled in that change!

kmatch98 · 2021-03-20T02:46:21Z

Found an off-by-one error in bitmaptools.rotozoom. Here's the minor updates: https://github.com/kmatch98/circuitpython/tree/bitmap-dirty-improvements

I checked bitmap.fill, bitmap.blit and bitmaptools.fill_region and bitmaptools.rotozoom. Tommorrow I will look a spend a little more time looking at bitmap loading (imageload and OnDiskBitmap) and doing changes to palettes.

Here's my test code so far:

# Trials for bitmap dirty rectangle tracking

# Move text around, label and bitmap_label
# Make bitmap boxes
# 1. Move them
# 2. Change palette
# 3. Change individual pixels with direct indexing
# 4. Add blit copy into them
# 5. Add rotozoom copy into them

# ToDo:
# OnDiskBitmap
# - move them
#

# image load bitmaps
# - move bitmaps

import displayio
import board
import time
import math
import gc
from bitmaptools import rotozoom, fill_region

delay_time=1

display=board.DISPLAY

bmap0=displayio.Bitmap(20,20, 5)
bmap0.fill(3)

bmap1 = displayio.Bitmap(10,10, 5)
palette1=displayio.Palette(5)
#palette1.make_transparent(3)
palette1[0]=0xFF0000
palette1[1]=0x00FF00
palette1[2]=0x0000FF
palette1[3]=0x00FFFF
palette1[4]=0xFFFFFF

tilegrid0=displayio.TileGrid(bmap0, pixel_shader=palette1)
tilegrid1=displayio.TileGrid(bmap1, pixel_shader=palette1)

group1=displayio.Group(max_size=10)
group1.x=100
group1.y=100

group1.append(tilegrid0)
group1.append(tilegrid1)

display.show(group1)

print('1 red square in upper left corner of light blue square')
time.sleep(1)

print('2 move by 10,10 pixels')
tilegrid1.x=10
tilegrid1.y=10
time.sleep(1)

print('3 change fill color of bitmap to blue')
bmap1.fill(2)
time.sleep(1)

print('4 direct write of one red pixels at upper left of blue square')
bmap1[0]=0 # works ok
time.sleep(1)

print('5 direct write of one red pixel at 1,1 in blue square')
bmap1[1,1]=0
time.sleep(1)

print('6 SLOW direct write of red pixels into blue')
for i in range(bmap1.width):
    for j in range(bmap1.height):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
        bmap1[i,j]=0 #
        time.sleep(0.05)
time.sleep(delay_time)

print('7 FAST direct write of light blue pixels into those red ones')
for i in range(bmap1.width):
    for j in range(bmap1.height):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
        bmap1[i,j]=3 #
time.sleep(delay_time)


print('8 SLOW direct write of red pixels into blue')
for i in range(bmap1.width):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
    bmap1[i,i]=0
    time.sleep(0.05)
time.sleep(delay_time)


display.auto_refresh=False
print('9 auto_refresh=False: FAST direct write of light blue pixels into those red ones\n then refresh')
for i in range(bmap1.width):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
    bmap1[i,i]=3
display.refresh()
time.sleep(delay_time)
display.auto_refresh=True


print('10 auto_refresh=True: FAST direct write of red pixels into light blue')
for i in range(bmap1.width):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
    bmap1[i,i]=0
time.sleep(delay_time)

print('11 blit bmap1 into the upper left of larger bmap0')
bmap0.blit(0,0,bmap1)
time.sleep(delay_time)

print('12 dual rotozoom of bmap1 into the other two corners, bottom left should be rotated 90 degrees')
# Note: May have some rounding errors that make it look not perfect
rotozoom(dest_bitmap=bmap0, ox=(bmap1.width*3)//2, oy=bmap1.height//2, source_bitmap=bmap1)
rotozoom(dest_bitmap=bmap0, ox=bmap1.width//2, oy=(bmap1.height*3)//2, source_bitmap=bmap1, angle = 11/7)
time.sleep(delay_time)

print('fill bmap0 with light blue')
bmap0.fill(3)
time.sleep(delay_time)

print('pop off bmap1 from group')
group1.pop()
time.sleep(delay_time)

print('fill_region with red')
fill_region(dest_bitmap=bmap0,
                        x1=1,y1=1,
                        x2=bmap0.width-1,y2=bmap0.height-1,
                        value=0)
time.sleep(delay_time)


while True:
    pass

jepler · 2021-03-20T15:03:29Z

@kmatch98 this is great! you should add your test in tests/circuitpython-manual

kmatch98 · 2021-03-20T17:02:10Z

Ok, I did some more tests and didn't find anything more. All I found else was that one issue in rotozoom (listed above). Once you merge that, I think it's good to me. I'll add a review.

Here's my latest test code that I used, now including tests with adafruit_imageload and displayio.OnDiskBitmap.

# Trials for bitmap dirty rectangle tracking

# Move text around, label and bitmap_label
# Make bitmap boxes
# - Move them
# - Change palette
# - Change individual pixels with direct indexing
# - Add blit copy into them
# - Add rotozoom copy into them
#
# image load bitmaps
# - move bitmaps
# - adjust scale
#
# OnDiskBitmap
# - move them
#


import displayio
import board
import time
import math
import gc
from bitmaptools import rotozoom, fill_region
import adafruit_imageload
from _pixelbuf import colorwheel

delay_time=0.5

display=board.DISPLAY

bmap0=displayio.Bitmap(20,20, 5)
bmap0.fill(3)

bmap1 = displayio.Bitmap(10,10, 5)
palette1=displayio.Palette(5)
#palette1.make_transparent(3)
palette1[0]=0xFF0000
palette1[1]=0x00FF00
palette1[2]=0x0000FF
palette1[3]=0x00FFFF
palette1[4]=0xFFFFFF

tilegrid0=displayio.TileGrid(bmap0, pixel_shader=palette1)
tilegrid1=displayio.TileGrid(bmap1, pixel_shader=palette1)

group1=displayio.Group(max_size=10)
group1.x=100
group1.y=100

group1.append(tilegrid0)
group1.append(tilegrid1)

display.show(group1)

print('1 red square in upper left corner of light blue square')
time.sleep(1)

print('2 move by 10,10 pixels')
tilegrid1.x=10
tilegrid1.y=10
time.sleep(1)

print('3 change fill color of bitmap to blue')
bmap1.fill(2)
time.sleep(1)

print('4 direct write of one red pixels at upper left of blue square')
bmap1[0]=0 # works ok
time.sleep(1)

print('5 direct write of one red pixel at 1,1 in blue square')
bmap1[1,1]=0
time.sleep(1)

print('6 SLOW direct write of red pixels into blue')
for i in range(bmap1.width):
    for j in range(bmap1.height):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
        bmap1[i,j]=0 #
        time.sleep(0.05)
time.sleep(delay_time)

print('7 FAST direct write of light blue pixels into those red ones')
for i in range(bmap1.width):
    for j in range(bmap1.height):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
        bmap1[i,j]=3 #
time.sleep(delay_time)


print('8 SLOW direct write of red pixels into blue')
for i in range(bmap1.width):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
    bmap1[i,i]=0
    time.sleep(0.05)
time.sleep(delay_time)


display.auto_refresh=False
print('9 auto_refresh=False: FAST direct write of light blue pixels into those red ones\n then refresh')
for i in range(bmap1.width):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
    bmap1[i,i]=3
display.refresh()
time.sleep(delay_time)
display.auto_refresh=True


print('10 auto_refresh=True: FAST direct write of red pixels into light blue')
for i in range(bmap1.width):
    #print('bmap1.  {},{} color index is {}'.format(i,i,bmap1[i,i]))
    #print('writing red at {},{}'.format(i,i))
    bmap1[i,i]=0
time.sleep(delay_time)

print('11 blit bmap1 into the upper left of larger bmap0')
bmap0.blit(0,0,bmap1)
time.sleep(delay_time)

print('12 dual rotozoom of bmap1 into the other two corners, bottom left should be rotated 90 degrees')
# Note: May have some rounding errors that make it look not perfect
rotozoom(dest_bitmap=bmap0, ox=(bmap1.width*3)//2, oy=bmap1.height//2, source_bitmap=bmap1)
rotozoom(dest_bitmap=bmap0, ox=bmap1.width//2, oy=(bmap1.height*3)//2, source_bitmap=bmap1, angle = -90 * 2 * math.pi/360)
time.sleep(delay_time)

print('13 fill bmap0 with light blue')
bmap0.fill(3)
time.sleep(delay_time)

print('14 pop off bmap1 from group')
group1.pop()
time.sleep(delay_time)

print('15 fill_region with red, leave one pixel border')
fill_region(dest_bitmap=bmap0,
                        x1=1,y1=1,
                        x2=bmap0.width-1,y2=bmap0.height-1,
                        value=0)
time.sleep(delay_time)

print('16 change red to yellow through palette color change')
palette1[0]=0xFFFF00
time.sleep(delay_time)

print('17 change scale to 3x')
group1.scale=3
time.sleep(delay_time)

print('18 change scale to 3x')
group1.x=30
group1.y=30
time.sleep(delay_time)

print('19 Load an image using adafruit_imageload, into bmap0')
bmap0, palette1 = adafruit_imageload.load("/purple.bmp",
                                         bitmap=displayio.Bitmap,
                                         palette=displayio.Palette)
# did not update ***** Didn't work with fast or standard reading speed
# did not update with "latest" or in 6.2
# maybe this is an issue with changing the reference of bmap0 that leaves a phantom bitmap "bmap0"
# out there somewhere in lala land.....
time.sleep(delay_time)

for _ in group1:
    group1.pop()
time.sleep(delay_time)
group1.append(tilegrid0)
# did not update either. *** Maybe when a tilegrid bitmap is changed that it doesn't realize it?
# did not work with "latest" or in 6.2
# maybe this is an issue with changing the reference of bmap0 that leaves a phantom bitmap "bmap0"
# out there somewhere in lala land.....

# tilegrid3=displayio.TileGrid(bmap0, pixel_shader=palette1)
# group1.append(tilegrid3)
# time.sleep(delay_time)
# # this displays the purple.bmp bitmap.

group1.pop(-1)
del bmap0, palette1
# clear out some RAM
gc.collect()
print('gc.mem_free: {}'.format(gc.mem_free()))

print('20 Load an image using adafruit_imageload: purple.bmp, into bmap2, scale is 3x -> off screen')
bmap2, palette2 = adafruit_imageload.load("/purple.bmp",
                                         bitmap=displayio.Bitmap,
                                         palette=displayio.Palette)
tilegrid2=displayio.TileGrid(bmap2, pixel_shader=palette2)
group1.append(tilegrid2)
time.sleep(delay_time)

print('21 set scale back to 1')
group1.scale=1
time.sleep(delay_time)

print('22 move x to 0')
group1.x=0
time.sleep(delay_time)

print('23 move y to 0')
group1.y=0
time.sleep(delay_time)

print('24 change some palette colors')
for i in range(125):
    palette2[i]=colorwheel(i)
    time.sleep(0.01)
time.sleep(delay_time)

print('25 pop everything out of group1')

for _ in group1:
    group1.pop()

time.sleep(delay_time)

print('26 add OnDiskBitmap: purple.bmp')
file="/purple.bmp"
f= open(file, "rb")
odb = displayio.OnDiskBitmap(f)
face = displayio.TileGrid(odb, pixel_shader=displayio.ColorConverter())
group1.append(face)
time.sleep(delay_time)

print('26 move OnDiskBitmap')
group1.x=30
group1.y=30
time.sleep(delay_time)

print('27 move back home OnDiskBitmap')
group1.x=0
group1.y=0
time.sleep(delay_time)

while True:
    pass

kmatch98

Looks like you added the rotozoom fix, so all looks good to me.

jepler added 5 commits March 17, 2021 20:25

arrayblit: mark bitmap area as dirty

47ca792

displayio: area: add displayio_area_canon

3b506f0

This routine will be used to simplify code that deals with ranges of bitmap coordinates.

displayio_bitmap_set_dirty_area: rewrite in terms of displayio_area

36d608a

.. simplifying code in the process. For instance, now fill_region uses area routines to order and constrain its coordinates. Happily, this change also frees a modest amount of code space.

tannewt requested a review from kmatch98 March 18, 2021 17:39

tannewt added the displayio label Mar 18, 2021

switch to > in displayio_area_canon

40829d4

jepler added 2 commits March 19, 2021 20:40

rotozoom: switch to using set_dirty_area + write_pixel

95ac6c7

Merge commit '40829d4cc85394513ca631cff23254344e29786c' of https://gi…

39500cd

…thub.com/kmatch98/circuitpython into bitmap-dirty-improvements

jepler marked this pull request as ready for review March 20, 2021 01:42

off by one error in rotozoom dirty_area

9b18893

kmatch98 approved these changes Mar 20, 2021

View reviewed changes

jepler merged commit e084a92 into adafruit:main Mar 20, 2021

kmatch98 mentioned this pull request Mar 25, 2021

displayio and vectorio: move to displayio_area_union and away from _expand #4486

Merged

dhalbert mentioned this pull request Mar 27, 2021

6.2.0 rc.0 release notes #4437

Closed

jepler deleted the bitmap-dirty-improvements branch November 3, 2021 21:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bitmap dirty improvements #4432

Bitmap dirty improvements #4432

jepler commented Mar 18, 2021

jepler commented Mar 18, 2021

kmatch98 commented Mar 18, 2021

FoamyGuy commented Mar 19, 2021

FoamyGuy commented Mar 19, 2021 •

edited

Loading

jepler commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

jepler commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

jepler commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

kmatch98 left a comment

Bitmap dirty improvements #4432

Bitmap dirty improvements #4432

Conversation

jepler commented Mar 18, 2021

jepler commented Mar 18, 2021

kmatch98 commented Mar 18, 2021

FoamyGuy commented Mar 19, 2021

FoamyGuy commented Mar 19, 2021 • edited Loading

jepler commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

jepler commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

jepler commented Mar 20, 2021

kmatch98 commented Mar 20, 2021

kmatch98 left a comment

Choose a reason for hiding this comment

FoamyGuy commented Mar 19, 2021 •

edited

Loading