ellipse and circle implementation

[OmarAli3]

No description provided.

[per1234]

This test sketch:

#include <ArduinoGraphics.h>

const byte canvasWidth = 11;
const byte canvasHeight = 11;

class ASCIIDrawClass : public ArduinoGraphics {
  public:
    ASCIIDrawClass() : ArduinoGraphics(canvasWidth, canvasHeight) {}

    virtual void set(int x, int y, uint8_t r, uint8_t g, uint8_t b) {
      // the r parameter is (mis)used to set the character to draw with
      _canvasBuffer[x][y] = r;
      (void)g;
      (void)b;
    }

    // display the drawing
    void endDraw() {
      ArduinoGraphics::endDraw();

      for (byte row = 0; row < canvasHeight; row++) {
        for (byte column = 0; column < canvasWidth; column++) {
          // handle unset parts of buffer
          if (_canvasBuffer[column][row] == 0) {
            _canvasBuffer[column][row] = ' ';
          }
          Serial.print(_canvasBuffer[column][row]);
        }
        Serial.println();
      }
    }

  private:
    char _canvasBuffer[canvasWidth][canvasHeight] = {0};
};

ASCIIDrawClass ASCIIDraw;

void setup() {
  Serial.begin(9600);
  while (!Serial);
  ASCIIDraw.beginDraw();
  ASCIIDraw.background('X', 0, 0);
  ASCIIDraw.clear();
  ASCIIDraw.stroke('*', 0, 0);
  ASCIIDraw.fill('O', 0, 0);
  ASCIIDraw.ellipse(5, 5, 10, 10);
  ASCIIDraw.endDraw();
}

void loop() {}

gives this output:

XXXXXXXXXXX
XXX*****XXX
XX*XXXXX*XX
X*XXXXXXX*X
X*XXXXXXX*X
X*XXXXXXX**
X*XXXXXXX*X
X*XXXXXXX*X
XX*XXXXX*XX
XXX*****XXX
XXXXXXXXXXX

Three problems:

• There is an extra "pixel" set at position (10, 5).
• Ignoring the extra "pixel", the ellipse is 9x9, not 10x10.
• No fill.

[per1234]

Why did you decide not to functionalize the shared code? I see from this comment that you started out to do that at one point.

[OmarAli3]

Just for reducing the number of function calling.

[per1234]

I'll leave that choice up to you and whoever does the final review on this, but I would request that you remove the commented out code, since it doesn't serve any purpose.

[per1234]

You are treating fill as an alternative stroke. The fill should fill the inside of the ellipse, so it needs to be done in addition to the stroke.

[OmarAli3]

fixed.

[OmarAli3]

Sorry still not fixed. I notice another bug with fill.
I am trying to solve it.

[OmarAli3]

@per1234
Can you please tell me what the exact output for the above seketch is needed to be ?

[per1234]

@OmarAli3 I would expect something that looks approximately like this:

XX******XXX
X*OOOOOO*XX
X*OOOOOO*XX
*OOOOOOOO*X
*OOOOOOOO*X
*OOOOOOOO*X
*OOOOOOOO*X
X*OOOOOO*XX
X*OOOOOO*XX
XX******XXX
XXXXXXXXXXX

I might not have the shape of the circle quite right, but your code seems to do that well enough already.

What I expect is a circle that:

• Is 10 characters wide by 10 characters high
• Has its inside filled with Os.
• Doesn't have an extra "pixel" on the right edge.

[OmarAli3]

After working on the problem :

• I have found that the problem of my code is that I calculate the exact "pixel" position in float then it's casted to integer to pass into set() that take only integers, so I suggest to make override set() that take floats and then whatever graphic display library that inherit this library should floor or ceil the "pixel" position in the way that fit its "pixel" size.
• I tested my code in c++ with Borland Graphics Interface library and found that it is working very well as the "pixel" size is very samll and also noticed that as the width and height increase the error disappear.

So now I have two choices:

• To work on the fill problem only and I guess that the "pixel" position problem will be fixed with real graphic diplay devices.
• To find a different approach that in some way it approximate the position values as integers.

@per1234 , Do You suggest something?

[per1234]

I guess that the "pixel" position problem will be fixed with real graphic diplay devices.

As far as I know, this library is currently only used by the Arduino_MKRRGB library (though I'm sure it will be used for many other applications soon). The Arduino MKR RGB shield is a 12x7 LED matrix, so it is actually less of a "real graphic display" in some ways than the Serial Monitor.

Do You suggest something?

Because of the use with the MKR RGB Shield, I do think it's important to avoid this extra pixel. That will be very noticeable when it's an extra LED lit up. I have confirmed the issue also occurs with using the Arduino_MKRRGB library and the Arduino MKR RGB shield.

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[leonardocavagnis]

Fixed by #30