fredizzimo / infinity_ergodox

Unofficial keyboard firmware for Input Club's Infinity Ergodox
GNU General Public License v2.0
61 stars 16 forks source link

Add bitmap render #11

Closed seancaffery closed 1 year ago

seancaffery commented 8 years ago

Adds IC logo to start up animation. Similar to what the default firmware displays.

I've made it compatible with the format the default firmware uses so people could use the scripts in that repo to generate the data if they like. I'm not sure how important that compatibility is, if at all.

I'd to get some thoughts on this implementation and if it's something that you would like. Not sure if this functionality should live in the visualizer instead.

fredizzimo commented 8 years ago

I think adding the display of an image is a great addition to the default example visualizer. However I'm not that happy with your implementation. Allthough the Kll firmware uses "pages" there's no need for that, a simple x/y loop would be enough(128x32). The reason for the pages in the KLL, is that it talks directly to the hardware, while here we have a display abstraction.

But even better than writing the manual loop would be to blit the image directly, with gdispGBlitArea, or gdispBlitArea, if you don't want to specify the display number.

Also your bitmap is stored in the reverse order, while the display naturally should use top to bottom, left to right, for both bits and bytes. I found this program for converting images. So I recommend that you generate the c array with that instead. Or possibly some other software.

It would also be nice if the progress of converting images was documented, so that other people know how to do.

Edit: Sorry I did not read your description well enough. But no I don't think we should try to be compatible with the original in this regard.

seancaffery commented 8 years ago

I originally had it rendering pixels as you describe but I changed my mind :|

I'll look into using the display API.

seancaffery commented 8 years ago

@fredizzimo I've updated to use gdispGBlitArea and added a short description of how to generate the required image data

fredizzimo commented 8 years ago

I will look a bit closer into this when I have more time, maybe later today, or during the weekend.

The display is not supposed to use the RGB888 format, and therefore the blit function should take an array in monochrome format.

Until the reason is found for this, I won't accept the pull request.

Other than that everything looks good. You could perhaps use the gdispBlitArea function instead of the gdispGBlitArea, in that case, the GDISP parameter would get removed, as would the last two zeroes. Also at least when the format of the array is right, then I think it could be formatted as shorter lines, 16 bytes each, so that each line would represent one line on the display. So in total there would be 32, 16 bytes lines.

fredizzimo commented 8 years ago

I checked the Ugfx code, and it seems like either the documentation or the implementation is wrong. The documentation says this:

The bitmap is in the pixel format specified by the low level driver.

And the low level driver, st7565ergodox, specifies GDISP_PIXELFORMAT_MONO, so if the documentation is correct, then the source data should be in monochrome format. But in reality it only works if the driver supports hardware blitting. If software blitting is used, then there's a forced conversion from the global color format, since it internally calls gdispDrawPixel. We specify the global color format as RGB888, since the LCD backlight is RGB, and the LEDs are Grayscale, so monochrome is not enough.

The workaround is to implement blitting for the driver, so that it doesn't do any conversion. I will probably do that during the weekend. I rather do that, than to have all the images specified in the RGB888 format, since the memory is quite limited(256 KB, including the program code). A full 128x32 image in RGB888, is 12kb, while a monochrome is 512 bytes.

I will probably also report this bug to the Ugfx team.

seancaffery commented 8 years ago

Cool, thanks. I was wondering about that. RGB888 seemed way too large to me.

fredizzimo commented 8 years ago

Sorry for the delay. I have now added hardware blits, so you could try out the new image format, which should be monochrome.

Could you also please arrange the byte data in a format of 8 x 32, so that each row would represent a physical row on the screen. That makes it somewhat possible to see the image from the data already. It also makes things easier to read, when the lines are not too long. The LCD image converter generates an array like that by default at least for me.

Also remember to specify the array format as uint8_t, instead of pixel_t, but you need to add a cast to pixel_t when passing it to the blit function, otherwise there will be a compiler warning.

Edit: you could also display the image for a bit longer and shorten the TMK Infinity Ergodox display time. Maybe 2.5 seconds for the image and 2.5 seconds for the TMK Infinity Ergodox text