Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RAMRD (0x2E) implementation #4

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

tobozo
Copy link

@tobozo tobozo commented Dec 30, 2018

Adding readPixel(x, y) and readPixels(x, y, w, h, buffer) so screenshots become possible.

/!\ It seems to only work at 16MHz otherwise palette corruption occurs

Copy link
Author

@tobozo tobozo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The readPixels(int16_t x, int16_t y, uint16_t w, uint16_t h, uint16_t *block) method needs startWrite and endWrite otherwise the color palette gets corrupted.

@@ -114,6 +114,50 @@ uint32_t WROVER_KIT_LCD::readId() {
return r;
}


uint16_t WROVER_KIT_LCD::readPixels(uint16_t *colors, uint32_t len) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are returning 16 bits from 32bit value

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh bummer, fixed that quick :D

}


uint16_t WROVER_KIT_LCD::readPixel(int16_t x, int16_t y) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method is redundant, or add uint16_t readPixel() as a variant also

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if readPixels(x, y, w, h, colors) works like drawBitmap, and readPixels(colors, len) works like writePixels(color, len), then readPixel() should work like ... pushColor() ?

No startWrite, no setAddrWindow, just read the value where it is, right ?
but then it would need a wrapper, which brings us back to this previously removed code

image

should I restore this too ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using readPixel() alone without frequency change will result in a corrupted palette. In my opinion this is what it should look like to return non-corrupted colors, assuming a setAddrWindow() call occured earlier:

uint16_t WROVER_KIT_LCD::readPixel() {
    uint32_t freq = _freq;
    if(_freq > 16000000){
        _freq = 16000000;
    }
    uint16_t buf[1];
    startWrite(); // begin transaction
    readPixels(buf, 1);
    endWrite(); // end transaction
    _freq = freq;
    return buf[0];
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants