Integrate TilePaletteMapper and TileGrid further #10318
Conversation
…onal on TileGrid init, make TPM mark single tile dirty instead of whole TileGrid when mapping is changed.
There was a problem hiding this comment.
I understand there is a circularity to this that needs to be broken. I'd do it the other way so TG always has a pixel_shader. Allow TPM to be constructed without allocating the mapping memory and "bind" the two when the TG is created. This can also check that multiple TG aren't given the TPM. (Or it could track multiple TG to mark dirty.)
| @@ -21,33 +22,38 @@ | |||
| //| bitmap with a wider array of colors.""" | |||
| //| | |||
| //| def __init__( | |||
| //| self, palette: displayio.Palette, input_color_count: int, width: int, height: int | |||
| //| self, palette: displayio.Palette, input_color_count: int, tilegrid: displayio.TileGrid | |||
There was a problem hiding this comment.
I wonder if it makes more sense to reverse this and make TPM not take width, height or tilegrid. That way you could create the TPM and pass it to a TG. The TG can then init it. That makes it fit better within the pixel_shader model I think.
| if (tilegrid->pixel_shader == mp_const_none) { | ||
| mp_raise_ValueError(MP_ERROR_TEXT("TileGrid must have a pixel_shader")); | ||
| } |
There was a problem hiding this comment.
We wouldn't need this if TPM could be constructed without a TG.
It looks fine to me. I'm not sure why Weblate didn't do it but it seems to be active. Updating these here is fine. |
This makes the following functional changes:
tilegridas an argument instead ofwidth, andheightpixel_shaderargument to be None or omitted. But_add_layer()now validates that it it isn't None before it can be added to a Group to be shown on the display.displayio_tilegrid_mark_tile_dirty()function that gets called byset_tiledisplayio_tilegrid_mark_tile_dirty()to mark individual tiles as dirty when the mapping for that tile is updated, rather than causing the whole TileGrid to be refreshedI have tested the updated TPM functionality with the Matrix CircuitPython code (updated to use the new API), and with the text editor that I am working on for Fruit Jam OS, it uses TPM to highlight the bottom bar with hotkeys in it. All functionality appears to work as expected.
I have not specifically tested / validated the smaller dirty area refresh by ensuring that it speeds up refresh rate when single mappings are updated. That will confirm that it isn't updating the full TileGrid any longer, I will do that once I get the cursor changed to use TPM in the text editor.
I noticed that this change is showing some edits to the .pot file that I did not knowingly make. I ran pre-commit locally and it seems to have made some of those changes even though they appear unrelated to TileGrid or TilePaletteMapper. I'm not sure if that is a git or update / merge issue or something else. Let me know if I need to do anything further in that file though.