Properly calculate BPP for displayio.Bitmap

[deshipu]

No description provided.

[tannewt]

Looks good! Thank you for the fix!

[tannewt]

After thinking about this I don't think this is the right fix. Bitmap only handles 1, 2, 4 or 8 bits. It can't do number of bits that don't fit evenly into a byte.

[deshipu]

Hmm, in that case, the original code was correct up to 8 bits, be just need a special case for multiples of 8.

[deshipu]

We probably should also have a check for a maximum value — what that would be?

[deshipu]

Actually we don't need to check for max value, because 2ⁿ is always going to be larger than n, so we can't get a number so large that we can't handle, now that we increase bits slower.

[tannewt]

The implementation caps at 8 bit color: https://github.com/adafruit/circuitpython/blob/master/shared-module/displayio/Bitmap.c#L48

I think this implementation may still infinite loop for value counts > 0x80000000 because the minimum value based on bits will wrap around. Do we want to support 32 bit color in the future or is 24 enough? We can cap the number for the API here and leave the above check for the implementation.

[deshipu]

Oh, you mean the 1U << bits will wrap around with bits >= 32?
I wonder if we could do value_counts >> bits instead...

[deshipu]

I suppose we now need to add a check that value_counts > 0...

updated

[tannewt]

Looks good! Unfortunately the translation check isn't happy because make translate wasn't run after adding it.

[dhalbert]

@deshipu Can you fix the translation conflicts and resubmit? (Maybe after your other PR's are cleared.)

[tannewt]

Travis is happy. Thank you!