-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Implement Auto Exposure plugin #12792
Conversation
The generated |
This is really cool and you write great docs. I had a look because I'm a bit interested in the histogram usage (#11468). Out of scope here but I'm hoping for a generalized histogram solution in the future for Bevy. |
Thanks for your feedback! I've made changes accordingly and fixed the CI. A generalized histogram would be a bit more involved as the two compute shader passes rely on each other to work correctly, so there might be a slight performance hit if you want to separate them. I guess that's something to think about for a follow up PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only have the time for a very cursory review, but it looks like high quality textbook implementation of exposure compensation. Hope to come back and give it a more through look over, but what I've seen is good enough for me.
It would be really nice to get this in for the 0.14 release. None of the open comments seem like they should be blocking. Sure there's stuff we could improve for new users, but I think it's all pretty clear if you've used something like this before. I'd like to suggest we merge this and perhaps @pcwalton or I can work on improving the docs as follow up.
Oh and @mweatherley, food for thought: This consumes the CubicGenerator
from the math crate. First use of splines i've seen "in the wild", might be something to evaluate the curve trait PR on.
Yep, none of my concerns are blocking here! I've committed the straightforward suggestions to reduce noise. |
Yeah, it looks to me like this could ultimately just use any |
I'll be addressing the doc things, I have some time for it. Latest merge with main unfortunately isn't working anymore so I'll be fixing that first! (Probably because of #13121 ) |
@pcwalton you optimized out the |
Very nice docs additions, thanks! Definitely wasn't necessary but it's great to leave the codebase just a bit nicer. |
It could absolutely use a generic |
Objective
This PR is based on an implementation I already built for a personal project before #8809 was submitted, so I wasn't able to adopt that PR in the proper way. I've still drawn inspiration from it, so @fintelia should be credited as well.
Solution
An auto exposure compute shader builds a 64 bin histogram of the scene's luminance, and then adjusts the exposure based on that histogram. Using a histogram allows the system to ignore outliers like shadows and specular highlights, and it allows to give more weight to certain areas based on a mask.
Changelog