drawCircleHelper and fillCircleHelper do nothing, fillCircleHelper corner argument ignored or misapplied

[ghost]

• Arduino board: Arduino NANO version 3

• Arduino IDE version: 1.8.10

• Other HW, SW and/or environment dependencies when bugs found:
  Win 10
  128x128 1.5-in OLED color display w SSD1351 chip, from Banggood, product 1384079
  SPI can be HW or bit-banged- the bugs are not related to the SPI interface or its speed
  Adafruit-GFX ver 1.5.6 and later
  Adafruit-SSD1351 version 1.2.2

Bugs found:

1. In Adafruit-GFX.cpp, the method drawCircleHelper has no effect:
   no pixels are written, in any color, in any quadrant
   Root cause:
   In Adafruit_GFX.cpp, version 1.5.6, about line 403:
   Missing startWrite() and endWrite calls around code.

2. Similarly, method fillCircleHelper has no effect: no pixels are written, in any color
   Root cause: same as above: missing startWrite/endWrite around code.

After fixing the above two problems, the following additional bugs became visible, when
trying to refresh/overwrite certain quadrants of a dial gauge:

3. In method fillCircleHelper
   The quadrant (corner) arguments are misapplied, produce incorrect results.
   For example,
   a command to fill quadrant 1 fills both 1 and 4
   a command to fill quadrant 2 fills both 2 and 3

To clarify quadrants, I define them as follows, for a circle with zero degrees on the right:
      quadrant 1 = sector from 180 degrees to 90 degrees
      quadrant 2 = sector from 90 degrees to 0 degrees
      quadrant 3 = sector from 360 degrees to 270 degrees
      quadrant 4 = sector from 270 degrees to 180 degrees

4. Also in method fillCircleHelper, there is a vertical artifact which the fill algorithm cannot remove. That is, in the example sketch, if the dial gauge succeeds in drawing a 90-degree needle, I cannot "erase" it (over-write it) with the background color using the fillCircleHelper method. I can only do so with a fillRectangle command. See the attached sketch.

Other testing done:
I checked later library versions, and, for my SSD1351 display and sketch, backward compatibility got worse, not better.

I have attached a sketch which triggers or demonstrates the above defects. It is called oled_AF_GFx_1_5_7_bugs.ino.

I have also attached a photo which shows #3 and #4 above.

Best regards, Tealok2

oled_AF_GFx_bugs_1_5_7.txt

[ghost]

I have solutions for the bugs articulated above, in the methods drawCircleHelper, fillCircleHelper.

While working out my solutions, two additional issues, in the same methods, became apparent:
5 - the application of the corner argument is applied differently in the two methods
6 - there is NO implementation for 2 of the 4 corner bits in fillCircleHelper

Here is my thinking about how to just articulate exactly what I mean by the above two bugs:

For convenience, I have mentally renamed the two affected methods drawSector and fillSector.
For clarity, I propose replacing the variable corner with quadrant.
For rigor, I propose sticking to the Cartesion convention for quadrant numbering:
- https://en.wikipedia.org/wiki/Cartesian_coordinate_system
-

For backward compatibility, I propose keeping the bit-map design evidenced in the code for drawCircleHelper, of 1 bit per quadrant.

For clarity and rigor, I propose the following bit-to-quadrant map:
- bit 20 maps to quadrant 1
- bit 21 maps to quadrant 2
- bit 22 maps to quadrant 3
- bit 23 maps to quadrant 4

With the above established, it is possible to articulate bugs 5 and 6 more precisely:
5 - in drawSector, bit 2**0 maps to quadrant 2, but in fillSector, to quadrant 1
6 - in fillSector, there is NO implementation for quadrants 3 and 4

Attached is a picture of my dial gauge after fixing bugs 1-4.

Best, @tealok2

PS: (to any AF responder)
Somewhere on the AF forum (python I think) I read about someone implementing CI. Can you please comment on whether this library (Adafruit-GFX-Library) is in a CI-directed performance and/or regression suite, to prevent degradation of compatibility over time?

It can be challenging, but from experience it is possible to keep a bunch of DUTs hooked up to an array of USB ports, and to upload code and test with every checkin... or at least every release.

Fully automating detection of pixels-out-of-place is beyond unreasonable, but you could do something really simple... just bolt a bunch of the LCDs and OLEDs that YOU sell (you DO have a non-trivial matrix of boards, architectures, libraries and chips/drivers, so don't try to guarantee what you DON'T sell), build and put up in the displays the examples you ship. Then task SOMEONE to go look, and notice if the examples are all screwed up, or appear normal. @tealok2

[ghost]

Disregard the question about CI, because I found a pull request rejected because of Travis errors.