PBR Fixes: Apply band factor to spherical harmonic coefficients in accelerated baker light and prevent fireflies artifacts #2439
Conversation
…ifacts on overbright areas
|
🖼️ Screenshot tests have failed. The purpose of these tests is to ensure that changes introduced in this PR don't break visual features. They are visual unit tests. 📄 Where to find the report:
✅ If you did mean to change things: ✨ If you are creating entirely new tests: Note; it is very important that the committed reference images are created on the build pipeline, locally created images are not reliable. Similarly tests will fail locally but you can look at the report to check they are "visually similar". See https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-screenshot-tests/README.md for more information |
|
The screenshot test fails because it needs to be re-tuned to the correct lighting after the fix.
|
|
Looks good to me. Let me know if this all done and ready to merge, and then I'll work on a 3.8.1 release that includes this PR as well as the other PR related to instancing breaking. Also if you're able to, it would be best to upload the new screenshot for the screenshot tests to fix that in this PR before merging. But if you're busy and unable to do that, then I can fix the screenshot test in another PR once this one is merged. |
|
Thank you, it should be good to go, but it is untested on android, also i would want to test it on a real pbr scene. |
Yes, you should be able to just use the newly generated image to replace the old one in the jme-screenshot-tests directory, and then that should make the test pass again. I also won't be of much help testing this currently. Even though I do have some larger PBR scenes using light probes in my project, I am still not at the point of generating my own probes yet, and I just use the DefaultProbe.j3o (from jme-testdata) in most of my scenes. But I do plan to generate custom probes for my scenes eventually, so when that time comes, I'll be able to offer much more help testing generated probes. |
I ended up just putting the other PR in 3.8.1 and held off on this since you said it still needs tested more. And since its not standard to do a beta/alpha for a 3.8 patch release I figured its best probably to hold off on this PR so that it can be more thoroughly tested during the 3.9 alpha release that should be coming soon. |
|
Merging this so it can be tested in the soon-to-come 3.9.0-alpha1 release. I'll also submit another PR to update the screenshot test very soon. |
2 participants
Band factors were not applies to spherical harmonic coefficients in the accelerated baker, causing bad lighting on high roughness materials.
This PR fixes that for both IBLHybridEnvBakerLight (gpu+cpu) and IBLGLEnvBakerLight (gpu) bakers.
This PR also clamps the luminance in the prefiltered envmap kernel, to avoid fireflies artifacts when sampling over bright areas (that could be due to artistic choice or bad tonemap).