microsoft / pxt-neopixel

A Neo-Pixel package for pxt-microbit
https://makecode.microbit.org/pkg/microsoft/pxt-neopixel
MIT License
58 stars 141 forks source link

Block order dependency in brightness block #13

Open DaveAtKitronik opened 7 years ago

DaveAtKitronik commented 7 years ago

There appears to be a dependency on block order for the neopixel package set brightness block. Used on its own the block does not change the brightness of the LED. If it is followed by a show colour block then the brightness changes.  Additionally it only appears to change the brightness of the pixel that has been coloured. This script: https://makecode.microbit.org/_Ra5f7VR9xVXf shows button A does not change brightness, button B does. Initially reported to micro:bit support as issue 2678, Jonny suggested posting this directly here.

DaveAtKitronik commented 7 years ago

@jaustin Tagging you for reference.

DaveAtKitronik commented 5 years ago

This is still broken in newer makecode. The following testcase shows that if you perform a show colour the brightness changes as expected, but merely performing a show after changing the brightness does not do anything. https://makecode.microbit.org/_cpL8EogiKdUg microbit running latest firmware DETAILS.TXT @pelikhan any idea on when this might get fixed? It is not intuitive for a user to have to repeatedly recolour the pixels to get them to change brightness.

pelikhan commented 5 years ago

This is currently by design. Internally, we store the colors in a buffer without brightness information. When applying "show color", the current brightness is applied to the color passed by the user and stored in the buffer. We save memory but we also loose the information about the original color. We are already tight in memory on the micro:bit and using long neopixels strips would have further implications here.

DaveAtKitronik commented 5 years ago

The user experience (for a novice) is that the block should change the brightness of the display on the next show command. Examining the code it appears that the ease brightness block applies the change to the buffer at the point of executing - although it also does other sums as well. Show merely pushes out the buffer.
I think unpacking the buffer to apply the brightness to the elements as part of the set brightness block and then repacking it would fix the issue - I'm less clear on how this would cost memory?

pelikhan commented 5 years ago

Say we have this sequence

SetBrighetness 0 Setcolor blue —> resulting color = 0 Set brightness 255 -> resulting color 255

We loose the color information when we squish it with brightness. We would have to keep a second buffer with unmodified colors. Doable, costs a bit of memory and probably ok for most small makes.

Get Outlook for iOShttps://aka.ms/o0ukef


From: DaveAtKitronik notifications@github.com Sent: Tuesday, January 29, 2019 6:02:57 AM To: Microsoft/pxt-neopixel Cc: Peli de Halleux; Mention Subject: Re: [Microsoft/pxt-neopixel] Block order dependency in brightness block (#13)

The user experience (for a novice) is that the block should change the brightness of the display on the next show command. Examining the code it appears that the ease brightness block applies the change to the buffer at the point of executing - although it also does other sums as well. Show merely pushes out the buffer. I think unpacking the buffer to apply the brightness to the elements as part of the set brightness block and then repacking it would fix the issue - I'm less clear on how this would cost memory?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMicrosoft%2Fpxt-neopixel%2Fissues%2F13%23issuecomment-458550682&data=02%7C01%7Cjhalleux%40microsoft.com%7Cc71df15010bc40f60e8808d685f278a7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636843673783165371&sdata=gjc9VZdXlCzjhCe5nBzXiipZc28sGm%2BELTe6hIZ93rA%3D&reserved=0, or mute the threadhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAD-4KZkM1PYpa7Sm_AYuL3-1f9KaK6Qpks5vIFSRgaJpZM4PapP_&data=02%7C01%7Cjhalleux%40microsoft.com%7Cc71df15010bc40f60e8808d685f278a7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636843673783165371&sdata=e0eb0n9UE5bGvT3IxYI606wE125JE8W4D%2BilIslvjco%3D&reserved=0.

DaveAtKitronik commented 5 years ago

I see. Maybe the show should respect the brightness settings and do the application of the current brightness before pushing the buffer out?

pelikhan commented 5 years ago

That would mean allocating a second buffer to compute the final colors... which may result in OutOfMemory condition.