Higher quality bicubic lightmap sampling#16740
Higher quality bicubic lightmap sampling#16740alice-i-cecile merged 12 commits intobevyengine:mainfrom
Conversation
JMS55
added
the
S-Needs-Review
|
imo this should be optional (with at least linear being the other option). Try with a low res light map to see the effect. |
pcwalton
left a comment
pcwalton
left a comment
There was a problem hiding this comment.
Looks good modulo a little inaccuracy.
I could go either way on exposing an option for bilinear, and I won't block the PR on that. A quote from the Bakery author's blog post: https://ndotl.wordpress.com/2018/08/29/baking-artifact-free-lightmaps/
Bicubic interpolation. If you are not shipping on mobile, there are exactly 0 reasons to not use bicubic interpolation for lightmaps. Many UE3 games in the past did that, and it is a great trick. But some engines (Unity, I’m looking at you) still think they can get away with a single bilinear tap in 2018. Bicubic hides low resolution and makes jagged lines appear smooth. Sometimes I see people fixing jagged sharp shadows by super-sampling during bake, but it feels like a waste of lightmapping time to me.
See the post for a screenshot comparison.
The "if you are not shipping on mobile" comment is 6 years old, so it may well just be time to say bicubic everywhere.
|
Based on these images from @pcwalton I don't think bicubic is correct: https://discord.com/channels/691052431525675048/743663924229963868/1316259592308527134
|
|
Consensus is to make bicubic vs bilinear a toggle, and default to bilinear. Bicubic is oftentimes only a marginal quality improvement, can lead to light leaks, and is more expensive. |
|
I think prepass + deferred might be broken on main. I don't see any usage of |
alice-i-cecile
added
S-Ready-For-Final-Review
|
@DGriffin91 @JMS55 what's your final opinion on the status of this? Did d224365 resolve the concerns? |
|
It's fine now imo, but lightmaps being broken with deferred needs fixing first, so that I can setup the shaderdefs for bicubic sampling in the deferred gbuffer pass properly. |
alice-i-cecile
changed the title
alice-i-cecile
added
S-Blocked
|
Blocked on #16836, and then once that's merged I need to update the deferred queuing code. |
JMS55
added
S-Needs-Review
| let p1 = (vec2(iuv.x + h1x, iuv.y + h0y) - 0.5) * texel_size; | ||
| let p2 = (vec2(iuv.x + h0x, iuv.y + h1y) - 0.5) * texel_size; | ||
| let p3 = (vec2(iuv.x + h1x, iuv.y + h1y) - 0.5) * texel_size; | ||
| let color = g0(fuv.y) * (g0x * sample(p0, lightmap_slot) + g1x * sample(p1, lightmap_slot)) + g1(fuv.y) * (g0x * sample(p2, lightmap_slot) + g1x * sample(p3, lightmap_slot)); |
There was a problem hiding this comment.
Pretty sure I already reviewed this, so I'm just assuming this is correct :)
| /// | ||
| /// Bicubic sampling is higher quality, but slower, and may lead to light leaks. | ||
| /// | ||
| /// If true, the lightmap texture's sampler must be set to [`bevy_image::ImageSampler::linear`]. |
There was a problem hiding this comment.
Might want to mention that the default is false.
Add a `--bicubic` switch to the `lightmaps` example
alice-i-cecile
added
S-Ready-For-Final-Review
# Objective - Closes bevyengine#14322. ## Solution - Implement fast 4-sample bicubic filtering based on this shader toy https://www.shadertoy.com/view/4df3Dn, with a small speedup from a ghost of tushima presentation. ## Testing - Did you test these changes? If so, how? - Ran on lightmapped example. Practically no difference in that scene. - Are there any parts that need more testing? - Lightmapping a better scene. ## Changelog - Lightmaps now have a higher quality bicubic sampling method (off by default). --------- Co-authored-by: Patrick Walton <pcwalton@mimiga.net>
4 participants
Objective
Solution
Testing
Changelog