Adafruit_GFX::fillCircleHelper brief doesn't match method behavior, and a proposed resolution #482
Description
The method fillCircleHelper in the class Adafruit_GFX does not match the description of the method in the brief. Instead of having a mask for each quarter of a circle, it only has a mask for two halves, the left and the right. This is inconsistent with the drawCircleHelper method, which does have a mask bit for each quadrant. The implementation for fillCircleHelper appears to have been done specifically to support the method fillRoundRect. The existing fillCircleHelper method looks like this:
/**************************************************************************/
/*!
@brief Quarter-circle drawer with fill, used for circles and roundrects
@param x0 Center-point x coordinate
@param y0 Center-point y coordinate
@param r Radius of circle
@param corners Mask bits indicating which quarters we're doing
@param delta Offset from center-point, used for round-rects
@param color 16-bit 5-6-5 Color to fill with
*/
/**************************************************************************/
void Adafruit_GFX::fillCircleHelper(int16_t x0, int16_t y0, int16_t r,
uint8_t corners, int16_t delta,
uint16_t color) {
int16_t f = 1 - r;
int16_t ddF_x = 1;
int16_t ddF_y = -2 * r;
int16_t x = 0;
int16_t y = r;
int16_t px = x;
int16_t py = y;
delta++; // Avoid some +1's in the loop
while (x < y) {
if (f >= 0) {
y--;
ddF_y += 2;
f += ddF_y;
}
x++;
ddF_x += 2;
f += ddF_x;
// These checks avoid double-drawing certain lines, important
// for the SSD1306 library which has an INVERT drawing mode.
if (x < (y + 1)) {
if (corners & 1)
writeFastVLine(x0 + x, y0 - y, 2 * y + delta, color);
if (corners & 2)
writeFastVLine(x0 - x, y0 - y, 2 * y + delta, color);
}
if (y != py) {
if (corners & 1)
writeFastVLine(x0 + py, y0 - px, 2 * px + delta, color);
if (corners & 2)
writeFastVLine(x0 - py, y0 - px, 2 * px + delta, color);
py = y;
}
px = x;
}
}
and fillRoundRect like this:
void Adafruit_GFX::fillRoundRect(int16_t x, int16_t y, int16_t w, int16_t h,
int16_t r, uint16_t color) {
int16_t max_radius = ((w < h) ? w : h) / 2; // 1/2 minor axis
if (r > max_radius)
r = max_radius;
// smarter version
startWrite();
writeFillRect(x + r, y, w - 2 * r, h, color);
// draw four corners
fillCircleHelper(x + w - r - 1, y + r, r, 1, h - 2 * r - 1, color);
fillCircleHelper(x + r, y + r, r, 2, h - 2 * r - 1, color);
endWrite();
}
My proposed resolution looks like this:
/**************************************************************************/
/*!
@brief Quarter-circle drawer with fill, used for circles and roundrects
@param x0 Center-point x coordinate
@param y0 Center-point y coordinate
@param r Radius of circle
@param corners Mask bits indicating which quarters we're doing
@param delta Offset from center-point, used for round-rects
@param color 16-bit 5-6-5 Color to fill with
*/
/**************************************************************************/
void Adafruit_GFX::fillCircleHelper(int16_t x0, int16_t y0, int16_t r,
uint8_t corners, int16_t delta,
uint16_t color) {
int16_t f = 1 - r;
int16_t ddF_x = 1;
int16_t ddF_y = -2 * r;
int16_t x = 0;
int16_t y = r;
int16_t px = x;
int16_t py = y;
delta++; // Avoid some +1's in the loop
while (x < y) {
if (f >= 0) {
y--;
ddF_y += 2;
f += ddF_y;
}
x++;
ddF_x += 2;
f += ddF_x;
// These checks avoid double-drawing certain lines, important
// for the SSD1306 library which has an INVERT drawing mode.
if (x < (y + 1)) {
if (corners & 0x4)
writeFastVLine(x0 + x, y0, y + delta, color);
if (corners & 0x2)
writeFastVLine(x0 + x, y0 - y, y + delta, color);
if (corners & 0x8)
writeFastVLine(x0 - x, y0, y + delta, color);
if (corners & 0x1)
writeFastVLine(x0 - x, y0 - y, y + delta, color);
}
if (y != py) {
if (corners & 0x4)
writeFastVLine(x0 + py, y0, px + delta, color);
if (corners & 0x2)
writeFastVLine(x0 + py, y0 - px, px + delta, color);
if (corners & 0x8)
writeFastVLine(x0 - py, y0, px + delta, color);
if (corners & 0x1)
writeFastVLine(x0 - py, y0 - px, px + delta, color);
py = y;
}
px = x;
}
}
with fillRoundRect updated like so:
/**************************************************************************/
/*!
@brief Draw a rounded rectangle with fill color
@param x Top left corner x coordinate
@param y Top left corner y coordinate
@param w Width in pixels
@param h Height in pixels
@param r Radius of corner rounding
@param color 16-bit 5-6-5 Color to draw/fill with
*/
/**************************************************************************/
void Adafruit_GFX::fillRoundRect(int16_t x, int16_t y, int16_t w, int16_t h,
int16_t r, uint16_t color) {
int16_t max_radius = ((w < h) ? w : h) / 2; // 1/2 minor axis
if (r > max_radius)
r = max_radius;
// smarter version
startWrite();
writeFillRect(x + r, y, w - 2 * r, h, color);
// draw four corners
fillCircleHelper(x + w - r - 1, y + r, r, 0x04 | 0x02, h - 2 * r - 1, color);
fillCircleHelper(x + r, y + r, r, 0x08 | 0x01, h - 2 * r - 1, color);
endWrite();
}
This keeps the implementation consistent (between drawCircleHelper and fillCircleHelper) at the cost of fillCircleHelper being slightly less efficient (four calls to writeFastVLine instead of two). Also note the new OR'd mask values in the fillCircleHelper calls in fillRoundRect.
I apologize I couldn't do this as a pull request. My copy of the library isn't up-to-the-minute, and I discovered the issue as part of working on something tangentially related.