Tri color fix

[aaronr8684]

Resolves #58

Tested on the following screens:

• 7in5b_V2 - aaronr8684
• 2in7b_V2 - donbing

Could also be tested on:

• 2in7b - @txoof
• 5in83c - @dr-boehmerie
• 4in2c - aaronr8684 (pending delivery)

Potential issue:
@robweber Default value of '16' will work with yellow or red filters, but assigning a custom value higher will eliminate yellow and then red at an even higher value. Also at a value higher than 128, no color will be shown as it's currently written. We could hard code the img_color values for now (commented line 226) and come up with a better solution later, or put some additional logic or variable(s) to handle the color image logic separately (e.g. 'bandwidth' or similar). If you don't like the issue of no color with the variables, I think the best route is to use the hard coded line and add additional logic later to offer some customizability.

[aaronr8684]

I did testing on the threshold numbers and here are the values for each screen type:
<= 75 shows red with a red palette
<= 29 shows yellow with a yellow palette

[missionfloyd]

What about the way pimoroni does it?

Edit: Here's what I came up with.

#!/usr/bin/env python3

from PIL import Image
import numpy

BLACK = 1
RED = YELLOW = 2

pal_img = Image.new('P', (1, 1))
pal_img.putpalette((255, 255, 255, 0, 0, 0, 255, 0, 0) + (0,0,0)*253)
img = Image.open("lena_std.tif")
image = img.quantize(palette=pal_img)
buf = numpy.array(image, dtype=numpy.uint8).reshape((image.height, image.width))
buf_black = numpy.asarray(numpy.where(buf == BLACK, 0, 255), dtype=numpy.uint8)
buf_color = numpy.asarray(numpy.where(buf == RED, 0, 255), dtype=numpy.uint8)
img_black = Image.fromarray(buf_black)
img_color = Image.fromarray(buf_color)

image

img_black

img_color

_display() function using this method:

import numpy

# palette indexes
BLACK = 1
RED = YELLOW = 2

def _display(self, image):
    if(self.mode == 'bw'):
        # send the black/white image and blank second image (safer since some drivers require data)
        img_white = Image.new('1', (self._device.height, self._device.width), 255)
        self._device.display(self._device.getbuffer(image), self._device.getbuffer(img_white))
    else:
        # apply the color filter to get a 3 color image
        image = self._filterImage(image)

        # separate out black from the other color
        buf = numpy.array(image, dtype=numpy.uint8).reshape((self.height, self.width))

        buf_black = numpy.asarray(numpy.where(buf == BLACK, 0, 255), dtype=numpy.uint8)
        buf_color = numpy.asarray(numpy.where(buf == RED, 0, 255), dtype=numpy.uint8)

        img_black = Image.fromarray(buf_black, mode="L")
        img_color = Image.fromarray(buf_color, mode="L")

        self._device.display(self._device.getbuffer(img_black), self._device.getbuffer(img_color))

[aaronr8684]

A concern is that numpy seems to be the issue of a lot of install fails. I don't know why, but I've seen it mentioned in multiple libraries.

Also, this doesn't seem any simpler and, in my opinion, harder to read

[aaronr8684]

Your suggested code doesn't work on the 7in5b_V2

[missionfloyd]

It's inverting black and white again.

Changing
buf_black = numpy.asarray(numpy.where(buf == BLACK, 0, 255), dtype=numpy.uint8)
to
buf_black = numpy.asarray(numpy.where(buf == BLACK, 255, 0), dtype=numpy.uint8)
should do it, at least on your screen.

[aaronr8684]

But therein lies the problem. We're chasing fixing each individual screen instead of coming up with a solution that works for all. Does the numpy solution work the same on the 2in7b_V2 or is it the opposite behavior of mine?

[missionfloyd]

What doesn't make sense, though, is that your code and mine produce identical img_blacks, but mine are apparently getting inverted.

Your img_blacks are positive, right? As are all of waveshare's test images.

Anyway, this point method, why are you using thresholds? You're still dealing with an image that's exactly three colors: black, white, and whatever shade of gray that red or yellow ends up as.

img_black = img_black.point(lambda p: 0 if p == 0 else 255) # extract black pixels
img_color = img_color.point(lambda p: 0 if 0 < p < 255 else 255) # extract everything but black and white

This does the same thing, and works with both red and yellow without needing threshold values.

A lookup table works, too, and is a tiny bit faster.

img_black = img_black.point([0] + [255]*255)
img_color = img_color.point([255] + [0]*254 + [255])

[aaronr8684]

I'm using thresholds at the request of @robweber since the number directly effects the end result and he thought it would be good to give that control to any end user that wanted it instead of hard coding the values.

As I've said before, I think the main issue lays in the difference between a pillow 'P' image and 'L' image. For whatever reason waveshare doesn't like a custom mapped palette.

I don't really care what the solution is as long as it works with all tri-color screens. There are probably dozens of ways to solve this problem. I found one that works. It's almost certainly not the simplest, most elegant, or most efficient, but it just works.

This "bug fix" is just a dependency for me before I use Omni as a replacement for the direct driver usage in another project. I found a fix that works and created the PR. If you want to optimize it or find another solution, create a branch and I'll pull it down and test. If at least one other person (who's screen behaves differently from mine) confirms the solution also works, I'll be happy with accepting the alternative. At this point I'd like to just move on to the next thing on my list.

If no one objects to this solution be A working solution, maybe we can accept this PR and then you can research until your heart's delight for a more elegant solution that works with all of the screens and update it later.

[missionfloyd]

Okay, here you go https://github.com/missionfloyd/omni-epd/tree/point-filter-nothreshold

This should "just work" as well. It's just a slight alteration to your code.

[robweber]

Trying to work my way through this whole thread and without even comparing the code it's a bit frustrating to see the communication break down on this. I do value all the contributions made to this project, especially given that often hardware is needed to test the various components; so it's really relying on others to give feedback on what is working and what isn't. It's supposed to be fun and collaborative.

I'll give everything a review tomorrow to try and get this closed out and something working in the mainline ASAP.

[aaronr8684]

Sorry if this was frustrating. I am honestly willing to test any alternative solution, but I'd also like to get something working in place so I can start working on the next part.

I really do appreciate the alternatives and creative ways to solve each problem. That's why I'm here, to learn from others. The end result is just a bonus. I am having fun learning and expanding my understanding of all of this stuff. Thanks for all the hard work you've both put into this already. I hope that my contributions can improve the project overall and not take away from the fun you both have while working on this.

[missionfloyd]

I guess I kind of see it as a puzzle that needs to be solved. ("It doesn't work, why?")

It does sound like your code works as intended, I just think it can be simplified a bit. Personally, I don't think much is gained by the ability to change the threshold in this case; with so few colors, there's only a few different results out of 256 possible values.

There's also already a setting called "threshold".

But ultimately, it's not a big deal. If you guys think it's a good addition, then alright.

[aaronr8684]

I guess I kind of see it as a puzzle that needs to be solved. ("It doesn't work, why?")

I get that drive. It was perplexing to me as well. Why two seemingly similar bw image behaved differently once sent to the driver.

Personally, I don't think much is gained by the ability to change the threshold in this case

I agree IN THIS CASE, but it would offer the option to alter the behavior on the bw mode as well as the 2-color screens in the future. As tested above, in the tri-color case, it's just a matter of if the color is displayed or not.

There's also already a setting called "threshold".

I missed this. Good catch. I'm not sure if we want to try and incorporate this value into this fix or just change the new threshold name to something else.

[aaronr8684]

Okay, here you go https://github.com/missionfloyd/omni-epd/tree/point-filter-nothreshold

This should "just work" as well. It's just a slight alteration to your code.

It's appears to work on mine, has anyone tested this on a screen that behaves differently?

Hopefully, the yellow screen I ordered will be here shortly and I won't have to rely on other testers as much.

[robweber]

We can use the slightly faster version (missionfloyd edition) if it's confirmed working on both the inverted and non-inverted required displays. Don't want to end up in the same situation where something works on one type but not the other. Glad we could get to something not too convoluted in the end, a lot of methods were tried on this one.

I'm going to merge in the point version today simply to get some kind of fix in the mainline since a good section of displays is essentially broken and we know that's been tested. Plus I'm traveling for work starting tomorrow so it will be a week before I can review anything else. Adjusting for a better/faster/quicker version can always be done.

I'm going to hardcode the threshold values for now to avoid confusion. I'm always a fan of letting users mess with the values but in this case it might just be more confusing. I really tried to weigh out the pros and cons from the discussion here and it seems it's really down to splitting hairs more than anything. I'll reference the original issue in the merge as it will be easier to modify the main branch for the parts of this PR I'll need since I'll be stripping out the threshold user configuration pieces.

I do think that a future PR to address the color thresholds would be a fun addition. I can see how allowing a plus or minus gradient around the desired color might give a more vibrant appearance to a limited color display than cutting hard on a single RGB value.

Big thanks to everyone continuing to test this. Loading up different devices and constantly testing small changes is sort of a pain.

[aaronr8684]

We can use the slightly faster version (missionfloyd edition) if it's confirmed working on both the inverted and non-inverted required displays.

missionfloyd's version has NOT been confirmed to work on all screens, just mine.

I'm going to hardcode the threshold values for now to avoid confusion.

I'll submit a commit that has my recommended hard code values that should work with yellow or red screens.

[robweber]

That was quick - thanks. If/When the faster version is verified we can always update that in another PR.

[aaronr8684]

Tested fresh install of omni and it works as expected. thanks!

[missionfloyd]

That was quick - thanks. If/When the faster version is verified we can always update that in another PR.

I should probably clarify; it was only about 2ms faster on a pi zero.

[aaronr8684]

@robweber FYI, just tested Omni-EPD on the WS 4.2" C display and it works as intended. If you want to mark it as tested on the Readme directly as well instead of me submitting a PR.

[robweber]

Done: 3106b3b