EEVEE Next: Subsurface Scattering #107407

Miguel Pozo · 2023-04-27T18:12:08+02:00

Miguel Pozo commented

2023-04-27 18:12:08 +02:00

Port of subsurface scattering from the eevee-rewrite branch to EEVEE Next.
https://projects.blender.org/fclem/blender/src/branch/eevee-rewrite

Other changes:

Fix gbuffer normal and ior packing.
Write the material eClosureBits to the gbuffer stencil.

Known Issues:

SSS Transmittance energy is greater than Cycles with the same settings.

Cycles	EEVEE Next

Port of subsurface scattering from the eevee-rewrite branch to EEVEE Next. https://projects.blender.org/fclem/blender/src/branch/eevee-rewrite Other changes: * Fix gbuffer normal and ior packing. * Write the material eClosureBits to the gbuffer stencil. Known Issues: * SSS Transmittance energy is greater than Cycles with the same settings. | Cycles | EEVEE Next | | --- | --- | | ![imagen](/attachments/22bfa61e-5772-4dc0-bbdd-cc41f2224a5c) | ![imagen](/attachments/3b74ac72-f3e0-4e7e-9c0d-9181ea299a17) |

imagen.png

616 KiB

eevee-next-sss_large_slabs.blend

1.1 MiB

imagen.png

299 KiB

❤️ 1 🚀 2

Miguel Pozo added this to the Viewport & EEVEE project 2023-04-27 18:12:43 +02:00

Miguel Pozo requested review from Clément Foucault 2023-04-27 18:12:53 +02:00

Miguel Pozo added the

Module

Viewport & EEVEE

label 2023-04-27 18:13:05 +02:00

Clément Foucault requested changes 2023-05-11 16:39:46 +02:00

source/blender/draw/engines/eevee_next/eevee_pipeline.cc Outdated

						
				@ -392,0 +395,4 @@

				  if (closure_bits_ & CLOSURE_SSS) {

				    inst_.subsurface.end_sync();

				    subsurface_ps_.init();

Clément Foucault commented

2023-05-11 12:16:08 +02:00

This subsurface pass should go inside SubsurfaceModule.

Note that, currently, the subsurface algorithm is quite simple, we might extend it in the future. Making everything in one place will avoid much trouble later on.

You can init the pass in all cases and only submit it if the closure bit is present.

This subsurface pass should go inside `SubsurfaceModule`. Note that, currently, the subsurface algorithm is quite simple, we might extend it in the future. Making everything in one place will avoid much trouble later on. You can init the pass in all cases and only submit it if the closure bit is present.

pragma37 marked this conversation as resolved

source/blender/draw/engines/eevee_next/shaders/infos/eevee_material_info.hh Outdated

						
				@ -183,0 +184,4 @@

				    .define("SSS_TRANSMITTANCE")

				    .sampler(SSS_TRANSMITTANCE_TEX_SLOT, ImageType::FLOAT_1D, "sss_transmittance_tx");

				GPU_SHADER_CREATE_INFO(eevee_subsurface_eval)

Clément Foucault commented

2023-05-11 11:06:12 +02:00

Move subsurface infos to their own file.

pragma37 marked this conversation as resolved

Miguel Pozo force-pushed pull-eevee-next-sss from e26cef6b9d to 1d28e81235

2023-05-12 16:01:09 +02:00

Compare

Miguel Pozo added 2 commits 2023-06-01 17:54:45 +02:00

Merge branch 'main' into pull-eevee-next-sss 7f2926f9cf

Diffuse Light Pass 1f47c1b122

Jeroen Bakker approved these changes 2023-06-14 09:03:05 +02:00

Jeroen Bakker left a comment

The PR has been discussed on blender.chat.

There is a difference between Cycles and Eevee-next. In a future iteration we should find out if we can get closer to Cycles.
Brecht did looked at the code and couldn't detect any issue.

I find the code to a level that it is fine to land in main.

The PR has been discussed on blender.chat. * There is a difference between Cycles and Eevee-next. In a future iteration we should find out if we can get closer to Cycles. * Brecht did looked at the code and couldn't detect any issue. I find the code to a level that it is fine to land in main.

Clément Foucault requested changes 2023-06-14 15:08:44 +02:00

Clément Foucault left a comment

It is a bit difficult to guess what is causing this energy difference. I would say add a corrective factor that you empirically fit the best you can and leave a comment saying this is a workaround and should be fixed at some point.

There were many places where I did that in the past. It is better to break scenes by correcting a 2% energy error than a 50% energy error (given it is only a scaling issue).

I would also like to point out that it is still broken on Metal. But it is a problem for the Metal backend and should not hold this patch from landing.

It is a bit difficult to guess what is causing this energy difference. I would say add a corrective factor that you empirically fit the best you can and leave a comment saying this is a workaround and should be fixed at some point. There were many places where I did that in the past. It is better to break scenes by correcting a 2% energy error than a 50% energy error (given it is only a scaling issue). I would also like to point out that it is still broken on Metal. But it is a problem for the Metal backend and should not hold this patch from landing.

source/blender/draw/engines/eevee_next/eevee_defines.hh Outdated

						
				@ -97,2 +97,4 @@

				#define SHADOW_RENDER_MAP_SLOT 13

				#define RBUFS_UTILITY_TEX_SLOT 14

				#define HIZ_TEX_SLOT 15

				#define SSS_TRANSMITTANCE_TEX_SLOT 16

Clément Foucault commented

2023-06-14 14:56:28 +02:00

Do not use slot 16 (or anything above slot 15). It is not available in all implementations.

pragma37 marked this conversation as resolved

source/blender/draw/engines/eevee_next/eevee_subsurface.cc

						
				@ -0,0 +50,4 @@

				  subsurface_ps_.bind_texture("gbuffer_closure_tx", &inst_.gbuffer.closure_tx);

				  subsurface_ps_.bind_texture("gbuffer_color_tx", &inst_.gbuffer.color_tx);

				  subsurface_ps_.bind_ubo(RBUFS_BUF_SLOT, &inst_.render_buffers.data);

				  subsurface_ps_.bind_image(RBUFS_COLOR_SLOT, &inst_.render_buffers.rp_color_tx);

Clément Foucault commented

2023-06-14 14:57:15 +02:00

Also bind RBUFS_VALUE_SLOT otherwise it will trigger a warning / validation error.

Also bind `RBUFS_VALUE_SLOT` otherwise it will trigger a warning / validation error.

pragma37 marked this conversation as resolved

Miguel Pozo added 5 commits 2023-06-14 18:07:49 +02:00

Merge branch 'main' into pull-eevee-next-sss aa7fc90c55

Remap texture slots 8d6a0f4e4d

Bind rp_value_tx d8ce569c21

Workaround for transmittance_profile values. 07dce2224b

Change default thickness 11c6138bc4

Miguel Pozo added 1 commit 2023-06-14 19:50:29 +02:00

Add texture slot workaround d34f22d0f4

Miguel Pozo commented

2023-06-14 19:58:09 +02:00

I've added a workaround so the values are closer to the Cycles ones, and mentioned the issue in the code comments. :(
I've also set the default thickness value to 0 so the result for very thin geometry matches Cycles and the current EEVE implementation better.

I've also "fixed" the texture slots issue, but I think we should look for a better solution.
I've opened another PR with one possible option: #108984

I've added a workaround so the values are closer to the Cycles ones, and mentioned the issue in the code comments. :( I've also set the default thickness value to 0 so the result for very thin geometry matches Cycles and the current EEVE implementation better. ![imagen](/attachments/7bd52f5b-89c7-4e70-a5a8-c9bf90710a02) I've also "fixed" the texture slots issue, but I think we should look for a better solution. I've opened another PR with one possible option: #108984

imagen.png

301 KiB

👍 1

Clément Foucault approved these changes 2023-06-15 10:07:07 +02:00

Clément Foucault left a comment

I am not sure if changing the default thickness to 0 is a good thing. This thickness it to avoid the ringing artifact caused by shadow map imprecision. The default is a good one for default settings and object size. Maybe we can make it 0.05 but clearly not 0.

Otherwise the patch can land.

I am not sure if changing the default thickness to 0 is a good thing. This thickness it to avoid the ringing artifact caused by shadow map imprecision. The default is a good one for default settings and object size. Maybe we can make it 0.05 but clearly not 0. Otherwise the patch can land.

👍 1

Miguel Pozo added 2 commits 2023-06-15 15:45:25 +02:00

Merge branch 'main' into pull-eevee-next-sss 9264ab9104

Revert "Change default thickness" fbe7c3c770