Support Embedded Graphics using the screen system call#579
Conversation
bradjc
force-pushed
the
embedded-graphics
branch
from
0a1bb6f to
8bc3eaf
Compare
This library implements the DrawTarget trait for the Tock screen syscall.
bradjc
force-pushed
the
embedded-graphics
branch
from
8bc3eaf to
0c1529a
Compare
|
Ok this is now updated with both demo apps and the Makefiles in each demo, giving examples of out-of-tree builds. |
|
I don't understand what change is leading to the CI failures. https://github.com/tock/libtock-rs/actions/runs/17407217165/job/49414698382, eg |
It requires a Tock runtime, so can't be run using normal host test.
| pub struct TockMonochromeScreen { | ||
| /// The framebuffer for the max supported screen size (128x64). Each pixel | ||
| /// is a bit. | ||
| framebuffer: [u8; (128 * 64) / 8], |
There was a problem hiding this comment.
Where is this max screen size coming from?
| use libtock::display::Screen; | ||
| use libtock_platform::ErrorCode; | ||
|
|
||
| pub struct TockMonochromeScreen { |
There was a problem hiding this comment.
Why is this monochrome-specific? The Screen isn't, is it?
There was a problem hiding this comment.
Something has to be responsible for the pixel format, and the intent is this screen struct is for 128x64 pixel monochrome screens with the Mono_8BitPage pixel format (because, well, that is what I need).
Would it be ok to keep the static buffer, fixed use case, and single size with an updated struct name?
There was a problem hiding this comment.
So the kernel screen driver at least partially handles this and the userspace is meant to either set the pixel format explicitly (command 26) or get the current pixel format (command 25) and adapt to it. The kernel, then, does a conversion from whatever the driver's pixel format is to the screen's native pixel format (some screens hardware does this, but for some it needs to be done in software). There is no specified default, it's just whatever the underlying screen uses as a default.
I guess this is fine if you explicitly set the pixel format somewhere. But it's not "good" and I don't think we should keep this long term.
There was a problem hiding this comment.
I guess this is fine if you explicitly set the pixel format somewhere. But it's not "good" and I don't think we should keep this long term.
I completely agree, but, it's a lot of pretty unglamarous work.
A reasonable step now would be to at least check that the kernel is providing a screen with the expected pixel format.
There was a problem hiding this comment.
Just call set_pixel_format somewhere for now.
| let (width, height) = Screen::get_resolution().unwrap_or((0, 0)); | ||
|
|
||
| Self { | ||
| framebuffer: [0; 1024], |
There was a problem hiding this comment.
In the C version, this is dynamically allocated. I understand why it might be better to statically allocate, especially with good support in Rust, but a fixed 1024-byte array kinda sucks.
Can it be parameterized by a const type parameter (perhaps with a default?) instead? At least as far as the kernel is concerned, the framebuffer can be arbitrarily smaller than the screen resolution (or larger technically, it's just a buffer). It would perhaps complicate draw_iter not to assume framebuffer size == resolution but...
Current implementation is tied to a monochrome, 128x64, 8bitpage screen. Make that explicit. Add some comments.
2 participants
This uses #568.
It provides basic support for using the embedded graphics library with the Tock screen syscall.
I was able to port the spin demo from libtock-c and verify that it works on the nrf52840dk with the attached monochromatic screen.