Cycles: Experiment with shadow catcher
ClosedPublic

Authored by Sergey Sharybin (sergey) on Feb 11 2016, 11:42 AM.
Tags
None
Tokens
"Like" token, awarded by kb0."Mountain of Wealth" token, awarded by johnroper100."Love" token, awarded by aliasguru."Love" token, awarded by hype."Love" token, awarded by aditiapratama."Love" token, awarded by looch."Love" token, awarded by venomgfx."Love" token, awarded by poor."Mountain of Wealth" token, awarded by electronicpulse."Love" token, awarded by juang3d."Love" token, awarded by GiantCowFIlms."Love" token, awarded by gandalf3."Love" token, awarded by saphires."Love" token, awarded by noemis."Love" token, awarded by quollism."Love" token, awarded by sebastian_k."Love" token, awarded by duarteframos."Love" token, awarded by januz."Love" token, awarded by Lapineige."Love" token, awarded by railla."Love" token, awarded by zuggamasta."Love" token, awarded by plasmasolutions."Love" token, awarded by plyczkowski.

Details

Summary

DISCLAIMER: This is more a code dump of a local branch, not somewhat really finished or so. Underlying math is the subject for rework since it's not quite physically based at all.

Publishing to start collaboration with other Cycles developers who are looking into solving this puzzle.

What do we consider a shadow catcher?

That's a good question actually, and there's no single formulation of what it exactly is and mathematically it's a bit malformed in the constraints we're working on. Ideally shadow catcher is a difference between image rendered without artificial objects and with them. Such approach gives best ever shadows, but takes 2x more time to render. So for good usability we need to get some assumptions, make system a bit more biased but give artists an useful tool.

Shadow catcher is mainly used by VFX artists to inject artificial objects into real footage. At least that definition we'll stick to
in Blender. Hence here's what shadow catcher should be capable of doing:

  • Receive shadows from other objects: be totally transparent when there's no shadows cast on it, be more opaque in shaded areas.
  • Ignore self-shadowing and shading. Shadows caused by occlusion with itself already exists in the footage. Same applies to the

shading -- all shading caused by material itself are also in the footage already.

  • Interact with other objects in the scene. This sounds a bit tricky but makes sense actually. Consider situation when one needs to put sharp glossy object into the footage: you'll want objects from a real scene to be reflected in the artificial object. And often you'll want the object on which shadow is to be cast to be reflected in such situations. Surely you can escape with copying object and playing with ray visibility, but that's complicated scene setup instead of making it simpler.
  • Be affected with indirect light. Cycles is the GI render engine after all!

How to use the shadow catcher?

  1. Create an object on which you want to receive shadow.
  2. Create some basic material setup which is close to a real object.
  3. Enable "Shadow Catcher" in Object buttons -> Cycles Settings.
  4. Be happy! (hopefully, once we've debugged all the code)

What this patch actually contains?

It contains all the bits which tries to implement definition of shadow catcher above. It is trying to implement it all in a way so we don't need to make big changes in the ray integration loop, hence it has some tricky magic to deduct what was the received shadow from the light passes and will fail in certain situations, mainly when there is no direct lighting of the object at all. It is totally tweakable to become more artists friendly, i just didn't have enough time to try all the ideas and used whatever latest semi-working formula was.

Major changes are in fact made around shadow_blocked() to exclude shading from self. This part is based on an older patch which tried to expose it to an user. That exposing settings are somewhat malformed and shouldn't really be used. In fact, we should remove those settings from the interface.

Some pictures?

Sure, here's one from a hackish patch:

(This is a glossy monkey on a checker board floor, floor is makred as a catcher, Here's .blend file

)

Diff Detail

Repository
rB Blender
Branch
cycles_shadowcatcher_v5
Build Status
Buildable 492
Build 492: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes

Fixes for AO

BPT gets closer to PT with change and it's seems logical thing to do anyway,
but somehow there is still some difference in AO between BPT and PT.

Also disabled shadow tricks for split kernel OpenCL -- it's a bit tricky to
implement them in this kernel.

applying the patch currently fails, the kernel_types.h.rej file says:

diff a/intern/cycles/kernel/kernel_types.h b/intern/cycles/kernel/kernel_types.h    (rejected hunks)
@@ -728,10 +744,11 @@
     SD_OBJECT_HAS_VOLUME        = (1 << 26),  /* object has a volume shader */
     SD_OBJECT_INTERSECTS_VOLUME = (1 << 27),  /* object intersects AABB of an object with volume shader */
     SD_OBJECT_HAS_VERTEX_MOTION = (1 << 28),  /* has position for motion vertices */
+    SD_OBJECT_SHADOW_CATCHER    = (1 << 29),  /* object is used to catch shadows */
 
     SD_OBJECT_FLAGS = (SD_HOLDOUT_MASK|SD_OBJECT_MOTION|SD_TRANSFORM_APPLIED|
                        SD_NEGATIVE_SCALE_APPLIED|SD_OBJECT_HAS_VOLUME|
-                       SD_OBJECT_INTERSECTS_VOLUME)
+                       SD_OBJECT_INTERSECTS_VOLUME|SD_OBJECT_SHADOW_CATCHER)
 };
 
 #ifdef __SPLIT_KERNEL__

Hey guys, I just realized in production tests this is self-shadowing. Are we doing something wrong? Is there anyway to remove that?

Rebase against latest master

More real work is upcoming.

Fix typo in light sampling

Was causing missing shadows in files with no world in certain cases.

Hi Sergey,

What are the chances to have the most anticipated patch of the century D1788 merged to master soon? The functionality seems to be complete, we're running the patched build from 3rd of October here without any issues. However, due to the speed of developers, this patch often becomes un-applyable after a short time period, at least for non-C++ guys. Would be nice to know if you're having plans to merge it with master?

best regards,
Rainer

I think the one really remaining issue is the self shadowing from Enviroment maps. The object that is a shadow catcher should never cast shadows

Mainly rebasing against latest master, don't expect any difference in behavior

@Lukas Stockner (lukasstockner97), i've tried implementing your idea of using non-MIS-weighted
numbers for the shadow, but there were following issues:

  • It made BPT somewhat darker shadows than with PT
  • I'm not sure we can store MIS weight in BsdfEval anymore: this weight is different for different terms in the BSDF eval sum, so don't think we can divide final sum bu overall MIS weight to get number we want?

Either i misunderstood which exact MIS you suggest to exclude from there or
did some mistakes in implementation. Think this is most relevant experiment
patch for this idea:

1diff --git a/intern/cycles/kernel/kernel_accumulate.h b/intern/cycles/kernel/kernel_accumulate.h
2index a87e6bf..04dc53f 100644
3--- a/intern/cycles/kernel/kernel_accumulate.h
4+++ b/intern/cycles/kernel/kernel_accumulate.h
5@@ -52,30 +52,36 @@ ccl_device_inline void bsdf_eval_init(BsdfEval *eval, ClosureType type, float3 v
6​ {
7​ eval->diffuse = value;
8​ }
9+ eval->mis_weight = 0.0f;
10​ }
11
12-ccl_device_inline void bsdf_eval_accum(BsdfEval *eval, ClosureType type, float3 value)
13+ccl_device_inline void bsdf_eval_accum(BsdfEval *eval,
14+ ClosureType type,
15+ float3 value,
16+ float mis_weight)
17​ {
18+ const float3 mis_value = value * mis_weight;
19​ #ifdef __PASSES__
20​ if(eval->use_light_pass) {
21​ if(CLOSURE_IS_BSDF_DIFFUSE(type))
22- eval->diffuse += value;
23+ eval->diffuse += mis_value;
24​ else if(CLOSURE_IS_BSDF_GLOSSY(type))
25- eval->glossy += value;
26+ eval->glossy += mis_value;
27​ else if(CLOSURE_IS_BSDF_TRANSMISSION(type))
28- eval->transmission += value;
29+ eval->transmission += mis_value;
30​ else if(CLOSURE_IS_BSDF_BSSRDF(type))
31- eval->subsurface += value;
32+ eval->subsurface += mis_value;
33​ else if(CLOSURE_IS_PHASE(type))
34- eval->scatter += value;
35+ eval->scatter += mis_value;
36
37​ /* skipping transparent, this function is used by for eval(), will be zero then */
38​ }
39​ else
40​ #endif
41​ {
42- eval->diffuse += value;
43+ eval->diffuse += mis_value;
44​ }
45+ eval->mis_weight += mis_weight;
46​ }
47
48​ ccl_device_inline bool bsdf_eval_is_zero(BsdfEval *eval)
49@@ -333,7 +339,7 @@ ccl_device_inline void path_radiance_accum_light(PathRadiance *L, float3 through
50​ }
51
52​ #ifdef __SHADOW_TRICKS__
53- float3 light = throughput * bsdf_eval_sum(bsdf_eval);
54+ float3 light = throughput * bsdf_eval_sum(bsdf_eval) / bsdf_eval->mis_weight;
55​ L->path_total += light;
56​ L->path_total_shaded += shadow * light;
57​ #endif
58@@ -345,7 +351,7 @@ ccl_device_inline void path_radiance_accum_total_light(
59​ const BsdfEval *bsdf_eval)
60​ {
61​ #ifdef __SHADOW_TRICKS__
62- L->path_total += throughput * bsdf_eval_sum(bsdf_eval);
63+ L->path_total += throughput * bsdf_eval_sum(bsdf_eval) / bsdf_eval->mis_weight;
64​ #else
65​ (void) L;
66​ (void) throughput;
67diff --git a/intern/cycles/kernel/kernel_shader.h b/intern/cycles/kernel/kernel_shader.h
68index c1b3153..2d8de0e 100644
69--- a/intern/cycles/kernel/kernel_shader.h
70+++ b/intern/cycles/kernel/kernel_shader.h
71@@ -509,7 +509,7 @@ ccl_device_inline void _shader_bsdf_multi_eval(KernelGlobals *kg, ShaderData *sd
72​ float3 eval = bsdf_eval(kg, sd, sc, omega_in, &bsdf_pdf);
73
74​ if(bsdf_pdf != 0.0f) {
75- bsdf_eval_accum(result_eval, sc->type, eval*sc->weight);
76+ bsdf_eval_accum(result_eval, sc->type, eval*sc->weight, 1.0f);
77​ sum_pdf += bsdf_pdf*sc->sample_weight;
78​ }
79
80@@ -537,7 +537,8 @@ ccl_device_inline void _shader_bsdf_multi_eval_branched(KernelGlobals *kg,
81​ float mis_weight = use_mis? power_heuristic(light_pdf, bsdf_pdf): 1.0f;
82​ bsdf_eval_accum(result_eval,
83​ sc->type,
84- eval * sc->weight * mis_weight);
85+ eval * sc->weight,
86+ mis_weight);
87​ }
88​ }
89​ }
90@@ -925,7 +926,7 @@ ccl_device_inline void _shader_volume_phase_multi_eval(const ShaderData *sd, con
91​ float3 eval = volume_phase_eval(sd, sc, omega_in, &phase_pdf);
92
93​ if(phase_pdf != 0.0f) {
94- bsdf_eval_accum(result_eval, sc->type, eval);
95+ bsdf_eval_accum(result_eval, sc->type, eval, 1.0f);
96​ sum_pdf += phase_pdf*sc->sample_weight;
97​ }
98
99diff --git a/intern/cycles/kernel/kernel_types.h b/intern/cycles/kernel/kernel_types.h
100index ea3d514..9c1dcca 100644
101--- a/intern/cycles/kernel/kernel_types.h
102+++ b/intern/cycles/kernel/kernel_types.h
103@@ -455,6 +455,7 @@ typedef struct BsdfEval {
104​ float3 subsurface;
105​ float3 scatter;
106​ #endif
107+ float mis_weight;
108​ } BsdfEval;
109
110​ /* Shader Flag */

Mind checking if i understood your correct?

@Brecht Van Lommel (brecht), can we start some discussion about what is the best approach to get
this into master? What i have in mind is:

  • Figure out whether i got Lukas's idea right.
  • Make this an Experimental option, so we keep some margin about making possible changes in the render results if we'll re-consider some parts of the change to work better with denoiser.

Committing this first as experimental seems ok.

I'm not sure what the MIS idea is, I can't find any discussion about that? The problem that I remember from when I tried to implement something similar (D524) is that handling the BSDF sampling component of MIS is difficult. For light sampling we trace a shadow ray, and it is relatively easy to compute a shadowed and unshadowed contribution. For BSDF sampling we compute it by piggy-backing on the indirect light path, but because of the way that works we never compute unshadowed contributions.

A workaround would be to disable lamp MIS for shadow catchers and only use light sampling (at the cost of more noise and inability to handle perfectly sharp glossy). Is that the intended idea here?

To support lamp MIS, an extra call to indirect_lamp_emission() with an infinite ray length could work, next to the call to indirect_background()? I don't fully understand yet how that part of the patch works, handling transparency and shadow catcher self hits correctly is not so simple.

intern/cycles/kernel/kernel_types.h
463

Suggest to rename to combined_unshadowed and combined_shadowed, shaded is a bit ambiguous terminology here.

That is pretty much what I had in mind, yes. The problem indeed seems to be just summing up the weights when using BPT.
I didn't extensively test this, but as far as I can see this should work:

1diff --git a/intern/cycles/kernel/kernel_accumulate.h b/intern/cycles/kernel/kernel_accumulate.h
2index a87e6bf..6444ba2 100644
3--- a/intern/cycles/kernel/kernel_accumulate.h
4+++ b/intern/cycles/kernel/kernel_accumulate.h
5@@ -52,10 +52,17 @@ ccl_device_inline void bsdf_eval_init(BsdfEval *eval, ClosureType type, float3 v
6​ {
7​ eval->diffuse = value;
8​ }
9+#ifdef __SHADOW_TRICKS__
10+ eval->sum_no_mis = make_float3(0.0f, 0.0f, 0.0f);
11+#endif
12​ }
13
14-ccl_device_inline void bsdf_eval_accum(BsdfEval *eval, ClosureType type, float3 value)
15+ccl_device_inline void bsdf_eval_accum(BsdfEval *eval, ClosureType type, float3 value, float mis_weight)
16​ {
17+#ifdef __SHADOW_TRICKS__
18+ eval->sum_no_mis += value;
19+#endif
20+ value *= mis_weight;
21​ #ifdef __PASSES__
22​ if(eval->use_light_pass) {
23​ if(CLOSURE_IS_BSDF_DIFFUSE(type))
24@@ -96,7 +103,7 @@ ccl_device_inline bool bsdf_eval_is_zero(BsdfEval *eval)
25​ }
26​ }
27
28-ccl_device_inline void bsdf_eval_mul(BsdfEval *eval, float value)
29+ccl_device_inline void bsdf_eval_mis(BsdfEval *eval, float value)
30​ {
31​ #ifdef __PASSES__
32​ if(eval->use_light_pass) {
33@@ -115,8 +122,19 @@ ccl_device_inline void bsdf_eval_mul(BsdfEval *eval, float value)
34​ }
35​ }
36
37+ccl_device_inline void bsdf_eval_mul(BsdfEval *eval, float value)
38+{
39+#ifdef __SHADOW_TRICKS__
40+ eval->sum_no_mis *= value;
41+#endif
42+ bsdf_eval_mis(eval, value);
43+}
44+
45​ ccl_device_inline void bsdf_eval_mul3(BsdfEval *eval, float3 value)
46​ {
47+#ifdef __SHADOW_TRICKS__
48+ eval->sum_no_mis *= value;
49+#endif
50​ #ifdef __PASSES__
51​ if(eval->use_light_pass) {
52​ eval->diffuse *= value;
53@@ -333,7 +351,7 @@ ccl_device_inline void path_radiance_accum_light(PathRadiance *L, float3 through
54​ }
55
56​ #ifdef __SHADOW_TRICKS__
57- float3 light = throughput * bsdf_eval_sum(bsdf_eval);
58+ float3 light = throughput * bsdf_eval->sum_no_mis;
59​ L->path_total += light;
60​ L->path_total_shaded += shadow * light;
61​ #endif
62@@ -345,7 +363,7 @@ ccl_device_inline void path_radiance_accum_total_light(
63​ const BsdfEval *bsdf_eval)
64​ {
65​ #ifdef __SHADOW_TRICKS__
66- L->path_total += throughput * bsdf_eval_sum(bsdf_eval);
67+ L->path_total += throughput * bsdf_eval->sum_no_mis;
68​ #else
69​ (void) L;
70​ (void) throughput;
71diff --git a/intern/cycles/kernel/kernel_shader.h b/intern/cycles/kernel/kernel_shader.h
72index c1b3153..e6fc3d6 100644
73--- a/intern/cycles/kernel/kernel_shader.h
74+++ b/intern/cycles/kernel/kernel_shader.h
75@@ -509,7 +509,7 @@ ccl_device_inline void _shader_bsdf_multi_eval(KernelGlobals *kg, ShaderData *sd
76​ float3 eval = bsdf_eval(kg, sd, sc, omega_in, &bsdf_pdf);
77
78​ if(bsdf_pdf != 0.0f) {
79- bsdf_eval_accum(result_eval, sc->type, eval*sc->weight);
80+ bsdf_eval_accum(result_eval, sc->type, eval*sc->weight, 1.0f);
81​ sum_pdf += bsdf_pdf*sc->sample_weight;
82​ }
83
84@@ -537,7 +537,8 @@ ccl_device_inline void _shader_bsdf_multi_eval_branched(KernelGlobals *kg,
85​ float mis_weight = use_mis? power_heuristic(light_pdf, bsdf_pdf): 1.0f;
86​ bsdf_eval_accum(result_eval,
87​ sc->type,
88- eval * sc->weight * mis_weight);
89+ eval * sc->weight,
90+ mis_weight);
91​ }
92​ }
93​ }
94@@ -569,7 +570,7 @@ void shader_bsdf_eval(KernelGlobals *kg,
95​ _shader_bsdf_multi_eval(kg, sd, omega_in, &pdf, -1, eval, 0.0f, 0.0f);
96​ if(use_mis) {
97​ float weight = power_heuristic(light_pdf, pdf);
98- bsdf_eval_mul(eval, weight);
99+ bsdf_eval_mis(eval, weight);
100​ }
101​ }
102​ }
103@@ -925,7 +926,7 @@ ccl_device_inline void _shader_volume_phase_multi_eval(const ShaderData *sd, con
104​ float3 eval = volume_phase_eval(sd, sc, omega_in, &phase_pdf);
105
106​ if(phase_pdf != 0.0f) {
107- bsdf_eval_accum(result_eval, sc->type, eval);
108+ bsdf_eval_accum(result_eval, sc->type, eval, 1.0f);
109​ sum_pdf += phase_pdf*sc->sample_weight;
110​ }
111
112diff --git a/intern/cycles/kernel/kernel_types.h b/intern/cycles/kernel/kernel_types.h
113index ea3d514..b172392 100644
114--- a/intern/cycles/kernel/kernel_types.h
115+++ b/intern/cycles/kernel/kernel_types.h
116@@ -455,6 +455,9 @@ typedef struct BsdfEval {
117​ float3 subsurface;
118​ float3 scatter;
119​ #endif
120+#ifdef __SHADOW_TRICKS__
121+ float3 sum_no_mis;
122+#endif
123​ } BsdfEval;
124
125​ /* Shader Flag */

Of course the naming could be improved...

@Brecht Van Lommel (brecht): The issue that I mentioned to @Sergey Sharybin (sergey) on IRC was that since the shadowcatcher only considers directly sampled light, including the MIS weight in the values used for it is wrong. My (uncommitted) fix in the denoiser branch was to just return the value from shader_eval_bsdf through direct_emission since I didn't really test BPT yet and didn't use BsdfEval, but the paste above is a cleaner fix.

As for compatibility with denoising, that's looking great - the two PathRadiance values are always generated, so they can just be used when writing the feature passes.

FYI: Currently the patch fails to apply in three modules (tried applying in blender-v2.78b-release branch on commit c727bc7):

  • ui.py
  • bvh_shadow_all.h
  • qbvh_shadow_all.h

The last commit when applying worked was 2ae39ff

Just as a side note: We're using the ShadowCatcher patch in production for a few months now without any issues.

Update against latest master

Update once again, used wrong flag in previous update

Just as a side note: We're using the ShadowCatcher patch in production for a few months now without any issues.

@Rainer Trummer (aliasguru) Did you use the SHADOW_CATCHER_BACKGROUND define to respect the film transparency setting? I would vote for always have that enabled (aka remove the ifndef parts) as this proved to be intuitively right and led to the correct output in all of our productions . How did you handle it?

@Sergey Sharybin (sergey) thanks for the rebase!

@Rainer Trummer (aliasguru) Did you use the __SHADOW_CATCHER_BACKGROUND__ define to respect the film transparency setting? I would vote

We are using this patch in Cycles for Rhino with __SHADOW_CATCHER_BACKGROUND__ enabled.

One of our testers asked if it is possible to have shadows on catcher objects be reflected in other objects as well, but I think that'll be very hard to do in a good way.

@Thomas Beck (plasmasolutions) I Just checked out the master Branch and patched it with the current version of this patch here (and a few others in addition, like the SurfaceFollow Modifier one). But I haven't changed any ifndef defines due to the total lack of C++ experience. I was only dealing with Python and C# so far.

Where would I need to change those definitions, and to what? I see an #ifdef Block in both kernel_path.h and kernel_path_branched.h, are those the files?

Thanks also to @Sergey Sharybin (sergey) for once again updating this patch!

Where would I need to change those definitions, and to what? I see an #ifdef Block in both kernel_path.h and kernel_path_branched.h, are those the files?

In intern/cycles/kernel/kernel_types.h (search for "kernel features" and add it there).

Cheers Thomas

Several changes:

  • Updated against latest master
  • Enabled background color by default now (seems to match to what people are requesting).
  • Worked around issue in scenes where there's just a shadow catcher and background (such case would cast shadow on the shadow catcher caused by indirect background light). Not fully happy here with the solution, but not sure what we can do better here.
  • Applied patch from Lukas so hopefully we are more aligned with denoise branch.

Worked around compilation error of OpenCL kernel

I tested the latest patch and found it working nicely.
It behaves a bit different than before where i could just add a bit tranparency to the shader to preserve
bottom reflection.
This can now be obtained other way ( see image ) so i would say all works as expected.
Also the hdri MIS problem when mixing various lamp types seems to be gone.
Made a lot of A/B comparison to find odd effects from SC to no avail.

Image 1: pure hdri


Image 2: hdri + sunlamp

Jens

Hi @Sergey Sharybin (sergey),

tested it as well in many different ways: Different light types, one light, multiple lights, colored lights, no lights, no world, plain world lighting, >1 shadow catcher, ... everything seems to work as intended!
There's only one case where I was unsure if the result is correct: textures on the catcher seem to give weird shadowing with point or spot lamps (no bump or displacement was used):

I'll append the blend here just in case it's indeed incorrect:

One last question: Is it somehow possible to make the catcher behave as if it was a real object (in the sense of coloring the shadow with the object color) or is the only option to tint it in post?

Cheers and thanks, Thomas

One last question: Is it somehow possible to make the catcher behave as if it was a real object (in the sense of coloring the shadow with the object color) or is the only option to tint it in post?

Not sure I understand this, but assuming you are blending the shadow on top of some colored object, that color would come through in the non-shadowed or partially shadowed areas. Anything else seems like it would be double counting the object color? Shadows don't have the color of the object they are on.

@Brecht Van Lommel (brecht) Yep, that was far too imprecise from me. What I meant was if the shadow could be optionally coloured by the bounce light from a coloured object when this object is near the catcher. But that can be perfectly done with the Diffuse Indirect pass, so ignore that ... was too late apparently.

I'm merging this into the denoising branch now, and it looks pretty good so far. Three minor things noted inline.

intern/cycles/kernel/kernel_accumulate.h
376

If the whole state is passed anyways, why keep the bounce as a separate argument?

intern/cycles/kernel/kernel_path.h
455

I'd move shadow_color into PathRadiance - it's not really necessary, but it would be nice to have it next to the other two shadowcatcher variables.

Also, in the denoising branch the L_sum computation happens outside of kernel_path_integrate, so it would save an additional parameter there (since the PathRadiance is already passed in as a pointer from kernel_path_trace).

intern/cycles/kernel/split/kernel_background_buffer_update.h
161 ↗(On Diff #8250)

Why not pass state here as well?

intern/cycles/kernel/kernel_accumulate.h
376

Initial provision was to avoid dereference of the argument when building without shadow tricks. But guess that;s not important since all platforms should be including that sooner or later (OpenCL split might be a problem).

Let's just switch to single state here.

intern/cycles/kernel/kernel_path.h
455

Don't have strong opinion here. If that makes easier to merge with denoiser, let's just move it.

intern/cycles/kernel/split/kernel_background_buffer_update.h
161 ↗(On Diff #8250)

Was giving some usual issues with address spaces. But now i look into the code and seems we can just change it to path_radiance_accum_background(ccl_addrspace State*) without issues.

Not sure why that wasn't done in original change. Will doublecheck and update.

@Thomas Beck (plasmasolutions), your issue was caused by using low light threshold, which was
not compatible with the way how we store catched shadow. I disabled the light
threshold if there is a shadow catcher on a way. Should be good enough for now

Real object will have black shadow, shadows are colorless. @Brecht Van Lommel (brecht) already explained
this and you seems to find a workaround which works for you.

@Lukas Stockner (lukasstockner97), hopefully i've address all points fro myou.

intern/cycles/blender/addon/properties.py
1101

Maybe use a description like this:

"Only render shadows for this object, for compositing renders into real footage"

intern/cycles/kernel/kernel_path_surface.h
27–32

Shouldn't this be done in direct_emission?

When only setting the random number to 0, it will still divide by the termination probability even though it's never terminating.

intern/cycles/blender/addon/properties.py
1101

Pardon my jumping in here, but I'd like to suggest using "on" instead of "for"; to me "for" suggests that shadows cast by the object are what will be rendered, rather than shadows caught by the object.

So:

Only render shadows on this object, for compositing renders into real footage

Update for review from Brecht

The patch needs a manual rebase now that the split kernel changes are in master - If you're fine with it, I can do that since I already did it for the denoising branch.

Also, I noticed something confusing in the code, I added an inline comment.

intern/cycles/kernel/kernel_path_surface.h
75

These else-blocks are inconsistent - in the all-lights branch, path_radiance_accum_total_light is called when the shadow test fails, while in the two single-light cases it's called when direct_emission fails.

Afaics all four of them should be in case shadow_blocked fails, or is there a specific reason for the difference?

Update against latest master and address issue raised by Lukas

intern/cycles/kernel/kernel_path_surface.h
75

There is no specific reason. Was a mistake in the early patch and later on i needed to go all-light and fixed the code there. Other places i just keeping forgetting.

Updating the patch against latest master, will send it here soon.

RUben (kb0) added a subscriber: RUben (kb0).

Read through the code quickly, looks good to me. Just found one little code style issue. :)

intern/cycles/kernel/kernel_path.h
640

Code style.

This revision is now accepted and ready to land.Mar 21 2017, 9:57 AM

This update includes:

  • Rebase against latets master
  • Style fix reported by Thomas
  • OpenCL implementation by Hristo
  • OpenCL tweak to solve speed regression

Our local benchmark machine shows 1% slowdown on RX480. Not sure yet
whether it comes from the flackynbess of the system (it seems to be
slower than our previous machine) or it's a slowdown caused by our
changes.

If more people tests OpenCL before/after this patch on scenes without
shadow catcher it'll be great.

Update. I've loaded optimzied defaults in BIOS and now i don't see measurable difference in performance before/after the patch.

YAFU (YAFU) added a comment.EditedMar 21 2017, 6:55 PM

Hi Sergey.
It seems Shadow Catcher is working well in my tests, it seems to react well to the variation of different lights:
Edit: corrected the previous scene (had forgotten to select to repeat with offset in one of the modifiers):

Road image packed. It is from here:
https://www.pexels.com/search/road/

Scene is animated, Intensity of light varies during animation. Shadow Catcher plane in layer 3.

But I have a doubt about scenes like those where there are colored lights. How to capture those colored lights? :)
Just wanting to know this to be able to do a better comparison side by side, Shadow Catcher vs No Shadow Catcher.

Add missing light accumulation to next iteration setup kernel

Okay, just did the (hopefully) final review pass.
I'm fine with committing as it is right now, but I added three inline comments regarding things that could still be changed.

Some areas need slight changes to work with the denoiser, but since they're not needed for the shadowcatcher itself I'd just include them in the upcoming denoiser review instead of adding unrelated changes here already.

intern/cycles/kernel/kernel_accumulate.h
397

Seems redundant to have the same path_total increment inside both parts of the if.

intern/cycles/kernel/kernel_path_volume.h
120

I might be overlooking something, but that change seems unrelated to the shadowcatcher.

intern/cycles/kernel/kernel_shadow.h
320

It seems a bit excessive to run a full shader_setup_from_ray, which handles things like differentials and smooth normals, just to get P and Ng.
On the other hand, this code is only executed in one specific case, so it might not actually matter enough to justify duplicating code.

Thanks for the review!

Will apply the (update to current review) patch tomorrow when i'll be back to studio and when i'll be available online to fix possible issues we overview.

intern/cycles/kernel/kernel_accumulate.h
397

Indeed. Also we should perhaps pre-calculate throughput*value. Not sure all compilers will optimize those multiplications to a single operation.

intern/cycles/kernel/kernel_path_volume.h
120

You're right. Must have been left after some other experiments. Will remove from the patch.

intern/cycles/kernel/kernel_shadow.h
320

That is correct. However, this is how this was working for stepped shadow blocked check already and in some other areas. I remember experimenting with lighter version of shader_setup_from_ray() which will only calculate normal, but that turned out to be a bit annoying, because there are still quite some calculations to be done there: mainly, need to refine intersection. Surely, we can skip differencials still. Will blow the dust out of that patch and apply separately. Don't want to put all possible optimizations into a single commit :)

On "environment" output shadow catcher produce black area. Therefore is complex to use manual compositing. I think shadow catcher must be transparent on "environment " and "Z" outputs.

intern/cycles/kernel/kernel_path_volume.h
120

Actually, this is part of ccl_addr_space change needed for OpenCL support.

@Maksim (m0xf), current design of Cycles does not allow one to easily ignore some objects for Z depth. There is similar issue happens with Z pass for volumes.

As for making catcher being "transparent" for Environment pass i'll look into, but will need to gather more feedback from other artists.

The initial implementation is in master now. Hopefully i did not forget anything from the feedback.