Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display abstraction interface #1644

Open
dotMorten opened this issue Aug 20, 2021 · 15 comments
Open

Display abstraction interface #1644

dotMorten opened this issue Aug 20, 2021 · 15 comments
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-displays LED, LCD, Canvas Class, Drawing, Fonts enhancement New feature or request Priority:2 Work that is important, but not critical for the release

Comments

@dotMorten
Copy link
Contributor

I've noticed all the various displays have different methods exposed to work with them. I'd like to suggest an interface (or abstract base class) is provided for all of them, so we can write common code, and quickly swap out the display used in our code, without having to change anything but the display creation.
I've found that I'd often change displays to something better, and having rewrite quite a lot of code because of it, and it would also make it easier for people to adapt existing code and apps for the display they have.

For instance it's pretty typical that there's a method for SendBitmap(Bitmap), ClearScreen() and FillRect(Color,Rectangle)`, and it would be nice to be able to write display-agnostic code against all of these. It would also allow us to build a common text and line drawing API for all of these.
The interface would also need a few getters to get display info like width/height, bit depth, etc, information about whether there's on/off control (and if so methods for doing that).

Describe the ideal solution

Suggestion for interface

public interface IDisplay
{
     void SendBitmap(Bitmap bmp);
     void FillRect(Color color, Rectangle rect);
     void ClearScreen();
     Task ResetScreenAsync();
     void SetDisplayOn();
     void SetDisplayOff();     
     bool IsDisplayOnOffSupported() { get; } 
     ushort Width { get; }
     ushort Height { get; }
     byte BitsPerPixel { get; }
     bool IsColorDisplay { get; }
}
@dotMorten dotMorten added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Aug 20, 2021
@ghost ghost added the untriaged label Aug 20, 2021
@raffaeler raffaeler added area-displays LED, LCD, Canvas Class, Drawing, Fonts enhancement New feature or request labels Aug 22, 2021
@pgrawehr
Copy link
Contributor

I fully agree with the requirement. Additionally, there should be an implementation for ICharacterLcd for graphic displays, so they easily support printing text. This can probably be done in a generic way, by providing a class that implements ICharacterLcd and outputs a bitmap. Oh, but note that we should not be using System.Drawing.Bitmap any more, but the ImageSharp equivalent instead (see #1403).

And then, it would be great if we had an OS-independent way of taking a screen shot of the main display, so that one could do things like mirroring (part of) the main screen to a SPI display. Looking for the linux way of doing that didn't really bring up many solutions for me yet. [This is independent, but would make these displays a lot more useful]

@raffaeler
Copy link
Contributor

I am also favorable to the proposal but I have a couple of observations.

The name of the interface is ambiguous. Displays based on the Hitachi interface can't do most of the thing specified in the interface. I would rather split text and graphics display methods. They could eventually be implemented both on most graphical devices.

Also, given the amount of devices that can display something, it would be better providing a method to return its capabilties. This could avoid the mess of catching the NotImplementedException.

@richlander
Copy link
Member

It would be good to look at some of the display options at https://www.daktronics.com/ and see if this interface (or any we propose) would satisfy the style of displays that those folks provide.

@dotMorten dotMorten mentioned this issue Sep 13, 2021
@krwq
Copy link
Member

krwq commented Sep 14, 2021

@dotMorten we've already had a long discussion about similar interface in the past. See conversation at #189 - I'm fine with continuing the conversation after taking all points in there and this thread into consideration. Please do not block your PR on this. If you want we can schedule a call to share all our past points why we didn't ship anything like that yet and what should we take into account and do initial review - ideally if you find an existing abstraction inside ImageSharp which you could use in our bindings that would be best - I'd personally encourage us to not try to solve problems in this repo which are out of scope for it (our main goal is to provide bindings for different devices and not be a generic purpose library for anything related to IoT) - having said that most of the people can appreciate good and consistent design (which implies abstraction) but best if we didn't design it ourselves and rather reused something existing.

@maloo
Copy link
Contributor

maloo commented Apr 28, 2022

Maybe we could take inspiration from OpenWF Display spec that I helped write a few years ago. It is written specifically with these type of embedded display HWs in mind. https://www.khronos.org/registry/OpenWF/

Some comments on proposed API.

  • Allow buffer (bitmap) to be created by display, this allows implementation to do buffer flipping instead of a forced bitmap copy as proposed.
  • Remove fill and clear, those are bitmap/buffer operations.
  • BitsPerPixel should be an enum, otherwise you can't diffentiate between RGB16 and BGR16 for example, or RGB18 packed as RGB24 etc.
  • IsColor should be a property of pixel format.
  • Display should list supported formats. This will be a combination of the display interface and the display.
  • You might want a SetDisplayMode instead of SetOn/Off since some displays have partial modes.

@maloo
Copy link
Contributor

maloo commented Apr 28, 2022

Old comment of basing bitmaps/buffers on Memory seems viable still, ImageSharp supports that too.
https://github.com/SixLabors/ImageSharp/blob/main/src/ImageSharp/Image.WrapMemory.cs#L44
This should allow wrapping existing buffers like Linux frame buffer and use IDisplay to "flip it" and render on it using ImageSharp or just simple basic Span rendering.

@krwq
Copy link
Member

krwq commented May 4, 2022

@maloo would you be interested in driving this issue? Most of us are not graphics experts by any means, would you be interested perhaps in making some short presentation or just proposing specific APIs/changes? (or both) We can review that during one of our triage meeting. Note that what we care about a lot is possibility to port this code to nanoFramework so less complexity makes it easier

@maloo
Copy link
Contributor

maloo commented May 4, 2022

I would love to, but as everyone else I have many other higher prio stuff. I was planning to do this for a product a few years ago when I started using dotnet iot, but I ended up using neopixels instead for the interface and that I already pushed.
But maybe we could have short teams call and discuss what you would need from me?

@richlander
Copy link
Member

Just want to put on the table that we could also start with an existing API/implementation and morph it to what we want. There is a mid-point between "start from scratch" and "use an existing package".

@krwq
Copy link
Member

krwq commented May 5, 2022

@maloo I don't want to promise you a specific timeline here - I will bring this up today during our triage meeting. We currently have couple of other subjects going on so not sure when this will be picked up (Serial Ports, Firmata, nanoFramework)

@maloo

This comment was marked as off-topic.

@pgrawehr

This comment was marked as off-topic.

@krwq
Copy link
Member

krwq commented Sep 22, 2022

[Triage] Blocked on #1767

@krwq krwq added blocked Issue/PR is blocked on something - see comments Priority:2 Work that is important, but not critical for the release labels Sep 22, 2022
@krwq krwq removed the blocked Issue/PR is blocked on something - see comments label Aug 31, 2023
@krwq
Copy link
Member

krwq commented Aug 31, 2023

[Triage] not blocked anymore

@krwq
Copy link
Member

krwq commented Aug 31, 2023

assigning to @pgrawehr to reiterate on the interface design and figure out next steps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-displays LED, LCD, Canvas Class, Drawing, Fonts enhancement New feature or request Priority:2 Work that is important, but not critical for the release
Projects
None yet
Development

No branches or pull requests

7 participants