Memoize scalar brightness calculations

[mcscope]

@tannewt mentioned that the double buffer use for global brightness != 1 lead to a slight slowdown, so I memoized it.

The way this works is we scale all pixels by global brightness when we insert them into the buffer, and then in show() we just flush the buffer. We still need two buffers, one to store the scaled values and one to store the true buffers.

Now, people may still want to update the global brightness and have the strip flash without resetting all the pixel values. So, in the brightness setter, I scale all the brightness values by the the new brightness value, using the 'true buffer' as the source of truth for the intended color.

This should uses the same amount of memory as before - both required two buffers. But with this, we save rebuilding the entire buffer on every single show() call, which feels like a significant win for folk who want a high refresh rate.

Testing:

Tested on hardware (Needs fresh testing on hardware since latest changes)

[mcscope]

Ahhhh. this would wipe out all your colors on your strip if you set brightness to 0. That seems bad.

I could do it with two persistent buffers, one for true values, and one for brightness-adjusted values. Both buffers would only need to modified in _setitem and the brightness setter, so we would save the computation time on show() but we wouldn't save the memory.

@tannewt what do you think

EDIT: Comment referred to old version of pull request which tried to use a single buffer.

[mcscope]

I'm rewriting to have two persistent buffers, but no work in show()

[mcscope]

Turns out lists of tuples take up 10x the space of a byte array, so rewrote to use byte array for both buffers

[mcscope]

Good feedback! I'll make those changes. I'm no longer on sprint days so I may be less speedy with them fyi.

[tannewt]

K no worries! All help is appreciated!

On Thu, May 17, 2018 at 1:30 PM Christopher Beacham < notifications@github.com> wrote:

Good feedback! I'll make those changes. I'm no longer on sprint days so I may be less speedy with them fyi.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/adafruit/Adafruit_CircuitPython_DotStar/pull/23#issuecomment-389964560, or mute the thread https://github.com/notifications/unsubscribe-auth/AADNqRFjOzzwDmFqBagS9mpPxV2uCIKAks5tzcGugaJpZM4UCDDu .

[mcscope]

@tannewt Can I write unit tests for this? will they be able to be run? Right now I only know how to run this on hardware. The logic to create and destroy the buffer based on brightness is kinda complicated and I'd like to pin it down with some tests.

[tannewt]

@mcscope We currently don't have a way of automatically run hardware tests like this. I'd suggest writing a test you can run manually and checking it into the examples folder so others can run it later.

I wish we had a way of doing it though. :-)

[kattni]

@mcscope Is this something you're still interested in implementing? As well as addressing the requested changes, we've since made changes to the DotStar lib that you'll need to incorporate into this PR to eliminate the merge conflicts. If not, please let me know so we can close the PR. If I don't hear back from you, I will proceed with closing it - it can always be reopened or resubmitted. Thanks!

[kattni]

Closing. Please reopen or create a new PR if you want to implement this.