bernii / embedded-graphics-framebuf

Generic framebuffer implementation in Rust for use with embedded-graphics library
MIT License
26 stars 7 forks source link

Framebuffer as a contiguous buffer? #4

Closed MabezDev closed 2 years ago

MabezDev commented 2 years ago

The current implementation is not really friendly to DMA engines, in general, it prefers a continuous block of memory (unless you have one of those fancy 2D DMA engines). I also believe in general (not been benchmarked so, in reality, it may not be true) that a single buffer is better for performance reasons, I think you'll get double bounds checks using an array of an array, as opposed to a single buffer.

Thoughts on changing the underlying storage? Or providing a storage trait to customise the backing storage?

jounathaen commented 2 years ago

I tried the straightforward approach on this:

pub struct FrameBuf<C: PixelColor, const X: usize, const Y: usize>(pub [C; X * Y]);

but this can't be done in stable Rust until all issues in https://github.com/rust-lang/rust/issues/76560 are resolved:

error: generic parameters may not be used in const operations
  --> src/lib.rs:77:80
   |
77 | pub struct FrameBuf<C: PixelColor, const X: usize, const Y: usize>(pub [C; X * Y]);
   |                                                                                ^ cannot perform const operation using `Y`
   |
   = help: const parameters may only be used as standalone arguments, i.e. `Y`
   = help: use `#![feature(generic_const_exprs)]` to allow generic const expressions

From the comments, this doesn't look like it's going to happen too soon. However, maybe a runtime check could do the trick. I think I'll if that works.

pyaillet commented 2 years ago

Oh, I have something similar to this in my own fork

I added:

I was able to use it with a modified version of mipidsi (see https://github.com/pyaillet/mipidsi/blob/add-write-raw/src/lib.rs#L237 ) and make it work with esp-idf SPI Dma on ttgo-display and twatch-2020

I did not push on these because my approach seemed too naive to me, though if you think it can help. Feel free either:

jounathaen commented 2 years ago

I think there is not too much difference to my approach in general. But I think it is not a good idea to transmute in this crate. If C and u16 have a different size, the transmutation produces undefined behaviour. I'd rather give a user access to the fundamental data structure, so that low level fiddling with the data can be done if necessary but is not the crate's responsibility. I'm also not sure what the purpose of the AsWords trait is.

As I did also implement it as well, but I reworked a few other things as well, let me shortly sketch how to achieve the DMA access with my version:

let mut fb_data = [Rgb565::BLACK; 128 * 160];
{
    let mut fbuff = FrameBuf::new(&mut fb_data, 128, 160);
    Line::new(Point::new(2, 2), Point::new(10, 2))
     .into_styled(PrimitiveStyle::with_stroke(BinaryColor::On, 2))
     .draw(&mut fbuf)
     .unwrap();
}
my_spi_access(&fb_data);

or something like so, but I'm not 100% sure if the borrow checker likes this:

let mut fb_data = [Rgb565::BLACK; 128 * 160];

let mut fbuff = FrameBuf::new(&mut fb_data, 128, 160);
Line::new(Point::new(2, 2), Point::new(10, 2))
 .into_styled(PrimitiveStyle::with_stroke(BinaryColor::On, 2))
 .draw(&mut fbuf)
 .unwrap();

my_spi_access(&fbuff.data);
pyaillet commented 2 years ago

Ok, I will have to try this, hopefully next week, it seems really interesting.

Thanks :)

jounathaen commented 2 years ago

This is now possible as of https://github.com/bernii/embedded-graphics-framebuf/commit/c33cc45a0eb99c35ab8079a4988cb7436f88a958. Still, an example is needed.