vectorio: palettes don't color dirty rectangles #5087
Conversation
This is a breaking change with previous palette semantic with respect to python code that uses vectorio. Displayio has breaking changes in cpy 7 for Group's removal of max_size parameter so this is as good a time as any to break everything. Currently: To color vectorio shapes correctly you have to pass in a palette with length 2. Palette[0] must be set transparent and palette[1] must be the color you want. New: To color vectorio shapes correctly you pass in a palette with length >= 1. Palette[0] will be the color of the shape. Also improves pixels per second when skipping areas that aren't covered by the shape.
shared-module/vectorio/VectorShape.c
Outdated
| } | ||
|
|
||
| // We double-check this to fast-path the case when a pixel is not covered by the shape & not call the color converter unnecessarily. | ||
| if (output_pixel.opaque) { |
There was a problem hiding this comment.
Is this check correct, or does it need to be negated with !?
I'm not sure if this is an enhancement or a simplification, but:
Haven't you made input_pixel be the same for every pixel now? If so, can the conversion to an output_pixel be done above, and the "double-check" becomes "if the output pixel is not opaque, NOTHING is drawn, and the loops can be skipped"? This also moves code out of the inner loop, so that maybe it becomes faster.
There was a problem hiding this comment.
I'm not sure why my local feather test harness is working right with that error. I swear I am a professional software developer 🤦
re input_pixel:
Look closer at what displayio_colorconverter_convert can do. This has to be in here (I tested that colorconverter transparency still works).
The perf increase here comes from skipping a function call per pixel that we know is not covered by the shape. We were computing a color that we knew we were never going supposed to consume.
There was a problem hiding this comment.
I woke up this morning and realized you meant something else about the pixel value than what I initially understood. So yeah I agree with you and will update it - I have a larger change baking atm and will wrap it into that.
This is a breaking change with previous palette semantic with respect to python code that uses vectorio.
Displayio has breaking changes in cpy 7 for Group's removal of max_size parameter so this is as good a
time as any to break everything.
Currently:
To color vectorio shapes correctly you have to pass in a palette with length 2. Palette[0] must be set transparent and palette[1] must be the color you want.
New:
To color vectorio shapes correctly you pass in a palette with length >= 1. Palette[0] will be the color of the shape.
Also improves pixels per second when skipping areas that aren't covered by the shape.
Related to and motivated by #5084