tinygo-org / drivers

TinyGo drivers for sensors, displays, wireless adaptors, and other devices that use I2C, SPI, GPIO, ADC, and UART interfaces.
https://tinygo.org
BSD 3-Clause "New" or "Revised" License
607 stars 192 forks source link

Pixel/image buffers #617

Closed aykevl closed 10 months ago

aykevl commented 10 months ago

This PR refactors pixel buffer handling.

Summary

I'd like to add a new pixel package for efficiently working with image buffers for displays, and add a DrawBitmap(x, y, bitmap) method to send these image buffers directly to a SPI display. This is intended as preparation for DMA support in displays (StartDrawBitmap) and should replace DrawRGBBitmap8. I've already done lots of tests with this design outside the drivers package and I think it works well.

Details

I've added a new package to efficiently work with pixel buffers in various formats: tinygo.org/x/drivers/pixel. This isn't new code: I've been using it for a while in this package: https://pkg.go.dev/github.com/aykevl/tinygl/pixel. It's pretty fast, see for example the demos in this thread.

The basic idea is as follows:

Comparisons with other approaches:

The main reason I'm doing this is because I would like to add DMA support to most displays using a very similar method: StartDrawBitmap (and IsAsync / Wait). I figured it would be much easier to add this as a synchronous API and only do the async one once the synchronous one is agreed upon. I didn't want to use StartDrawRGBBitmap8 with a byte array - I think that's just not a great API and too easy to make mistakes with.

Next up:

Anyway, looking forward to feedback on this. Questions? Comments? Is this the direction we want to take with TinyGo graphics? I know generics might complicate things but I really think it's the best tool for the job here.

bgould commented 10 months ago

This looks really nice.

I might add a pixel.Gray1 (black/white) pixel format, for e-ink screens.

FWIW the sh1106 probably could use the above format as well: https://github.com/tinygo-org/drivers/blob/release/sh1106/sh1106.go#L203-L226

One thing that comes to mind for me is the thought that pixel.Image could possibly be "windowed" to provide sort of a virtual scroll buffer. I've thought such a thing might be useful for instance in tinyterm

For example, if defining a pixel.Image that has equivalent width but greater height than the display, and then tracking a "scroll offset" value, then it might be possible to allowing drawing off-screen and scroll it into view by adjusting the offset.

aykevl commented 10 months ago

FWIW the sh1106 probably could use the above format as well: https://github.com/tinygo-org/drivers/blob/release/sh1106/sh1106.go#L203-L226

I don't think I have this display, but yeah that looks like a place where it would be useful!

One thing that comes to mind for me is the thought that pixel.Image could possibly be "windowed" to provide sort of a virtual scroll buffer. I've thought such a thing might be useful for instance in tinyterm

You mean storing the entire contents of the scroll buffer in-memory? Usually there isn't nearly enough memory for that sort of thing. In fact, most boards don't even have enough memory for a single framebuffer, hence why SPI displays typically have a built-in framebuffer. (The reason why using a framebuffer works for a display like the sh1106 is because it's small and each pixel is only 1 bit, instead of 16 like for most LCD displays).

For TinyGl I have a slightly different approach: https://pkg.go.dev/github.com/aykevl/tinygl#ScrollBox Basically I will call the Update method of the contents (basically, the redraw function) with a separate (x, y) for the display and for the contents. That way, it'll redraw exactly the part of the contents that is needed. And it only needs small buffers, not the entire screen.

Related: https://hachyderm.io/@ayke/110554855872552126 (but with some patches I still haven't pushed due to bugs)

In short, I don't think adding a scroll offset is a good way to do this. You'll quickly run into memory issues on common MCU/display combinations and there are alternatives for getting the same scroll effect.

bgould commented 10 months ago

Ok thank you for explaining. The TinyGl mehod you mentioned is close to what I what thinking, but it makes sense that it at a different layer of the "stack".

aykevl commented 10 months ago

Made a few small changes, see the force push: 0135459...db73aad.

aykevl commented 10 months ago

@soypat found a pretty big oversight on my part: bounds checking for the Set/Get methods. I've added the bounds checks, and checked the performance difference:

deadprogram commented 10 months ago

Thanks @aykevl for this set of features and thank you @soypat for the indepth reviews/suggestions! Now merging.

sparques commented 6 months ago

@aykevl Apologies if this is not the right place to ask, but I was curious why do colors in the pixel package have an RGBA() color.RGBA method rather than RGBA() (r, g, b, a uint32) method? With the latter, they implement color.Color and it's much easier to use these memory efficient pixel formats with the standard library.