search

scottschiller / Snowstorm

Enterprise-grade JavaScript snow effect for the internets, setting CPUs on fire worldwide every winter since 2003.
http://www.schillmania.com/projects/snowstorm/
Other
538 stars 144 forks source link

Do you accept PR that actually fix performance? #32

Open wardpeet opened 6 years ago

scottschiller commented 6 years ago

I will assume this was written with good intentions in mind, but comes off as snarky / frustrated - presumably because you saw that performance was sub-par, and/or, there haven't been any updates on this repo in some time. Notwithstanding, I appreciate the note because we're all human and I feel like this myself on occasion. "Dammit, does anyone still actually maintain this thing!?" 😉

If you have suggestions or ideas in the form of a PR or similar for improving performance, that is most welcome! I typically update this script yearly, before "snowstorm season" hits in November. I did not get to updates before November this year, but might have to patch something in for Firefox (see below). This script has been around since 2003, so it has quite a history to it (and should still work in IE 6, perhaps older.)

An issue was just reported in Firefox Quantum (57), it appears to have a regression in style calc/layout when there are a number of position: fixed elements on the screen. In this case, position: fixed is used where supported to make snow "stick" to the bottom of the window while scrolling. I've suggested a few options for workarounds.

Firefox 57 findings / recommendations here. https://github.com/scottschiller/Snowstorm/issues/31#issuecomment-348718597

It would be nice to get accelerated snow via CSS animations / transitions (albeit, animations might have to be simplified a bit), and/or transform: translate() for positioning instead of the current absolute top/right/bottom/left. The present approach uses the latter and is very convenient because percentage-based offsets can be used, and they are very flexible in that snow will automatically reposition relative to the screen (or parent element if someone wants it snowing only in one <div>, etc.)

wardpeet commented 6 years ago

Thanks for your message. I didn't want to come of as snarky, so sorry about that. I don't mind putting a PR together that fixes the things you are referring too. Lots of advertisements are using this script so that's why I wanted to update this plugin as I see plenty of people are using it.

I didn't want to write this issue as a criticism on your work as you mentioned this plugin was created in 2003 and the web grew and grew during that time.

I'll cook up a PR :)

ericnkatz commented 6 years ago

@scottschiller Thanks for writing and revising this over the years. 👍

scottschiller commented 6 years ago

Damn advertising! 🤛 🔥 🤜 Well, if it inspires more people to install ad blockers due to CPU usage, I call that a net win. :trollface:

I like the idea of <canvas> for a rendering target these days, potentially lower CPU than CSS animations. I just noted the site saddlebackleather.com running some snow (at time of writing, Dec. 2017), which appears to be a static background image (or two) with long CSS transitions. Firefox isn't great on CPU (80%?), but Chrome was surprisingly high - like 180% CPU on this 2011-era quad-core i7. Maybe it's because the snow is on <body> or similar and runs the whole page despite the animation only showing in the header, who knows. Regardless, that one's notably worse than Snowstorm - I didn't expect that, honestly.

<canvas> remains of interest.