Added support in GFXcanvas* classes to access pixel values

[michaelkamprath]

When using the GFXcanvas* classes as the image buffer for a hardware driver, there is a need to get individual pixel color values at given coordinates. Rather than sub- or client classes call getBuffer() and reinterpret the byte layout of the buffer, two methods are added to each of the GFXcanvas* classes that allow fetching of specific pixel values.

• getPixel(x, y) gets the pixel color value at the rotated coordinates in the image.
• getRawPixel(x,y) gets the pixel color value at the unrotated coordinates in the image. This is useful for getting the pixel value to map to a hardware pixel location. This method was made protected as only the hardware driver should be accessing it.

Also corrected some documentation on the GFXcanvas* classes so that they correctly describe how color values are being used in each of the 1-, 8-, and 16-bit modes.

[Tijgerd]

Seems to be good

[michaelkamprath]

Just curious, is there anything more I'd need to do to move this pull request forward to a yes or no resolution? I'd like to know if my own library can publicly depend on these changes.

[ladyada]

hiya we've got ~1200 repos so not all PRs get added especially if they are features rather than bugfixes. we'll keep open and take a look in the next few weeks :)

[michaelkamprath]

@ladyada sounds good. I just wanted to understand what happens next since this is the first time I've attempted a contribution to AdaFruit repo.

[michaelkamprath]

@ladyada have you had a chance to review this PR yet?

[siddacious]

@michaelkamprath I've taken a look and it looks good but some example code that uses these new features would help us test before merging.

[michaelkamprath]

@siddacious sample code can be found in my graphics library here. An example line where the new functionality I added can be seen here, which demonstrates the use case for getting the "raw pixel" unrotated so that the image data can be sent to the hardware.

Is this what you are looking for?

[michaelkamprath]

@siddacious & @ladyada Just checking in whether the sample code provided above was what you needed.

Please keep in mind that this change in now way should break any existing code. It keeps the same interface and functionality. All it does is expose the pixel data in each object so that the raw pixel data can be fetched by coordinates with out it going through the rotation mapping. With this change, pixel data can be completely encapsulated (thought he data buffer pointer is still available). I submit this is a safer thing for clients to do than grabbing the data buffer pointer and trying to decode the pixel data directly.

The benefits of this is that it removes the needs for a client to use the data buffer pointer directly if all they want is to get the pixel value at a specific, unrotated location. This sort of value fetching is frequently needed when trying to build the data stream to be sent to hardware. The sample code I point to shows this new feature being leveraged to build the bit-stream to be uploaded to the hardware controller for a LED device I designed.

Right now my hardware driver library does grab the data buffer pointer from the GFXcanvas object and directly interprets the memory block's content. But again, I do emphasize, this change should in now way affect any existing code, even if it does directly read the GFXcanvas object's data buffer. It just allows future users to use a better design paradigm with respect to the GFXcanvas objects.

[siddacious]

Apologies @michaelkamprath for some reason I didn't see your previous mention.

Adding an example to your PR is what I had in mind, both for testing and for helping other users understand and make use of the feature you're adding.

Thanks for your patience.

[michaelkamprath]

@siddacious Got it. Thanks. By "example", I am assuming you mean a *.ino file that goes into the /examples directory. Working on that now. Please tell me if you meant something else.

[michaelkamprath]

@siddacious I've added the demo as requested, but the check keeps failing on this clang step. I'm not sure why as the error message isn't too helpful. Looks to be a reformatting step, but I am not sure what is expected here. Any advice would be appreciated.

[michaelkamprath]

@siddacious @ladyada From the best I can determine, the clang reformatting step is reformatting just fine. Do you have any insight why it is failing?

Also, is the example code I added what you are looking for to make this pull request complete?

[ladyada]

yes please run clang-format (from clang8 + ) on your files :)

[michaelkamprath]

@ladyada @siddacious OK, I ran clang-format on all the files I am submitting. The git diff is much cleaner now (yay!). However, the clang check on your CI script still fails with the error of Process completed with exit code 1. and no other insight into what is wrong. Could someone please advise me what is going wrong here?

Also, is the example that I added what you are looking for as an example for the change I made?

[siddacious]

@michaelkamprath if you look here, you can see that clang-format still thinks it needs to make a change:
https://github.com/adafruit/Adafruit-GFX-Library/pull/237/checks?check_run_id=378595316

Not sure why your run didn't catch that, but if you mimic the change it's making it should pass.

I'll take a look at the example now.

[michaelkamprath]

@siddacious Thanks. I missed it because I wasn't expecting clang-format to not be stable. The file checked in on fb5698d was passed through clang-format, so when it failed again, I assumed there was something else going on, such as you had a custom style. However, I ran that file a second time through clang-format, and it had yet more changes to make. I wasn't expecting that.

OK, now it's passing all the tests. I will risk my question: is the example that I added what you are looking for as an example for the change I made?

[siddacious]

@michaelkamprath awesome, thanks for the fix.

Still reviewing and testing but so far yes, this looks like a really nice example :)

[michaelkamprath]

@siddacious Something odd is going on here. If you look at this line in the diff, that does not exist in my code on my machine. That line should not be missing. Something like this happened earlier in this PR too. I am not sure what is happening, but I will try to fix it.

[michaelkamprath]

@siddacious @ladyada OK, this PR is once again ready to be reviewed. I fixed the bug mentioned above. It got introduced during my formatting debacle.

[ladyada]

thanks - we'll take a look at testing it! no ETA as we have 1300 repos :)

[michaelkamprath]

@makermelissa @ladyada Just checking in to see what the status is here on determining whether this pull request is a go or no-go. Thanks!

[makermelissa]

Sorry @michaelkamprath, I haven't had a chance to test yet. In general, PRs to GFX tend to be slower because they need additional testing since there are so many libraries that depend on this one.

[michaelkamprath]

@makermelissa @ladyada Just checking in to see what the status is here on determining whether this pull request is a go or no-go. Thanks!

[michaelkamprath]

@makermelissa @ladyada Just checking in to see what the status is here on determining whether this pull request is a go or no-go. I see you did integrate a number of pull requests recently. I would love to learn the disposition of this one. Thanks!

[ladyada]

hi, we have hundreds of repos and PRs that are currently pending so we cannot promise an ETA on this PR. we will do a sweep of this library to check on all PRs and issues. as tens thousands of people use this library, we have to be very cautious :)

[michaelkamprath]

@ladyada Understood. Since it's been 10 months since I originally opened this, I just want to ensure this doesn't fall through the cracks, so please forgive the occasional bumps. Given that you have been incorporating PRs into this project, it looks like Ihave to once again rebase from master to make this PR mergeable. I'll get that done today.

[makermelissa]

Tested on an SSD1351. I tried out graphicstest to make sure nothing broke and that appeared to be fine and then ran the included demo and that worked fine, so I'm good with approving this. Thanks.

[michaelkamprath]

Thanks!