Eevee-Next: World Reflective Light #108149

Merged
Jeroen Bakker merged 33 commits from Jeroen-Bakker/blender:eevee-next-world-shader into main 2023-06-29 15:25:04 +02:00
Member

This PR adds world probe baking to Eevee-next. The world is baked to a
cubemap and is used for reflective light in the deferred render pass.

The world probe is baked to a resolution of 2048x2048. In the future this
would become a user facing setting, but wasn't considered essential for
the first implementation.

When updating the world cubemap the world surface shader is reused.
Currently the world surface shader clears many render passes. It was
decided to replace the render passes with dummy textures as the effort
and potential slowdown didn't weigh against the benefit of doing this nicely.
Updating the world reflection probe isn't expected to happen often.

A big difference with Eevee(-legacy) is that the roughness GGX reflection
parameter isn't baked into the texture (as mipmap levels), but is calculated
during shading. This improves accuracy as we don't assume that every
object is an infinitive small sphere. The result has more noise and that
will be tackled after SSR will land.

image

This PR adds world probe baking to Eevee-next. The world is baked to a cubemap and is used for reflective light in the deferred render pass. The world probe is baked to a resolution of 2048x2048. In the future this would become a user facing setting, but wasn't considered essential for the first implementation. When updating the world cubemap the world surface shader is reused. Currently the world surface shader clears many render passes. It was decided to replace the render passes with dummy textures as the effort and potential slowdown didn't weigh against the benefit of doing this nicely. Updating the world reflection probe isn't expected to happen often. A big difference with Eevee(-legacy) is that the roughness GGX reflection parameter isn't baked into the texture (as mipmap levels), but is calculated during shading. This improves accuracy as we don't assume that every object is an infinitive small sphere. The result has more noise and that will be tackled after SSR will land. ![image](/attachments/12de6025-67ca-40d2-9540-bcc8a8a1605d)
795 KiB
Jeroen Bakker added the
Module
EEVEE & Viewport
label 2023-05-22 16:04:35 +02:00
Jeroen Bakker added this to the 4.0 milestone 2023-05-22 16:04:39 +02:00
Jeroen Bakker added this to the EEVEE & Viewport project 2023-05-22 16:04:42 +02:00
Jeroen Bakker self-assigned this 2023-05-22 16:04:49 +02:00
Clément Foucault reviewed 2023-05-23 10:55:07 +02:00
Clément Foucault left a comment
Member
  • Create World Probe pipeline
  • Rename to ReflectionProbe(Module)
- Create World Probe pipeline - Rename to ReflectionProbe(Module)
@ -0,0 +30,4 @@
class Instance;
/* -------------------------------------------------------------------- */
/** \name Cryptomatte

👎

👎
Jeroen-Bakker marked this conversation as resolved
@ -0,0 +50,4 @@
class CubemapsModule {
private:
/** The max number of cubemaps to track. */
static constexpr int MaxCubemaps = 1;

Codestyle!

Codestyle!
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker reviewed 2023-05-23 14:25:31 +02:00
@ -65,0 +95,4 @@
side.cubemap_face_ps.framebuffer_set(&side.cubemap_face_fb);
side.cubemap_face_ps.state_set(DRW_STATE_WRITE_COLOR | DRW_STATE_DEPTH_ALWAYS);
side.cubemap_face_ps.material_set(manager, gpumat);
Author
Member

Currently reuses previous resources, as the last one would be the world background pass this kinda works. But should be fixed.

Currently reuses previous resources, as the last one would be the world background pass this kinda works. But should be fixed.
Jeroen Bakker reviewed 2023-05-23 14:27:03 +02:00
@ -0,0 +52,4 @@
}
/* -------------------------------------------------------------------- */
/** \name World
Author
Member

Reflection Probe

Reflection Probe
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker reviewed 2023-05-23 14:29:44 +02:00
@ -46,0 +55,4 @@
struct CubemapSide {
PassSimple cubemap_face_ps;
View view;
Author
Member

Use a single View instance and fill with different view ids.

Use a single View instance and fill with different view ids.
Author
Member

As far as I have seen this requires other changes.

As far as I have seen this requires other changes.
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker added 53 commits 2023-05-25 10:13:53 +02:00
Spawning surfels uses a new shader pipeline (called `capture`) to avoid
the complexity of modifying the deferred or forward pipeline.
The shaders are light to compile and should only be compiled on bake.

This is very WIP: the surfel projection box is hardcoded and the 3
projections are done every frame. The surfels are placed in a regular
grid because of the raster pipeline used to spawn them.

Note: While this is really fast, I am not sure this will scale well for
highly detailed lighting conditions. In the case where the surfel density
is too low, the aliasing might create really poor result.
This removes the complexity of resource sharing as it isn't even leveraged
yet. So keep the code as simple as possible for now.
This allow probe update and deletion to trigger an auto bake.
This should allow to keep both version working during the period where
both eevee versions are available.
Was missing manager sync, a buffer read and copy-on-write tagging.
This fixes the light leaking issue caused by the surfel lists.
The fix consists in clumping more surfel together when creating the lists
then rewire the coplanar surfels to more valid surfels up and down the
same list.
This is still broken as the light seems to never converge.
Now it follows proper conventional notation.

# Conflicts:
#	source/blender/draw/engines/eevee_next/shaders/eevee_surfel_ray_comp.glsl
Light bouncing needs a feedback mechanism. We cannot reuse the same
radiance for scattering light and accumulating it.

This splits the final accumulation to `radiance` and bouncing light into
`incomming_light` and `outgoing_light`.
The framebuffer default size was only set during the first bind. This
is because the `dirty_attachments_ tag` wasn't set and thus the
framebuffer size was never passed down to the GL.

Split to `default_size_set()` to not affect other code paths that use
`size_set()`.
This adds a separate pass to project the neighbor surfels to the final
irradiance sample points.
Waiting for the commit to be in master.
This make the baking step output local space spherical harmonics which
need to be rotated back to world before being evaluated.
In practice we rotate the evaluated direction.
# Conflicts:
#	source/blender/blenkernel/intern/lightprobe.cc
#	source/blender/draw/engines/eevee_next/shaders/eevee_debug_surfels_frag.glsl
#	source/blender/editors/render/render_shading.cc
#	source/blender/makesdna/DNA_lightprobe_types.h
This approach allow streaming of visible grids and
has a controlled memory budget.
This remove the per irradiance grid approach. The idea is to do the light
transport only once for each visibility collection and store the
irradiance for all irradiance grid using this visibility mask.

This removes the need for the lightprobe near/far clip parameters.
This doesn't implement the world irradiance extraction process,
but only reserve the storage for it.
This avoid triggering the asserts in place in the Metal
backend. However it does not work yet.
# Conflicts:
#	source/blender/blenloader/intern/versioning_400.cc
Jeroen Bakker force-pushed eevee-next-world-shader from 7d435f0eaa to 773c58ba3c 2023-05-25 15:30:37 +02:00 Compare
Jeroen Bakker changed title from WIP: Eevee-Next: World Lighting to Eevee-Next: World Lighting 2023-05-25 15:33:50 +02:00
Jeroen Bakker requested review from Clément Foucault 2023-05-25 15:36:13 +02:00
Jeroen Bakker force-pushed eevee-next-world-shader from f6b871b57a to e3608d689d 2023-05-26 09:22:26 +02:00 Compare
Jeroen Bakker reviewed 2023-05-26 19:17:32 +02:00
@ -0,0 +1,16 @@
void light_world_eval(ClosureDiffuse diffuse,
Author
Member

Do we also need to update out_diffuse

Do we also need to update out_diffuse
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker added 1 commit 2023-05-30 08:13:28 +02:00
Jeroen Bakker added 1 commit 2023-06-01 08:06:41 +02:00
Jeroen Bakker added 1 commit 2023-06-01 08:26:06 +02:00
Jeroen Bakker added 2 commits 2023-06-01 08:54:07 +02:00
Jeroen Bakker changed title from Eevee-Next: World Lighting to Eevee-Next: World Reflective Light 2023-06-01 09:00:24 +02:00
Jeroen Bakker reviewed 2023-06-14 17:50:57 +02:00
@ -96,6 +96,7 @@
/* Only during shadow rendering. */
#define SHADOW_RENDER_MAP_SLOT 13
#define RBUFS_UTILITY_TEX_SLOT 14
#define REFLECTION_PROBE_TEX_SLOT 16
Author
Member

We should not bind on numbers higher than 15. As more changes do this we should check on the best solution. @fclem looking at open PR we need 4 new slots (SSS, Reflection Probes, Global Illumination).

We should not bind on numbers higher than 15. As more changes do this we should check on the best solution. @fclem looking at open PR we need 4 new slots (SSS, Reflection Probes, Global Illumination).
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker force-pushed eevee-next-world-shader from e52410f3d8 to 845406721f 2023-06-16 08:02:06 +02:00 Compare
Jeroen Bakker added 1 commit 2023-06-19 09:28:03 +02:00
Jeroen Bakker added 1 commit 2023-06-19 09:28:26 +02:00
Jeroen Bakker added 1 commit 2023-06-19 09:38:46 +02:00
Jeroen Bakker added 1 commit 2023-06-23 14:48:54 +02:00
Jeroen Bakker added 1 commit 2023-06-26 07:33:58 +02:00
Jeroen Bakker added 1 commit 2023-06-27 07:46:40 +02:00
Jeroen Bakker reviewed 2023-06-27 11:05:20 +02:00
@ -0,0 +7,4 @@
float lod = linear_roughness * lod_cube_max;
vec3 R = -reflect(V, reflection.N);
vec3 world_light = textureLod_cubemapArray(reflectionProbes, vec4(R, 0.0), lod).rgb;
Author
Member

Check lightprobe_filter_glossy_frag

Check `lightprobe_filter_glossy_frag`
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker reviewed 2023-06-27 11:08:42 +02:00
Jeroen Bakker added 1 commit 2023-06-27 12:28:06 +02:00
Jeroen Bakker added 1 commit 2023-06-27 14:40:36 +02:00
Jeroen Bakker added 1 commit 2023-06-27 16:13:55 +02:00
Jeroen Bakker added 2 commits 2023-06-29 08:50:18 +02:00
Jeroen Bakker added 1 commit 2023-06-29 08:55:55 +02:00
Jeroen Bakker added 1 commit 2023-06-29 10:06:18 +02:00
Clément Foucault requested changes 2023-06-29 10:50:03 +02:00
Clément Foucault left a comment
Member

I'm unsure about having a dedicated ReflectionProbeModule when we have LightProbeModule which does kind of the same thing (detecting updates and deletion). What was the incentive for adding another one? (maybe I told you so, if that's the reason, I'm sorry.) Maybe it was because the IrradianceCache patch wasn't merged yet.

I'm unsure about having a dedicated `ReflectionProbeModule` when we have `LightProbeModule` which does kind of the same thing (detecting updates and deletion). What was the incentive for adding another one? (maybe I told you so, if that's the reason, I'm sorry.) Maybe it was because the IrradianceCache patch wasn't merged yet.
@ -59,0 +74,4 @@
side.cubemap_face_ps.state_set(DRW_STATE_WRITE_COLOR | DRW_STATE_DEPTH_ALWAYS);
}
const int2 extent(1);
constexpr eGPUTextureUsage usage = GPU_TEXTURE_USAGE_MEMORYLESS | GPU_TEXTURE_USAGE_SHADER_WRITE;

Can we use GPU_TEXTURE_USAGE_MEMORYLESS with GPU_TEXTURE_USAGE_SHADER_WRITE here? sounds like potential issue.

Can we use GPU_TEXTURE_USAGE_MEMORYLESS with GPU_TEXTURE_USAGE_SHADER_WRITE here? sounds like potential issue.
Jeroen-Bakker marked this conversation as resolved
@ -46,0 +53,4 @@
private:
Instance &inst_;
Texture dummy_renderpass_tx_;

Add comment saying these are required in order to reuse the background shader and avoid another shader variation.

Add comment saying these are required in order to reuse the background shader and avoid another shader variation.
Jeroen-Bakker marked this conversation as resolved
@ -46,0 +58,4 @@
Texture dummy_aov_color_tx_;
Texture dummy_aov_value_tx_;
struct CubemapSide {

This struct shouldn't be contained by the pipeline. The pipeline should only contain the pass and be view agnostic.

So you should only have one pass that is being drawn for the 6 views.

This struct shouldn't be contained by the pipeline. The pipeline should only contain the pass and be view agnostic. So you should only have one pass that is being drawn for the 6 views.
Jeroen-Bakker marked this conversation as resolved
@ -46,0 +83,4 @@
public:
WorldProbePipeline(Instance &inst) : inst_(inst){};
void sync();

You shouldn't have 3 sync functions. Kind of related to my other comments: Prefer syncing one view agnostic render pass and always sync it from World::sync().

You shouldn't have 3 `sync` functions. Kind of related to my other comments: Prefer syncing one view agnostic render pass and always sync it from `World::sync()`.
Jeroen-Bakker marked this conversation as resolved
@ -288,6 +338,7 @@ class UtilityTexture : public Texture {
class PipelineModule {
public:
WorldPipeline world;

Maybe we could rename this to BackgroundPipeline and use WorldPipeline for world rendering. Could do that in another PR.

Maybe we could rename this to BackgroundPipeline and use WorldPipeline for world rendering. Could do that in another PR.
Author
Member

I did the change in #109495

I did the change in #109495
Jeroen-Bakker marked this conversation as resolved
@ -0,0 +43,4 @@
{
if (cubemap.type == ReflectionProbe::Type::World) {
GPUMaterial *world_material = instance_.world.get_world_material();
instance_.pipelines.world_probe.sync(world_material);

Prefer always syncing it inside World::sync() just like the other world pipeline is doing inst_.pipelines.world.sync(gpumat);.

Prefer always syncing it inside `World::sync()` just like the other world pipeline is doing `inst_.pipelines.world.sync(gpumat);`.
Jeroen-Bakker marked this conversation as resolved
@ -0,0 +35,4 @@
* \{ */
class ReflectionProbe {
public:
enum Type { Unused, World };

Style: Uppercase enum members.

Style: Uppercase enum members.
Jeroen-Bakker marked this conversation as resolved
@ -0,0 +46,4 @@
class ReflectionProbeModule {
private:
/** The max number of probes to track. */
static constexpr int MAX_PROBES = 1;

Style: No uppercase

Style: No uppercase
Jeroen-Bakker marked this conversation as resolved
@ -0,0 +53,4 @@
*
* Must be a power of two; intension to be used as a cubemap atlas.
*/
static constexpr int MAX_RESOLUTION = 2048;

I don't know if this is covered by the next patch, but this should become an option.

I don't know if this is covered by the next patch, but this should become an option.
Author
Member

Yes, that will be part of the reflection probe baking patch. Might even land earlier when specific parts of that patch are stable. Currently this part isn't stable to land in main yet.

Yes, that will be part of the reflection probe baking patch. Might even land earlier when specific parts of that patch are stable. Currently this part isn't stable to land in main yet.
Jeroen-Bakker marked this conversation as resolved
@ -0,0 +54,4 @@
* Must be a power of two; intension to be used as a cubemap atlas.
*/
static constexpr int MAX_RESOLUTION = 2048;
static constexpr int MIPMAP_LEVELS = 12;

Maybe derive it from MAX_RESOLUTION ? log2(MAX_RESOLUTION) + 1

Maybe derive it from `MAX_RESOLUTION` ? `log2(MAX_RESOLUTION) + 1`
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker added 3 commits 2023-06-29 11:45:01 +02:00
Jeroen Bakker added 3 commits 2023-06-29 13:32:18 +02:00
Jeroen Bakker added 1 commit 2023-06-29 13:44:05 +02:00
Jeroen Bakker added 1 commit 2023-06-29 13:50:54 +02:00
Jeroen Bakker added 1 commit 2023-06-29 13:53:36 +02:00
Jeroen Bakker added 1 commit 2023-06-29 13:54:53 +02:00
Clément Foucault requested changes 2023-06-29 14:05:28 +02:00
@ -194,0 +195,4 @@
void CaptureView::render()
{
if (!world_capture_enable_) {

Replace by inst_.reflection_probes.do_world_update.

Replace by `inst_.reflection_probes.do_world_update`.
@ -194,0 +210,4 @@
float4x4 view_m4 = cubeface_mat(face);
float4x4 win_m4;
cubeface_winmat_get(win_m4, 1.0f, 10.0f);

use math::projection::perspective()

use `math::projection::perspective()`
Jeroen-Bakker marked this conversation as resolved
Jeroen Bakker added 1 commit 2023-06-29 15:11:10 +02:00
Clément Foucault approved these changes 2023-06-29 15:15:07 +02:00
Jeroen Bakker added 1 commit 2023-06-29 15:19:15 +02:00
Jeroen Bakker merged commit fb38bb9806 into main 2023-06-29 15:25:04 +02:00
Jeroen Bakker deleted branch eevee-next-world-shader 2023-06-29 15:25:06 +02:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
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
Asset Browser Project
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 Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#108149
No description provided.