microsoft / pxt-brainpad

Microsoft MakeCode editor for the GHI Brainpad
https://makecode.brainpad.com/
Other
3 stars 11 forks source link

test screen #45

Closed pelikhan closed 6 years ago

pelikhan commented 6 years ago

I somehow managed to loose my brainpad board. Please try out the screen rendering on the latest build.

let y = 0
let x = 0
forever(function () {
    x = Math.randomRange(0, screen.width - 1)
    y = Math.randomRange(0, screen.height - 1)
    screen.fillRect(x, y, 4, 4, 1)
})
greg-norris commented 6 years ago

We tested the code, but it didn't work on the hardware side. We suspect in looking at the code here https://github.com/Microsoft/pxt-brainpad/commit/e8f2e948f4e6a4f4ba77c03ebdab65894c281ced, is that lcd.InitScreen(); is never called. That may be the issue. We could possibly add it in the constructor so it's automatically called when created.

pelikhan commented 6 years ago

Added InitScreen.

pelikhan commented 6 years ago

I've somehow lost my brainpad(s). Any chance to send a few more?

greg-norris commented 6 years ago

We tested the code after the change. The files deploy, but the screen now just displays garbage. The BrainPad's we're overnighting have the test program on them. These also have our latest boot-loader. You still launch the loader the same way(holding 'reset' and pressing the 'L' button), but now the LED is illuminated when in boot-loader mode, and blinks as the file is transferring. Once the file transfer is complete, the LED turns off and the BrainPad automatically resets and runs the code.

pelikhan commented 6 years ago

Cool. I'll wait for them then... How do you expect the bits to be packed in the bytes for the screen rendering?

greg-norris commented 6 years ago

FedEx 780056421660

greg-norris commented 6 years ago

8 vertical rows row of 8 pixels each. I'll find a sample function.

pelikhan commented 6 years ago

Your C++ layer should interpret the byte buffer in the same way as in javascript. @mmoskal, how are color packed for a 1bpp screen?

greg-norris commented 6 years ago

Here is an example function that sets one pixel on the BrainPad

void SetPixel(int x, int y, bool set) {
    if (x >= 0 && x < 128 && y >= 0 && y < 64) {
        int offset = (x + (y / 8) * 128) + 1;
        int bit = 1 << (y % 8); 

        if (set) {
            buffer[offset] |= bit;
        }
        else {
            buffer[offset] &= ~bit;
        }
    }
}
mmoskal commented 6 years ago

I'm afraid we need it the other way - the bits would go on the X axis first, not Y, and the most significant bit goes first on the screen. That is to say, 0x40 in the first byte refers to pixel at (x=1,y=0).

see https://github.com/Microsoft/pxt-common-packages/blob/master/libs/screen/image.cpp#L142 and https://github.com/Microsoft/pxt-common-packages/blob/master/libs/screen/image.cpp#L55 (and forget about the offset of 3 it's for our header)

gus-ghielec commented 6 years ago

We can definitely convert the data before we send to the display but this will slow things down. I know @pelikhan wanted this to be as fast as possible.

May I ask what was the reason for selecting this format? Every 1BPP display I came across has been the same as what we have now and this is why we selected this format. By doing so, we simply send the buffer without any conversions.

pelikhan commented 6 years ago

@mmoskal

mmoskal commented 6 years ago

The reason we picked that is because this is how you write the image in binary:

00110100
01001010
01110101

does the display driver chip let you swap x and y? If so, then it's only a matter of swapping bits in bytes which you can do with a lookup table

gus-ghielec commented 6 years ago

Sadly, this display and most I have looked at before don't have such option. If you are lucky, there is a mirroring feature but that is about it. I believe you guys will face this issue with every 1bit display.

We can do the conversation on our end with performance penalty. Over 8000 pixels need to be moved with every flush.

mmoskal commented 6 years ago

The two color displays I checked (ST7735R and SSD1351) had the option to swap x and y (which sort of makes sense, because it's unclear how you want the screen mounted).

I don't think the conversion will be all that problematic - the display is connected over I2C, I assume something like 200kHz? And the CPU is around 50MHz. Even if you do conversion at 10 cycles per bit, you are still getting around 2.5% of CPU load when pushing data at max speed over I2C.

Out of curiosity, which display controller are you using?

gus-ghielec commented 6 years ago

I haven't looked at the datasheet you have but usually you can swap to rotate the display landscape to portrait, not to change the bit order. Maybe we are talking about the same thing? If you see the image and it is rotated 90 degrees then this is one thing but if the pixels are completely wrong then it is something else. Let us please just wait to see what the image looks like once @pelikhan fix the current issue first (bad pointer is my guess).

SSD1306 is what we use.

mmoskal commented 6 years ago

OK, so you can easily swap x/y. After that, just use a lookup table to swap bits in every byte. This should end up like 1/2 cycle per pixel, virtually nothing compared to the I2C slowness.

pelikhan commented 6 years ago

Let us please just wait to see what the image looks like once @pelikhan fix the current issue first (bad pointer is my guess).

Bad pointer?

gus-ghielec commented 6 years ago

The display shows garbage and not the random pixels we have in the test. Maybe we are not receiving the right pointer to the image buffer.

However, I just found a bug. We will fix it, plus add the conversion code to match what you guys have.

pelikhan commented 6 years ago

This is the line where I pass along the byte buffer.

https://github.com/Microsoft/pxt-brainpad/blob/master/libs/screen/screen.cpp#L20

gus-ghielec commented 6 years ago

Looking at PXT, this is what our pixel test function should look like. Can you compare to your code and give us thumbs up please? We will give you a render driver to accommodate this.

void SetPixel(int x, int y, bool set) {
    if (x >= 0 && x < 128 && y >= 0 && y < 64) {
        //original code: https://github.com/Microsoft/pxt-common-packages/blob/master/libs/screen/image.cpp#L55
        int offset = y * 8 ;
        offset += x >> 3;
        //original code: https://github.com/Microsoft/pxt-common-packages/blob/master/libs/screen/image.cpp#L142
        int mask = 0x80 >> (x & 7);

        if (set) {
            buffer[offset] |= mask;
        }
        else {
            buffer[offset] &= ~mask;
        }
    }
}
mmoskal commented 6 years ago

It should be offset = y * 16, otherwise looks OK

gus-ghielec commented 6 years ago

@mmoskal we have just done what you suggested and added the inverting lookup tables for individual bits... etc. Then after a few hours of work realized 2 show-stopper issues:

The current options are:

gus-ghielec commented 6 years ago

For the sake of speeding things up, we will just do the conversion and provide the code shortly. You guys can decide later.

pelikhan commented 6 years ago

Sounds like a plan.

pelikhan commented 6 years ago

You still launch the loader the same way(holding 'reset' and pressing the 'U' button),

I've tried this but I could not get the drive to show. The LED shortly goes green before going back to red.

greg-norris commented 6 years ago

Sorry you have to hold reset and press the LEFT button then release the reset. The green LED will light. I told you the UP button by mistake.

pelikhan commented 6 years ago

Ok, the LED light stays green but no drive shows up.

greg-norris commented 6 years ago

Hmm...we tested them and used the loader to add the test code you wrote before we shipped them. Let me check this out.

pelikhan commented 6 years ago

The program seems to crash immediately after sending image to the screen. Unlike CODAL example, we dynamically allocate the Display object -- so fields need to be zeroed as well.

Test program with v0.0.9 branch: https://makecode.com/_J4qR7v3dWcAq

gus-ghielec commented 6 years ago

This is the line where I pass along the byte buffer. https://github.com/Microsoft/pxt-brainpad/blob/master/libs/screen/screen.cpp#L20

Back to your original random pixel test. The reason I think the line above maybe passing the wrong pointer is because the display never changes even though we were drawing random pixels.

Also, I can't think of anything that would cause a crash on our end but I will take a second look.

pelikhan commented 6 years ago

Looks it's a problem on our side.

pelikhan commented 6 years ago

I've pushed a fix on our side but it still crashes. @gus-ghielec try allocating dynamically the brainpad model in your test since we do that in pxt. It has implications w.r.t. to field initializations.

pelikhan commented 6 years ago

I am seeing garbadge on the screen even without using the screen api.

forever(() => {
    lightbulb.setColor(Colors.Red)
    pause(100)
    lightbulb.setColor(Colors.Blue)
    pause(100)
})
pelikhan commented 6 years ago

... and it's working now :)

pelikhan commented 6 years ago

Sorry for the confusion, the error was on our side.

gus-ghielec commented 6 years ago

Great team effort. The display and bit formatting was the tricky side. The rest should be straight forward.