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

Implement Auto Exposure plugin #12792

Merged
merged 36 commits into from
May 3, 2024
Merged

Conversation

Kurble
Copy link
Contributor

@Kurble Kurble commented Mar 29, 2024

Objective

  • Add auto exposure/eye adaptation to the bevy render pipeline.
  • Support features that users might expect from other engines:
    • Metering masks
    • Compensation curves
    • Smooth exposure transitions

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

  • Added: AutoExposure plugin that allows to adjust a camera's exposure based on it's scene's luminance.

@Kurble Kurble changed the title Auto exposure Implement Auto Exposure plugin Mar 29, 2024
Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@NthTensor NthTensor added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Mar 30, 2024
@torsteingrindvik
Copy link
Contributor

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.

@Kurble
Copy link
Contributor Author

Kurble commented Mar 30, 2024

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?

@pcwalton pcwalton requested a review from IceSentry April 14, 2024 08:36
Copy link
Contributor

@NthTensor NthTensor left a 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.

@NthTensor NthTensor added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 2, 2024
@alice-i-cecile
Copy link
Member

Yep, none of my concerns are blocking here! I've committed the straightforward suggestions to reduce noise.

@mweatherley
Copy link
Contributor

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.

Yeah, it looks to me like this could ultimately just use any Curve<Vec2>, but I haven't looked extremely closely at it.

@Kurble
Copy link
Contributor Author

Kurble commented May 2, 2024

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 )

@Kurble
Copy link
Contributor Author

Kurble commented May 2, 2024

@pcwalton you optimized out the exposure value that's in the ColorGradingGlobal in #13121, but the auto exposure shader is writing it's result to there. I looked for an alternative solution, but the auto exposure pass can't use the exposure value of theExtractedCamera, as that is used to render the scene before it's histogram can be constructed.
So I just added an extra #else for when sectional color grading is not used.

@alice-i-cecile
Copy link
Member

Very nice docs additions, thanks! Definitely wasn't necessary but it's great to leave the codebase just a bit nicer.

@Kurble
Copy link
Contributor Author

Kurble commented May 3, 2024

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.

Yeah, it looks to me like this could ultimately just use any Curve<Vec2>, but I haven't looked extremely closely at it.

It could absolutely use a generic Curve<Vec2>, the only requirement is that the curve is monotonic on the x-axis. Could be a nice cleanup to use Curve::resample for the compensation curve texture once Curve has landed.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 3, 2024
Merged via the queue into bevyengine:main with commit d390420 May 3, 2024
31 checks passed
@JMS55 JMS55 mentioned this pull request May 7, 2024
3 tasks
@alice-i-cecile alice-i-cecile added the M-Needs-Release-Note Work that should be called out in the blog due to impact label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible M-Needs-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants