blazoncek / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
MIT License
31 stars 0 forks source link

Patch 1 #12

Closed Proto-molecule closed 2 years ago

Proto-molecule commented 2 years ago

Here is what I found,

Line 526: "now - lastRedraw" equates to a negative number and forces the screen into sleep mode if the settings are changed using there web UI. I'm assuming that you're using the variable "now" so that "mills()" function is called less often, With this edit, the variable "now" doesn't serve much purpose and may be adding to the CPU time, unless, every "lastRedraw = millis();" line is somehow replaced with "lastRedraw = now;".

Line 646, 660: if the array size isn't at least the length of the longest name of an effect or palette, then it will not fully print. "Fireworks Starburst" is the longest one at 19 characters. this it kinda a bandaid fix because a longer name may come in the future.

line 436: perhaps this needs to be 256?

blazoncek commented 2 years ago

Thank you! I was banging my head because of introduction of now. šŸ˜ƒ

blazoncek commented 2 years ago

Hi @Proto-molecule

Thanks for contribution. I had to make some additional changes sinc your original modification broke compatibility with Auto Save usermod. I also optimised other parts a bit. Tested quite extensively and the result is now in dev branch.

Proto-molecule commented 2 years ago

Absolutely @blazoncek you're welcome, Iā€™m happy that something came of it! I have a quick question though. It seems like WLED intentionally tries to not use ā€œmillisā€ and your previous comments ā€œI was banging my head because of introduction of now.ā€ reinforced this assumption. Whatā€™s the reasoning behind it? Im curious because I recently submitted a pull request to add a new effect and I use ā€œmillisā€ all over the place. And perhaps I shouldnā€™t haveā€¦

blazoncek commented 2 years ago

If you do not need exact millisecond read at each of the places it is best to use variable since it is much faster. Each milis() call uses precious CPU and as @pbolduc (I think it was him but am not sure) once demonstrated it is quite a lot.

And since usermod loop() code gets executed at each LED update cycle it should be as fast as possible to prevent FPS drop or missing frames. Same goes for effect itself.

blazoncek commented 2 years ago

I am adding a preset cycle (with a twist) but so far it works. Do you know how to add a star (ā­) or a heart (ā¤ļø) to the font?

blazoncek commented 2 years ago

NVM. Figured it out. For now just replaced benji font with built-in open iconic.

Proto-molecule commented 2 years ago

yeah I can add them to the existing benji fonts 1x1, 2x2, and 6x6 sizes. if you need another size let me know they are actually already in the source code here so it will be easy to add. (the palette and saturation icons I had to draw lol) the star is already available in the 1x1 and 2x2 size (u8x8_font_open_iconic_weather).

blazoncek commented 2 years ago

The idea is to minimize flash usage too. So the only icon missing is the palette one as you said. I also switched to 4x4 icons on overlay since they seem nicer. Already pushed so you can have a look.

Proto-molecule commented 2 years ago

Yeah Iā€™ve kinda been down this road before.

Limited flash was the reason I was forced to make my own font library. Originally I did exactly what you did and actually ran out of flash. Since WLED version 13.0, it seams like the flash limit was increased using the .csv file, so thereā€™s plenty of space now and it complies no problem.

Iā€™ve observed the following: Even if accessing a single glyph in the font, the compiler loads the entire font into flash. Combined, the 4x4 fonts (embedded), (weather), (play), and (thing) use up almost 3.5 times the amount flash as my single custom 6x6 font. I can only assume that this is the reason why the developer of the U8g2 split up the icons into small groups. Otherwise what harm is there in leaving just one large font i.e. ā€œall_icons_4x4ā€.

Originally I was going to make my custom front 4x4 but then saw how much space I was saving so then opted for the higher resolution.

Personally, I like how the higher/larger icons looks. Nothing wrong with the 4x4 look either though. The saturation you found is better. Donā€™t know how I missed that one lol

I put together all of the icons currently used in all of the sizes (1x-6x) and added them to the code here. Go ahead and take a look, hopefully it can help you return some lost simplicity in the code and save flash in the process. If you want anything else added (heart or star?), just let me know. Or if you find a black and white .png file online that you like, it can also be made into an icon as well ;)

Proto-molecule commented 2 years ago

Ive also entertained the idea of making the saturation and main color be displayed as a raising bar and filling pie chart respectfully. There is a little bit of space left below the power and wifi icon. For it to work, many ā€œbarā€ and ā€œpie chartā€ icons will be needed (from empty to full). Perhaps 10-20 of each in a 2x2 size. If you think itā€™s worth a try I can put together a draft.

blazoncek commented 2 years ago

Thank you for excellent explanation. I did not look at the compiled size I just assumed that built-in fonts always get loaded. On a second thought having 6x6 icons (on 64x128 display) has no drawbacks. šŸ˜„ I will try new fonts later.

BTW have you joined Discord? We can have chat over there...