EEVEE-Next: Horizon Scan: Use Spherical harmonics #118924
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#118924
Loading…
Reference in New Issue
No description provided.
Delete Branch "fclem/blender:eevee-next-horizon-scan-refactor"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This uses Spherical Harmonics to store the indirect lighting and
distant lighting visibility.
We can then reuse this information for each closure which divide
the cost of it by 2 or 3 in many cases, doing the scanning once.
The storage cost is higher than previous method, so we split the
resolution scaling to be independant of raytracing.
The spatial filtering has been split to its own pass for performance
reason. Upsampling now only uses 4 bilinearly interpolated samples
(instead of 9) using bilateral weights to avoid bleeding.
This also add a missing dot product (which soften the lighting
around corners) and fixes the blocky artifacts seen at lower
resolution.
Current implementation make use of much more memory than previously.
Spherical harmonics need 8 times the storage previously used by the horizon scan. This has an impact on the spatial filtering efficiency which is now done at tracing resolution instead of running it at target resolution.
But this raises the need for a separate resolution parameter for the horizon scan which most of the time requires less resolution than the direct ray-tracing.
Adding Miguel and Jeroen so they can test on their hardware if anything is much slower or broken.
EEVEE-Next: Horizon Scan: Do tracing only onceto EEVEE-Next: Horizon Scan: Use Spherical harmonicsCould you add an option to add 1/8 RPP
I think isn't a bad idea for have a perfect balance between performance / quality
Just a quick test for now, but at least on this scene performs and looks better (at max resolution).
Quite promising, congrats!
@blender-bot package
Package build started. Download here when ready.
This is just a test on metal. Details are much more natural. I couldn't find any differences when using the precision level slider. Is that expected? I also didn't see any visual differences when using a lower render resolution for the AO
Will still try to run it on iGPUs and do a quick scan of the code. So far so good!
I performed some performance measurements on different scenes. And got mixed results. For good comparison I merged
main
into this PR.I will look into the tree_creature performance in EEVEE-Next. I expected more uplift, but could be related to something else. Ok the performance bottleneck for the tree creature is shadowing. When disabling the shadowing I get (4.7 fps, 7.8 fps and 8.0 fps)
If you don't get any noticeable uplift, then the bottleneck is somewhere else.
The performance drop in rain at full resolution might be explained by the bandwidth usage being larger in this case. That's why the default is 4x which is roughly the same bandwidth usage as before.
Branch doesn't work on Metal.
The Metal issue should be fixed.
@ -68,1 +85,3 @@
ivec2 texel = (texel_fullres) / uniform_buf.raytrace.resolution_scale;
vec2 texel_size = 1.0 / vec2(textureSize(in_sh_0_tx, 0).xy);
ivec2 texel_fullres = texel * uniform_buf.raytrace.horizon_resolution_scale +
uniform_buf.raytrace.horizon_resolution_bias;
Do we need to check the bounds?
if (any(greaterThanEqual(texel_fullres, extent))) {
This one is necessary. The dispatch is in tiles that can process outside of the render target boundaries.
However, the background check is not needed since we already check for GBuffer header value.
@ -21,1 +22,4 @@
/* Avoid loading texels outside texture range. */
ivec2 extent = textureSize(gbuf_header_tx, 0).xy;
texel_fullres = min(texel_fullres, extent - 1);
Do we need to add a boundry check?
RAYTRACE_GROUP_SIZE = 8
That is exactly what this line does. If the thread is outside the valid range, it will sample the edge pixels. This is needed because of the jitter in
horizon_resolution_bias
.The
imageStore
should do the bound check for writing. However, it might be preferable to disable the threads that are outside the output bounds to leverageimageStoreFast
. Will do that.@ -2995,6 +2995,12 @@ void blo_do_versions_400(FileData *fd, Library * /*lib*/, Main *bmain)
}
}
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 402, 10)) {
Add
if (!DNA_struct_member_exists(fd->filesdna,
for clarity.See https://blender.chat/channel/blender-coders?msg=3aeDipEsYEtDJGwei for reference.
@blender-bot build
I only took a quick look at the code, but I've tested the patch on my RTX GPU and on my cheap integrated AMD laptop and the quality and performance are better on both.
The only case when it's slower is when all materials are diffuse-only.
But optimizing for the common/worse case seems like a good compromise to make.
@ -0,0 +10,4 @@
#pragma BLENDER_REQUIRE(eevee_lightprobe_eval_lib.glsl)
#pragma BLENDER_REQUIRE(eevee_closure_lib.glsl)
float bilateral_depth_weight(vec3 center_N, vec3 center_P, vec3 sample_P)
There are some functions duplicated between the denoise and resolve shaders.
Couldn't these be moved to a lib file?
@ -1895,6 +1895,7 @@ typedef struct SceneEEVEE {
float gtao_quality;
float gtao_thickness;
float gtao_focus;
int gtao_resolution;
Why are you using the
gtao
term for the DNA property?To avoid discrepancy with existing code. If we are to rename it, we will rename all of them at once.
Oh... because the horizon scan is driven by the
AOData
from theAmbientOcclussion
module.But these new properties are added to
RayTraceData
directly.I find that quite confusing, TBH.
I do agree that this is confusing. But I still think this is best left for a future cleanup.
@ -8176,0 +8186,4 @@
RNA_def_property_ui_text(prop,
"Resolution",
"Control the quality of the horizon scan lighting "
"(lower size increase vram usage and quality)");
Just a nitpick, but I would make clear that this also has a render time cost, not only a memory cost.
Something like
(lower size increases quality at the cost of performance and vram)
.I would argue that this is implicit from stating
quality
. For reference I copiedTile Size
setting for volumetrics. Not sure this has ever been an issue there.I'll do a UI pass with the UX team later. I don't think this is blocker.