Triangle, circle ellipse and rounded rectangle

[rkompass]

Hello Peter,

thank you first for this great nano-gui project.
Based on it I got my 160x128 st7735-based LCD running.
And it's really fast and takes only 11K of RAM with 4-bit color-mode. I'm impressed.
First I had to correct the _init() functions in st7735r.py and st7735r4b.py a bit.
Then I started to play around. I was disturbed by the circle() function in nanogui.py when applied with e.g. width=10 . It left dots uncolored. So I started to find out what your algorithm was and began to read about Bresenham algorithms.
In the end I programmed triangle, circle, ellipse and rounded rect basically with the same approach: Generator functions deliver (x,y)-pairs for the left and right boundary and hline()s are drawn between them.
This has the advantage that the outline of filled shapes agree with the corresponding lines of shapes drawn unfilled.
I put all this in one file. Hope you are interested. Would be nice to hear (read) your opinion.
Perhaps nano-gui can profit from it.
As I'm not familiar with github yet I cannot make a pull-request. I will instead drop this file here.
Another question: Would you prefer this being discussed on the micropython forum?
Cheers,
Raul

st7735r4b_mx2.py.zip

[peterhinch]

Thank you for the code.

Regarding the device driver there are many ST7735 variants, the driver works with the Adafruit display but may well need adapting for other hardware. Users of unsupported hardware need to undertake this themselves (as you have). I can only support hardware I possess.

As for circle, triangle etc, I think these should be handled in C. Coincidentally I raised this point yesterday.

A key aim of nano-gui is small size. Barring actual bug fixes, the only changes I'm prepared to make are to add new widgets as these don't add to the size of the core code.

[peterhinch]

I think a set of graphics primitives for framebuf is a good idea. It's possible that the maintainers will not want this done as part of the firmware because of the added size. So a Python solution would be good.

In my view it has no real connection with nano-gui. The deliberately undocumented circle and fill_circle methods exist only to support widgets and polar graphs. As you point out circle doesn't work well with large widths; the algorithm is crude. In practice nano-gui only ever calls it with the default value 1, so I might save a few bytes by removing this option.

What I suggest you do is create a FramebufGraphics class which provides these additional graphics methods. This could be a subclass of FrameBuffer and could be used with or without nano-gui. A small change to the device driver would be needed so that it could export its buffer

def __init__(self, ...):
    self.mode = framebuf.GS4_HMSB  # or whatever
    self.buf = bytearray(...)  # Now a bound variable

def buffer(self):
   return self.buf, self.width, self.height, self.mode

Your color_setup.py would then do

ssd = SSD(spi, pcs, pdc, prst, height)  # Create a display instance
gfb = FramebufGraphics(*ssd.buffer())  # Your framebuf is mapped onto the same physical memory as nano-gui

You could then call existing FrameBuffer methods (gfb.line(...)) and your added methods (gfb.ellipse(...)).

An alternative implementation would be to use composition rather than inheritance with stub methods to access the existing FrameBuffer methods.

I think a FramebufGraphics class would be a useful thing to develop and to offer to the community. If you do this, I'll modify the device drivers to enable access to the buffer.

[rkompass]

Hello Peter,

Yes, let's do this. Was this not done already by someone else?
I compactified my functions a bit and posted them on yesterday discussion you linked to.

Question: Do I need self.width, self.height, self.mode at all?
In the drawing routines only self.hline() and self.fill_rect() are used. Am I correct that based on
self.buf, self.width, self.height, self.mode I would have to create another framebuffer object to use self.hline()?
Aah I see, I would then have to do super().__init__(buf, width, height, mode) as you do in __init()__.

I could initialize with gfb = FramebufGraphics(ssd) and access buf and mode if you make these bound variables (any costs?)
Alternatively I could draw with ssd.hline() and ssd.fill_rect() which seems simpler and you wouldn't have to change anything at all.
Also there would be no extra memory footprint of another framebuffer object because it is already there?
For speed I could assign the drawing functions to local variables.
What do you think?

[peterhinch]

I could initialize with gfb = FramebufGraphics(ssd)

I had a feeling when I posted that there was a better way. That is much simpler, using composition rather than inheritance as ssd is an instance.

...access buf and mode

Making these bound variables would be fine, but I'd caution against accessing buf. The only reason my original suggestion presented these variables was (as you spotted) to enable another FrameBuffer to be instantiated. Accessing buf to draw graphics brings in complexity because of the different modes. Further there is a risk of future new modes being added to the C code.

These issues can be avoided by using the FrameBuffer graphics primitives. I would start on that basis using my unmodified driver and see how you get on.

[rkompass]

Hi Peter,
in case you want to test it: Here is a first version of the class. It's in framebufgraphics.py.
I added a test_framebufgraphics.py function which I used for testing and profiling memory and timings,
a simple eventlog.py class which is used therein and my versions of your st7735 drivers,
which basically have the colors included in the file. (I love simple 1-file imports, saves my brain from too much considerations)
FramebufGraphics.zip

[rkompass]

The initializer is awful :-)

class FramebufGraphics:
    """
    Basic drawing methods extending micropython framebuffer
    """

    def __init__(self, disp):
        self.disp = disp
        self.show = disp.show
        self.fill = disp.fill
        self.pixel = disp.pixel
        self.hline = disp.hline
        self.vline = disp.vline
        self.line = disp.line
        self.rect = disp.rect
        self.fill_rect = disp.fill_rect
        self.text = disp.text
        self.scroll = disp.scroll
        self.blit = disp.blit

[peterhinch]

The initializer is awful :-)

Is there a better way of doing this? Another approach is a set of stub methods, but yours is more efficient.

I'll try your code over the weekend. The code looks good. One minor query: given that a circle is a special case of an ellipse, a small simplification would be to have the .circle() method call .ellipse(). Is there a performance reason to avoid this?

[rkompass]

Yes, that simplification is probably justified. Circle is only 7 % faster than an ellipse with a=b (=r). Perhaps I will conduct a test to see if the generator function in this case really exactly prduces the same pixels.
I startet to try out @micropython.viper. Apparently there is no way to produce return values? Having the generators as viper functions should speed up things a lot.
Do you know of a more detailed presentation/investigation of viper functions?

[peterhinch]

You can return values

@micropython.viper
def foo(a:int, b:int) -> int:
    return a + b

Viper is a bit of a black art but it is phenomenally fast. Type annotations are vital. I assume you've seen the doc. Beyond that the best approach is to experiment and to look at other people's code. You'll have found some of my efforts in the display drivers. There is a lot of Viper in this from Robert Hammelrath.

As with all optimisation it's best to put the effort into the code which is most performance-critical, but given what you're doing I guess that's most of it...

[RaulKompass]

Hello Peter,

I managed to rewrite circle, ellipse and rounded rect in Viper.
vfb_graphics.zip
The new methods have the same names with just a v in front, e.g. fbgr.vcircle(55, 52, 50, LIGHTGREEN, width=25).
The speedup is about 3.5 (If you use my eventlog class you have to subtract the timings for calling evl.log('message') itself).
I would be glad if you had the time to do some testing.

[RaulKompass]

Yesterday I did an investigation into timing.
Basically by changing width and height infbgr.vrounded_rect(10, 40, 100, 20, 10, CYAN, 10).
The about 450 us processing time consist of:

• ~ 115 us for the drawing of ~2000 pixels within framebuffer.hline()
• ~ 150 us for the 20 framebuffer.hline() function calls
• ~ 50 us for the viper-computations of Bresenham-algorithms
• ~ 12 us for the calling of the viper-routine _vfbg_rrec() which does these computations from fbgr.vrounded_rect()
• ~123 us for the calling of ordinary micropython fbgr.vrounded_rect() and packing the arguments for _vfbg_rrec()

I draw the conclusion that more than half of remaining the processing time (150+123 us) are due to limits in Viper:

• The limit to 4 arguments in Viper makes the introduction of an extra calling routine necessary
• conversions from viper to micropython make the framebuffer.hline() function calls unnecessarily slow
  The latter I investigated with e.g.

@micropython.viper
def v_sum(a:int, b:int) -> int:
    return a+b

@micropython.viper
def v_asum(n:int) -> int:
    sum = 0
    i=1
    while i <= n:
        sum = int(v_sum(sum, i))
        i += 1
    return sum

Even a call of a viper routine from within viper seems to have the micropython overhead which amounts to ~ 6 us here.
Viper could be a perfect tool to do low-level stuff like this in speed. Without using C. The possible speedup of ~8 against non-viper uPy seems sufficient and probably could not be greatly improved by pure C coding.
What is your opinion here? I would like to discuss this with others including Damien. Would it be better to do this in the forum or in Github?
Cheers,
Raul

[peterhinch]

Damien is rarely active in the forum.

The project's general policy is that if something can be done in Python it should be. This is to save firmware bytes which are an overhead on everybody. In my view given the levels of performance you're achieving, I agree with you that there's no real case for implementing it in C. I think it's best offered as an unofficial independent repo.

For an official decision you'd need to contact @dpgeorge or @jimmo.