Skip to content

Basic fast filtering implementation#36588

Merged
akien-mga merged 1 commit intogodotengine:masterfrom
clayjohn:VULKAN-env-filter
Feb 28, 2020
Merged

Basic fast filtering implementation#36588
akien-mga merged 1 commit intogodotengine:masterfrom
clayjohn:VULKAN-env-filter

Conversation

@clayjohn
Copy link
Copy Markdown
Member

@clayjohn clayjohn commented Feb 27, 2020 •
edited by Calinou
Loading

This PR implements the fast filtering algorithm described here.

Fast filtering uses a two pass approach to blur environment maps to create radiance maps. It has incredible speed benefits over traditional importance sampling but comes at the cost of quality in certain scenarios. The implementation in this PR is both faster and higher quality than our current implementation of importance sampling, however, it can only be used to generate a 128x128 radiance map. So the current importance sampling implementation is left in place so users can have crisper environment reflections if they need.

comparison of fast filtering (top, 10ms) and importance sampling (below, 75ms)
Final Comparison

I have left the options structure more or less the same. Because there is no longer a low quality importance sampling I removed the "realtime_ggx" project setting and added a setting to choose the quality to use for the fast filtering algorithm. Low quality is default and provides excellent quality for most scenes. High quality can be used for use with especially high detail scenes.

Following this PR I would like to make another one improving the current importance sampling technique so that it is higher quality than the realtime option.

Speed Comparison

Current implementation using Importance Sampling

High Quality Low Quality
Array 75,000 us* 5,000 us*
Non-Array 20,000 us 670 us

Proposed implementation using Fast Filtering

High Quality Low Quality
Array 10,000 us* 4,000 us*
Non-Array 2,000 us 300 us

@clayjohn clayjohn added this to the 4.0 milestone Feb 27, 2020
@akien-mga
Copy link
Copy Markdown
Member

akien-mga commented Feb 27, 2020

Clang warning:

servers/visual/rasterizer_rd/rasterizer_scene_rd.cpp:829:6: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses]
        if (!storage->reflection_probe_get_update_mode(rpi->probe) == VS::REFLECTION_PROBE_UPDATE_ONCE) {
            ^                                                      ~~
servers/visual/rasterizer_rd/rasterizer_scene_rd.cpp:829:6: note: add parentheses after the '!' to evaluate the comparison first
        if (!storage->reflection_probe_get_update_mode(rpi->probe) == VS::REFLECTION_PROBE_UPDATE_ONCE) {
            ^
             (                                                                                        )
servers/visual/rasterizer_rd/rasterizer_scene_rd.cpp:829:6: note: add parentheses around left hand side expression to silence this warning
        if (!storage->reflection_probe_get_update_mode(rpi->probe) == VS::REFLECTION_PROBE_UPDATE_ONCE) {
            ^
            (                                                     )

@clayjohn
Copy link
Copy Markdown
Member Author

clayjohn commented Feb 27, 2020

@akien-mga Thanks!

Funny enough, that typo would have made the new code run 6 times every frame instead of once and I didn't even notice cause I still had > 60 FPS.

@clayjohn
Copy link
Copy Markdown
Member Author

clayjohn commented Feb 27, 2020

@akien-mga I will push a version with docs later today. Please don't merge until then.

@clayjohn clayjohn requested a review from a team as a code owner February 27, 2020 21:27
@clayjohn
Copy link
Copy Markdown
Member Author

clayjohn commented Feb 27, 2020

@akien-mga Should be ready to merge pending the outcome of CI.
:)

akien-mga
akien-mga reviewed Feb 27, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs reviewed.

@clayjohn
Copy link
Copy Markdown
Member Author

clayjohn commented Feb 28, 2020

The build failure looks like an issue with the CI itself.

@akien-mga
Copy link
Copy Markdown
Member

akien-mga commented Feb 28, 2020

CI passed but there's a conflict with recent docs changes.

@clayjohn
Copy link
Copy Markdown
Member Author

clayjohn commented Feb 28, 2020

Fixed :)

@akien-mga akien-mga merged commit 4ff84cb into godotengine:master Feb 28, 2020
@akien-mga
Copy link
Copy Markdown
Member

akien-mga commented Feb 28, 2020

Thanks!

@clayjohn clayjohn mentioned this pull request Jul 3, 2020
Closed
@NHodgesVFX NHodgesVFX mentioned this pull request Apr 24, 2022
Closed
@JMS55 JMS55 mentioned this pull request Aug 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akien-mga akien-mga akien-mga left review comments

@reduz reduz reduz approved these changes

Assignees

No one assigned

Labels

enhancement topic:rendering

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@clayjohn @akien-mga @reduz