Bitmaptools boundary_fill #5145
Conversation
There was a problem hiding this comment.
The extra check should prevent an infinite loop situation.
My only other comment is that I don't like the parameter name background_value. We are filling an existing color which may or may not be the background.
Is there a better name. TBH I don't know.
Maybe target_value, but does target represent what we're changing from or what we're changing to.
Maybe selection_value, that makes me think of selecting something in Photoshop.
Left field thought, remove background_value entirely. Use the color that is at the (x,y) position, which is what would happen in Photoshop when you flood fill at the pointer location. Although the background_value does add a little protection against filling the wrong area.
shared-module/bitmaptools/__init__.c
Outdated
| @@ -24,6 +24,7 @@ | |||
| * THE SOFTWARE. | |||
| */ | |||
|
|
|||
| #include <string.h> | |||
There was a problem hiding this comment.
Was this needed or a debug artifact no longer required? I didn't see anything called that required string.h.
There was a problem hiding this comment.
Good catch thank you. I've removed this in the latest version but will wait to make a new commit until I address the below feedback as well.
shared-module/bitmaptools/__init__.c
Outdated
| } | ||
|
|
||
| // set dirty the area so displayio will draw | ||
| displayio_area_t area = { 0, 0, destination->width, destination->height }; |
There was a problem hiding this comment.
Were you setting the entire display dirty on purpose? Not saying you need to for this but could track the min/max x and y values to mark only the changed area dirty.
There was a problem hiding this comment.
Yep I think this is correct it should be efficient to track min/max values as is works and then only make dirty the area that actually did get changed. I'll work on that and then make a new commit once it's complete.
Thanks again for taking a look!
shared-module/bitmaptools/__init__.c
Outdated
| ); | ||
|
|
||
| int16_t minx = x; | ||
| int16_t miny = x; |
There was a problem hiding this comment.
Shouldn't miny = y and maxx = x respectively?
With the current setting say a 1 point fill at (5,10) would mark the dirty area as (5,5) to (10,10) instead of just (5,10) to (5,10). Or another way if x starts at 5 but moves 2 wide to 7, the maxx is still 10 which is larger then the changes area.
There was a problem hiding this comment.
whoops, yes each should be their own respective values. Thank you
There was a problem hiding this comment.
fixed this with latest commit
| } | ||
|
|
||
| // set dirty the area so displayio will draw | ||
| displayio_area_t area = { minx, miny, maxx + 1, maxy + 1}; |
There was a problem hiding this comment.
Is there a reason the max x/y is +1 that maybe I'm missing?
There was a problem hiding this comment.
I am not 100% sure about the reason, it seems like displayio_bitmap_set_dirty_area might be inclusive on one end and exclusive on the other, I don't see anything documenting that though. And it's not consistent on every fill which definitely seems odd to me.
I did try initially without the +1s and some of the fills are not showing the far right and bottom edge lines of pixels being filled. Most of the logic was copied from the rotozoom function which does also have the +1s in it's dirty area as well.
|
I landed here after having some freeze problems. Just keeping this here for future reference and in case somebody gets to the same point as myself. import board
import displayio
import bitmaptools
display = board.
628C
DISPLAY
bitmap = displayio.Bitmap(display.width, display.height, 3)
palette = displayio.Palette(3)
palette[0] = 0x000000
palette[1] = 0xffffff
palette[2] = 0x0000ff
tile_grid = displayio.TileGrid(bitmap, pixel_shader=palette)
group = displayio.Group()
group.append(tile_grid)
display.show(group)
# Draw a rectangle
for x in range(150, 170):
bitmap[x, 30] = 1
bitmap[x, 50] = 1
for y in range(30, 51):
bitmap[150, y] = 1
bitmap[170, y] = 1
# Draw a non-rectangle
for i in range(30, 120):
bitmap[i, i] = 1
bitmap[i+30, i] = 1
for x in range(0, 30):
bitmap[x+30, 30] = 1
bitmap[x+120, 120] = 1
# fill rectangle white
bitmaptools.boundary_fill(bitmap,display.width//2, display.height//2, 1, 0)
# fill non-rectangle blue
bitmaptools.boundary_fill(bitmap,40, 35, 2, 0)
while True:
passRESULTS
Tested on |
|
Sounds like the |
this adds a new function
boundary_fill()inside of bitmap tools which will fill all of the background colored pixels inside of an enclosed region of a Bitmap with the fill color passed by user.This is the script I used to test it:
It looks like this when it runs:
I'm not sure if I did something wrong during merging main into this branch, or something with the translations. There is a change relating to
morpheans_morphesp-240in the circuitpython.pot file that I didn't make intentionally, I'm not sure where it came from. If there is something I need to do differently to resolve that let me know and I can.I also suspect the current code for the boundary_fill implementation may not be as memory efficient as it possibly could be, it causes a MemoryError if you try to fill a region that is "too large" Maybe there is some way that it could be re-using the arrays and/or tuples that are currently being created when it adds the 4 adjacent points. I tinkered with this a bit but could not figure out the proper way to do it. If someone can point me in the right direction on that I'm happy to try to make that improvement.