Rings and keywords

[NoobTracker]

It should be a little better now.

[marcelstoer]

@DaveSprague you previously fixed our circle drawing algorithms (twice actually). Can I invite you to take a look at these suggested improvements as well?

[NoobTracker]

That would be good because I figured out the ring algorithm more or less by guessing. However, it is relatively efficient, only one row of pixels is painted twice.

[DaveSprague]

Sorry, it would be helpful if I knew what issue these changes are intended to fix?

[DaveSprague]

Or what improvements these changes are providing

[NoobTracker]

These updates fix the problem that circles with fillCircle () are one pixel taller than wide. In addition, there are now functions for filled circles and the possibility to have the circles optimized so that only quadrants that are on the screen are drawn. But I don't know if that doesn't have disadvantages anywhere.

[DaveSprague]

Here's my take having just looked over the code for a bit. I don't have a hardware setup to do any tests at this point but I'll try to do some testing on it in the next few days.

It's not clear to me if there's much benefit of doing the "quads" optimization unless we have a use case that needs it where the performance gain is significant? It makes the drawCircle and drawFilledCircle code much more complicated and therefore more difficult to maintain.

I do like the idea of being able to draw rings but I wonder if it's worth having a separate function to do that compared to drawing a filled circle and then drawing a smaller inscribed filled circle inside it using the background color.

Another way to approach the issue in a more general way would be to have a version of the drawCircle algorithm, called drawArc, that, in addition to the center and radius, takes a starting and ending angle so that you can do arbitrary arcs of both non-filled and filled circles.

The ring feature is what really got me thinking about this since having the ability to draw a "gas gauge" would be quite nice for visualizing certain quantities.

Once you have the ability to draw arcs (non-filled, filled, and rings) with a start and stop angle, you also then have the ability to draw only the parts of the circle that are visible on screen if you want to optimize the performance along the lines of the quad feature.

So my suggestion would be to limit the changes in this commit to be just the ones that fix the filled circle to have equal height and width -- it's really great that you caught that. Have you tested it for circles of radius 0 and 1?. Although I have a very limited experience contributing to open-source projects, I would think it makes sense to keep pull requests focused on fixing one or a very few and related bugs or adding one or a few related features.

As a separate effort, perhaps you would like to look into adding drawArc and drawFilledArc functions that could be used to draw pie charts, gas gauges, etc. That's going to be significantly more complicated than the quad approach but it's a feature that's commonly supported in higher end graphics libraries and could be very useful for visualizing quantities on microcontroller/IoT applications.

[NoobTracker]

I tested it with different radii and it worked (for 0 I had to program a special case).
I am working on a kind of car race and with huge, long curves it takes a whole millisecond or more to draw them, although you can only see a maximum of two quadrants. In addition, the background of this game is a grid and it would be very difficult to draw the long lines for large circles, since you would paint the majority of the lines twice, once in the foreground color and once in the background color.
For example, you could turn the circuit program into a simple processing program, in which case no hardware is required. The circular algorithm does not use any special things like bit operations in the buffer, it only needs the possibility to draw pixels and horizontal and vertical lines.
There is an example (ring demo or something) that makes a ring jump around.

[NoobTracker]

I'm just noticing that the program is not compiling. Strangely, the program complains about an undefined reference to 'OLEDDisplay::drawHorizontalLine (short, short, short)', but has no problems with the vertical lines. I can't figure it out. In addition, the problem only occurs when you use fillRing(), but not when you create an instance.
I'm really confused.

[NoobTracker]

Fixed it.

[NoobTracker]

Is there still something wrong?

[marcelstoer]

Thank you Dave for the excellent feedback!

It's not clear to me if there's much benefit of doing the "quads" optimization unless we have a use case that needs it where the performance gain is significant? It makes the drawCircle and drawFilledCircle code much more complicated and therefore more difficult to maintain.

👍 I was asking myself the same question.

I do like the idea of being able to draw rings but I wonder if it's worth having a separate function to do that compared to drawing a filled circle and then drawing a smaller inscribed filled circle inside it using the background color.

👍 One function is enough

So my suggestion would be to limit the changes in this commit to be just the ones that fix the filled circle to have equal height and width -- it's really great that you caught that.

👍 Again, I agree - with both statements!

I would think it makes sense to keep pull requests focused on fixing one or a very few and related bugs

Absolutely. This one has a history 😜 #279, #280, #282, and #284 are all predecessors. @NoobTracker is getting familiar with authoring PRs. Kudos to your perseverance 👏

[NoobTracker]

The fillCircle function is time consuming and if used twice it would paint over everything in a smaller circle, which is very unattractive for certain purposes. With a huge ring that takes up a lot of screens but is only very thin, it would take a long time to paint everything twice.
My idea would be to add the option to fillCircle to specify a diameter for the inner circle that has the default value 0.

[DaveSprague]

Hi @NoobTracker,
I'm curious as to what resolution display you're using. My impression was that this 1306 library was mostly used quite small displays that are physically 1 - 2 inches in size and in the range of 128 pixel resolution or less in each dimension. Are you using this on a larger, higher-resolution display?

Also, you had mentioned making a race car game -- my impression is that this library is more targeted at simple IoT status display functions, etc. If you're main goal is using your code for a game where performance is the highest priority, you might consider using your own version of this library, tuned version of your game application and packaged with it, rather than trying to add those features into this code base which is not really targeted at high-performance game applications?

Dave

[NoobTracker]

Oh no, Google Translate translated my text poorly. I wanted to say that this program has advantages if you have a large ring that is wider than the screen is wide. I use normal 128x64 displays.
Google usually works well, but of course something always goes wrong if you slowly assume that it works and don't check everything anymore.
I now mainly use VGA when I want something with a screen. So my car race will probably end up being for VGA.

[marcelstoer]

My impression was that this 1306 library was mostly used quite small displays that are physically 1 - 2 inches in size and in the range of 128 pixel resolution or less in each dimension.

Correct, that's the hardware we target.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.