rm-hull / luma.oled

Python module to drive a SSD1306 / SSD1309 / SSD1322 / SSD1325 / SSD1327 / SSD1331 / SSD1351 / SH1106 OLED
https://luma-oled.readthedocs.io
MIT License
805 stars 161 forks source link

Problem about SSD1351 device color format converting #218

Closed Dreagonmon closed 6 years ago

Dreagonmon commented 6 years ago

Class: luma.oled.device.ssd1351 Funtion: display

Origin:

for r, g, b in self.framebuffer.getdata():
    if not(r == g == b == 0):
        # 65K format 1
        buf[i] = r & 0xF8 | g >> 5
        buf[i + 1] = g << 5 & 0xE0 | b >> 3
    i += 2

This code try to convert color RGB(888) to color 65K(565) Convert: rrrrrrrr,gggggggg,bbbbbbbb -> rrrrr ggg,ggg bbbbb when deal with the green byte, bit 76543210 should be convert to 765,432 but actually we get 765,210, the green color changed randomly.

The correct code is:

for r, g, b in self.framebuffer.getdata():
    if not(r == g == b == 0):
        # 65K format 1
        buf[i] = r & 0xF8 | g >> 5
        buf[i + 1] = g << 3 & 0xE0 | b >> 3
    i += 2

green byte just need to left shift by 3, not 5

rm-hull commented 6 years ago

@Dreagonmon yes, I think you're right. good catch!

While the code fix is quite straightforward, the regression test could be a bit hairy to modify.

If you fancy making the change and submitting a PR that would be appreciated - if not, I will try and get it done this week sometime.

[The tests can be run on a (non-raspberry-pi/laptop/desktop) linux or OSX machine with tox (install with pip)]

rm-hull commented 6 years ago

also note that SSD1331 driver is similarly affected

thijstriemstra commented 6 years ago

what's the tldr of this bug: basically not seeing correct green tones?

rm-hull commented 6 years ago

the bug is very very subtle and has virtually negligible effect: basically rather than having 2^6 (=64) shades of green taken from the 6 most significant bits, we have 64 shades of green taken from the 3 most significant bits and the 3 least significant bits. So there might be some odd green-color shifting.

The tests pass locally because the reference test image use solid colors. I thought we might have to re-generate those huge json calls files, but fortunately not...

rm-hull commented 6 years ago

2.5.1 released onto PyPi with this fix in