Thanks @peterhinch!

It's +444 bytes on PYBV11.

There's an issue when fill=0 and you specify coordinates such that it ends up drawing out of bounds because setpixel() doesn't have any checks. When fill=1, line() handles this. Interestingly if you move the check into setpixel() and then remove the check from line() and blit() then the size diff goes to +372. (perhaps this is preventing inlining of setpixel)

Out of curiosity, do you think there's a need to be able to draw ellipse arcs? I'm thinking maybe for something like a rounded rect, to draw the corners. This can be done in a later PR of course, unless it might mean we need to change the signature of the ellipse method. @rkompass

Thanks for the review.

I think the check should be done in setpixel. I suspect that inlining setpixel is probably unnecessary: in practical systems the bottleneck is likely to be transferring the framebuf contents to the display which typically takes tens of ms with fast SPI. Modifying setpixel, line and blit surely falls outside the scope of this PR. I could put a check in my code, but this would be pointless if you plan to move the check to setpixel. Please advise.

I have considered drawing arcs. The fill arg could be a bitmap defining what to draw and whether to fill. It wouldn't add much to the line drawn ellipse, a bit more for filled arcs. It's a worthwhile enhancement and I'm happy to do this if you guys are happy with the extra bytes. Again, please advise.

Incidentally the code format fail is puzzling me. Is there a tool I can run locally on the C code? I guess I should run black on the Python test script.

Incidentally the code format fail is puzzling me. Is there a tool I can run locally on the C code? I guess I should run black on the Python test script.

The diff showing the code formatting problems is printed in the build output: https://github.com/micropython/micropython/runs/7768326191?check_suite_focus=true

It's mostly missing whitespace e.g.: x,y --> x, y

I have prototyped arc drawing in Python and it's straightforward. This makes fill a bitmap as follows:
0 Fill if True
1 Quadrant 0 (x and y both positive).
2 Quadrant 1 (quadrants are counter-clockwise)
3 Quadrant 2
4 Quadrant 3
So to draw a filled full ellipse we would pass 0x1F or for a full line ellipse 0x1E. Perhaps a couple of global constants would help for these common cases?

As an indication of the added code here is the changed function:

def draw_points(ssd, cx, cy, x, y, fill, color):
    if fill & 1:
        if fill & 0x02:
            ssd.line(cx, cy - y, cx + x, cy - y, color)  # q0
        if fill & 0x04:
            ssd.line(cx - x, cy - y, cx, cy - y, color)  # q1
        if fill & 0x08:
            ssd.line(cx - x, cy + y, cx, cy + y, color)  # q2
        if fill & 0x10:
            ssd.line(cx, cy + y, cx + x, cy + y, color)  # q3
    else:
        if fill & 0x02:
            ssd.pixel(cx + x, cy - y, color)  # q0
        if fill & 0x04:
            ssd.pixel(cx - x, cy - y, color)  # q1
        if fill & 0x08:
            ssd.pixel(cx - x, cy + y, color)  # q2
        if fill & 0x10:
            ssd.pixel(cx + x, cy + y, color)  # q3

My view is that this is worth doing. Feedback appreciated!

I have pushed an update that enables quadrant selection and adds a pixel bounds check. The doc is updated but the tests need to be enhanced for the quadrant feature.

Now has updated test and is ready for review.

Thanks @peterhinch -- the quadrant solution is very neat and a great compromise on functionality vs size. I will review in more detail with @mbooth101 's polygon PR too now.

Some comments on performance.

I tested on an RP2 clocked at 250MHz with an ili9341 240x320x16bit display. A large display for a RAM based framebuffer. The theoretical time to refresh at 30MHz SPI is 40ms (in practice my driver takes longer as it translates 4 to 16 bit color on the fly). But the 40ms figure supplies context for rendering times.

The test drew a circle of diameter 200 on the display - something approaching a worst case size. I ran with filled and outline versions and timed the ellipse call:

        t = ticks_us()
        ssd.ellipse(110, 110, 100, 100, False, GREEN)  # ssd subclassed from framebuf
        dt = ticks_diff(ticks_us(), t)

Times were:

• Outline 304μs
• Filled 15.4ms

Observations: outline drawing is fast despite the pixel bounds check. Filled seemed slow, despite its use of the line draw code - but there are a lot of pixels to draw.

@jimmo I'll look at a rounded rect with a view to submitting a PR once this has gone through. I use rounded rects and circles in my GUI's so having these primitives in C will save time and bytes.

Wow, that was fast.
I was on holiday for a few days with my son and just got the e-mails showing the developments.
Also I'm not fluent with GitHub, but understand the code so far. As I tried to contribute some unfinished, or rather viper code on the polygon pull I will drop some thoughts here and ask one or two questions:

1. The algorithm by Zingl (that I referred to previously) for ellipse-drawing uses only 1 loop instead of two, thus has no need to distinguish between the cases y' > -1 and y' < -1. So I would expect it to be slightly more compact than the original (?) version by Kennedy used here (1 instead of 2 initializations for the loop). Speed should be the same.
   I could contribute a slightly changed version here within the next 6-14 days, if you are interested. A few bytes to save, perhaps.

2. The reason I started to program framebuffer graphics myself was a certain little problem: I wanted to draw a ring. A circle with a given line width > 1, so to say. With the circle code in @peterhinch 's impressing nano gui I observed that when I iterated through subsequent radii, say r= 17, 18, 19, 20 to draw a ring with r = 20 and ring width 4, some dots in the ring were left unpainted. This was not a bug at all but a consequence of the Bresenham algorithm used which is not made for this use case.
   Also Peter had not intended to draw rings.
   This observation/problem lead to the idea to generate not just the outer points of a circle but also the inner points (more precisely the outer points of the cut-out circle inside the ring). With two such generators synchronized the algorithm could draw horizontal lines, which speeded up the drawing of the ring considerably.
   Now my question: Does this phenomenon/problem exist for the present ellipse code? If you iterate over the half axes a and b to draw an ellipse with a certain width, will you not observe the same phenomenon of missing dots? Is this a problem of interest?
   My solution of this problem with synchronized generators for outer and inner (missing) ellipse would require the Zingl-algorithm btw., as it does the iteration through increasing y coordinate, and the switching of loops would complicate the thing.

3. I don't want to step on anyones toes. I learn how a C code for ellipses looks like here and the present solution seems to be pretty fast. And it is finished. So everything is fine. But if you are interested I could contribute a version that includes rings. Perhaps in another PR. Later. I'm certainly not as fast as you guys.:-) Would be nice to have some feedback on this. If it's not necessary and complicates things then tell me. I also have other projects running.

4. @peterhinch I suggest you have a look at my rounded rectangle viper code, as it's working and the algorithm is pretty clear. Could save you some hours work perhaps, if you are not finished with that too already.

I have working rounded rectangle code. The algorithm is trivial if you leverage existing code. For a filled rrect you need three filled rectangles and four filled circle quadrants. These can be rendered fast with existing code. For a line drawn rrect it's four lines and four circle segments.

Re drawing rings, all framebuf line drawing primitives draw single pixel lines. For most purposes a ring can be drawn by overlaying one circle with a smaller one. This isn't elegant but with our ellipse code even a large filled circle can be rendered in ~600μs. So I won't be pushing for a specific ring primitive. That said, I'm not a maintainer. A PR for a ring drawing primitive might well find favour and I won't argue against it.

Now superseded by #9045.