Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve and fix glow filter #239

Merged
merged 6 commits into from
Feb 6, 2020
Merged

Conversation

laino
Copy link
Contributor

@laino laino commented Jan 10, 2020

The old glow filter had a number of issues which this PR addresses.

  • It unnecessarily calculated maxTotalAlpha by adding stuff up in a loop, even though that value can be statically calculated with distance * (distance + 1.0) * outerLoopIterations. See this wikipedia article if you're interested in the details.

  • The final calculation used a few tricks to avoid dividing by zero. The new code doesn't even have to worry about that anymore and is much simpler too.

  • The old filter did extra loops where (distance - curDistance) was 0 and multiplied with the result. Essentially the old filter did a lot of work for the final distance step, then discarded the result.

  • A few things could be calculated once in the outer loop instead of the inner one or expressed using vector math.

  • Distance can't be changed because it's used in a loop and thus needs to be put into the fragment source. The old code gave it as an uniform as well and let the user change it. It's now read only. Changing distance mostly just lead to fun artifacts if your filter had any innerStrength.

If we decoupled distance steps from distance step size we could make that freely configurable at the cost of slightly more computation. Maybe I'll think about that tomorrow.

  • Using not maxed out color values like 0xf24545 lead to some fun artifacts (increase inner and out strength to really see them):

  • The filter should look pretty much identical to the old one except where the old one ran into problems.

  • Overall it's a lot less and more readable code, though I added blank lines and eliminated really long lines for readability.

@laino
Copy link
Contributor Author

laino commented Jan 12, 2020

  • I fixed some artifacts around edges of sprites when using really dark color values.

  • Added support for fractional distance values (the calculation just didn't take it into account and ended up being 'incorrect' ever so slightly - not sure if it was even visible).

  • Removed the distance slider from the demo.

@laino
Copy link
Contributor Author

laino commented Jan 12, 2020

.... And in the process I added (some) the color discoloration bugs of the old filter back in... Hmmm.

@laino
Copy link
Contributor Author

laino commented Jan 12, 2020

My math was off. To make fractional distance values work correctly one has to unroll the last step of the loop with the "fractional" step.

Not sure if this is even worth it because it makes the code more complicated.

Maybe distance should just be rounded.

Edit: Yeah all this makes zero sense. You can't even see the difference. I'll just round distance.

@bigtimebuddy
Copy link
Member

Screen Shot 2020-02-04 at 3 20 08 PM

@laino love this fix. performance is much better. One question: Is there anyway to smooth or avoid the weird spike artifacts on sharp corners?

@laino
Copy link
Contributor Author

laino commented Feb 5, 2020

The spikes are related to the "quality steps", basically the pattern we use to test in various distances from the current pixel. They could probably be reduced by (fake) randomizing/dithering the angles to some degree or just by increasing the quality value (at the cost of performance).

@bigtimebuddy bigtimebuddy merged commit 2cd898f into pixijs:master Feb 6, 2020
@bigtimebuddy
Copy link
Member

Thank you @laino!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants