almindor / mipidsi

MIPI Display Serial Interface unified driver
MIT License
125 stars 48 forks source link

Add direct pixel writes from u8 buffers #143

Closed almindor closed 1 month ago

almindor commented 1 month ago

Related to #142 this change will allow direct writes of &[u8] slice data via the display-interface without needing to do pixel format conversion in cases where the user knows it is safe to do so (e.g. prepared image data).

The hope is that this can get paired up with a change in embedded-graphics that enables such operations on a higher level (e.g. draw_image)

rfuest commented 1 month ago

I think this is a valid workaround until we have something better available in the future. We just need to make sure to document possible issues a user could run into:

almindor commented 1 month ago

I think this is a valid workaround until we have something better available in the future. We just need to make sure to document possible issues a user could run into:

  • The user is responsible for getting the endianness right
  • We should document that the ranges from sx to ex and from sy and ey are inclusive (this also applies to the existing set_pixels)
  • Adding bounds checking for the coordinates wouldn't be a bad idea. IIRC there has been some confusion in the past where a user tried to use display.set_pixels(0, 0, 320, 240, colors) instead of the correct display.set_pixels(0, 0, 319, 239, colors). Without an error this problem isn't easy to catch.
  • The method won't work with a 16bit display-interface-gpio, because it pads the each byte to a u16 instead of converting each two byte chunk into a u16. (https://github.com/therealprof/display-interface/blob/8fca041b0288740678f16c1d05cce21bd3867ee5/parallel-gpio/src/lib.rs#L267)

Sounds good, I'll add the documentation points. Do you plan to add the draw_image to embedded graphics? I'm wondering if I should wait for that so it's linked properly.

rfuest commented 1 month ago

Do you plan to add the draw_image to embedded graphics? I'm wondering if I should wait for that so it's linked properly.

I'm not sure when that will be added or how exactly it will be implemented. I would suggest to leave that part out and open an issue to remind us to update the comment after this has been added to e-g.