Use Pillow manipulation logic for grey-to-RGB conversion on EV3 to improve perf

[WasabiFan]

As reported in ev3dev/ev3dev#1150, our display update code is really slow. As in... several seconds for a single display update. I've concluded that the primary issue is that our home-grown conversion from 8bpp greyscale to 32bpp is an unoptimized blob of Python (compared to the pure C implementation in PIL/Pillow). After some investigation and experimenting, I found a couple ways to take advantage of the pre-build Pillow conversions, and included the fastest here.

Performance:

• v1.0 series (when the display driver accepted 1bpp directly): 27FPS max
• v2.0 series now (with our custom converter): 0.2FPS max
• Code included in this PR: 8FPS max

So this improves the current code by a factor of 40, but is still three times slower than what we had in v1.

I am tempted to open a PR on Pillow, but judging by A) their largely ignoring external PRs and b) the fact that the file in question hasn't been modified for almost a year, I'm not fond of my chances at getting code in there.

Note that this only updates the EV3; whatever platforms require the "565" scheme are still going to be slow, and I don't see a Pillow built-in for such a format. Perhaps someone could suggest something there.

[ddemidov]

This looks good if this works :)

[ddemidov]

If performance is important, it should be easy to make a C++ extension inside our library just for this, for example with https://github.com/pybind/pybind11.

[dlech]

There are a couple of other options here.

1. Use an RGB image instead of grayscale.
2. Fork the pillow debian package and add what we need to it.

[WasabiFan]

@dlech:

Use an RGB image instead of grayscale

I want to avoid that so the user doesn't have to be exposed to the implementation detail that we use RGB internally. Trying to explain why you have to pass three values like that and how they interact is by no means impossible, but it's certainly confusing.

I had also considered replacing the public PIL ImageDraw instance with a custom wrapper that did the translation, but I expect that we would implicitly lose functionality without realizing it.

Fork the pillow debian package and add what we need to it.

That certainly could work well. What do others think?

@ddemidov:

If performance is important, it should be easy to make a C++ extension inside our library just for this

On one hand, I like that because it would enable us to add performance-focuswd extensions to other areas if desired. On the other, it would lose us the simplicity of the pure-python setup we have now — it would require build changes, wrestling with the dev workflow, etc. I'm not entirely sure where I stand on that yet.

[dwalton76]

I’ve done some C libraries for python and the build part wasn’t bad. The performance improvement wasn’t a good as I expected though. It was certainly faster than python but not nearly as fast as running the same code in pure C. Pure C was 25x faster than the same C code in a python library.

[dwalton76]

Forking pillow seems reasonable to me if they aren’t accepting pull requests

[WasabiFan]

I think I'm going to see if I can get a working packer implemented in Pillow and open a PR. If they merge it, hooray! Then we probably just have to package it up for our own use immediately. Otherwise, we maintain a fork and go from there.

[WasabiFan]

For the immediate future, I think I'm going to merge this (unless I get any objections) so we can release it and then go from there. I still don't have a good solution to the other platform option which still uses a Python converter.

I have a working patch here and verified it's 2x faster on my desktop, but still need to validate on the EV3 before I'll open a PR.