EEVEE-Next: Jittered Soft Shadows #119753

Closed
Miguel Pozo wants to merge 46 commits from pragma37/blender:pull-eevee-jittered-shoft-shadows into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
9 changed files with 27 additions and 14 deletions
Showing only changes of commit 3f991d6be8 - Show all commits

View File

@ -117,8 +117,11 @@ class DATA_PT_EEVEE_light(DataButtonsPanel, Panel):
col.separator()
col.prop(light, "use_shadow", text="Cast Shadow")
col.prop(light, "use_shadow_jittering", text="Shadow Jittering")
pragma37 marked this conversation as resolved Outdated

You don't need to give a text to properties which alread have the same name.

You don't need to give a `text` to properties which alread have the same name.
sub = col.column(align=True)
sub.active = light.use_shadow_jittering
sub.prop(light, "shadow_jitter_overblur", text="Jittering Overblur")
col.prop(light, "shadow_softness_factor", text="Shadow Softness")
col.prop(light, "shadow_jittering", text="Shadow Jittering")
col.prop(light, "shadow_filter_radius", text="Filtering Radius")
col.prop(light, "shadow_resolution_scale", text="Resolution Scale")

View File

@ -116,12 +116,16 @@ void Light::sync(ShadowModule &shadows, const Object *ob, float threshold)
* So we use the original light radius instead. */
shadow_radius = la->shadow_softness_factor * la->radius;
}
float softness = la->shadow_softness_factor;
if (this->do_jittering) {
pragma37 marked this conversation as resolved Outdated

This becomes way too hard to keep in mind all these different versions of radius. It's better to just pass light radius, the jitter amount and the tracing amount. The clamping for the shadow projection near/far clip should be inside compute_projection_boundaries.

This becomes way too hard to keep in mind all these different versions of radius. It's better to just pass light radius, the jitter amount and the tracing amount. The clamping for the shadow projection near/far clip should be inside `compute_projection_boundaries`.

It's better to just pass light radius

The problem is, what radius? The original or the one set by the Light module?
The compute_projection_boundaries receives both.

I've cleaned the code a bit. We can clarify the naming, but keeping all the separate parameters around and having the softness/jittering logic scattered over the module is going to make things way more confusing, IMO.

> It's better to just pass light radius The problem is, what radius? The original or the one set by the Light module? The `compute_projection_boundaries` receives both. I've cleaned the code a bit. We can clarify the naming, but keeping all the separate parameters around and having the softness/jittering logic scattered over the module is going to make things way more confusing, IMO.

I'm fine with the new code that passes the light data struct. But you should not copy the value of do_jittering here.

So, as I previously suggested, keep the bl_light->shadow_softness_factor, bl_light->shadow_jitter_overblur and bl_light->radius inside ShadowPunctual and do the clamping compute_projection_boundaries. Currently you only moved the problem down a function.

And yes, I would use radius_jitter and radius_raytrace for clarity. I suggest to have them as getters (i.e: shadow_punctual.radius_jitter_get(bool do_jitter)).

I'm fine with the new code that passes the light data struct. But you should not copy the value of `do_jittering` here. So, as I previously suggested, keep the `bl_light->shadow_softness_factor`, `bl_light->shadow_jitter_overblur` and `bl_light->radius` inside `ShadowPunctual` and do the clamping `compute_projection_boundaries`. Currently you only moved the problem down a function. And yes, I would use `radius_jitter` and `radius_raytrace` for clarity. I suggest to have them as getters (i.e: `shadow_punctual.radius_jitter_get(bool do_jitter)`).
softness *= la->shadow_jitter_overblur;
}
this->punctual->sync(this->type,
this->object_mat,
la->spotsize,
radius,
this->influence_radius_max,
la->shadow_softness_factor,
softness,
shadow_radius);
}
}
@ -319,9 +323,9 @@ void LightModule::sync_light(const Object *ob, ObjectHandle &handle)
light.used = true;
::Light *bl_light = static_cast<::Light *>(ob->data);
light.jittering = inst_.shadows.do_jittering() ? bl_light->shadow_jittering : 0.0f;
light.do_jittering = inst_.shadows.do_jittering() && bl_light->mode & LA_SHADOW_JITTER;
if (handle.recalc != 0 || !light.initialized || light.jittering > 0.0f) {
if (handle.recalc != 0 || !light.initialized || light.do_jittering) {
light.initialized = true;
light.sync(inst_.shadows, ob, light_threshold_);
}
pragma37 marked this conversation as resolved Outdated

unused var

unused var

It was at some point, but not since I added the toggle on viewport playback/navigation.

It was at some point, but not since I added the toggle on viewport playback/navigation.

View File

@ -44,6 +44,7 @@ struct Light : public LightData, NonCopyable {
public:
bool initialized = false;
bool used = false;
bool do_jittering;
/** Pointers to source Shadow. Type depends on `LightData::type`. */
ShadowDirectional *directional = nullptr;

View File

@ -822,7 +822,7 @@ struct LightData {
/** Punctual: Shift to apply to the light origin to get the shadow projection origin. */
float shadow_projection_shift;
packed_float3 shadow_origin_shift;
float jittering;
float _pad0;
};
BLI_STATIC_ASSERT_ALIGN(LightData, 16)

View File

@ -348,7 +348,7 @@ void ShadowPunctual::end_sync(Light &light, float lod_bias, Sampling &sampling)
float3 origin_shift = float3(0.0f);
if (light.jittering > 0.0f) {
if (light.do_jittering) {
/* TODO: de-correlate. */
float3 random = sampling.rng_3d_get(SAMPLING_SHADOW_U);
if (is_area_light(light.type)) {
@ -359,9 +359,8 @@ void ShadowPunctual::end_sync(Light &light, float lod_bias, Sampling &sampling)
}
else {
origin_shift = sampling.sample_ball(random);
origin_shift *= sqrtf(light.radius_squared);
origin_shift *= shadow_radius_;
}
origin_shift *= light.jittering;
}
/* Shift shadow map origin for area light to avoid clipping nearby geometry. */

View File

@ -89,6 +89,8 @@ void shadow_tag_usage_tilemap_punctual(
}
vec3 lP = light_world_to_local(light, P - light._position);
lP -= light.shadow_origin_shift;
pragma37 marked this conversation as resolved Outdated

This is invalid to shift here. The visibility computation for early out should use the non-jittered position.

This is invalid to shift here. The visibility computation for early out should use the non-jittered position.
float dist_to_light = max(length(lP) - radius, 1e-5);
if (dist_to_light > light.influence_radius_max) {
return;
@ -107,7 +109,6 @@ void shadow_tag_usage_tilemap_punctual(
}
}
lP -= light.shadow_origin_shift;
lP.z -= light.shadow_projection_shift;
float footprint_ratio = shadow_punctual_footprint_ratio(

View File

@ -294,8 +294,6 @@ ShadowRayPunctual shadow_ray_generate_punctual(LightData light,
random_2d = sample_disk(random_2d);
}
random_2d *= 1.0 - light.jittering;
float clip_far = intBitsToFloat(light.clip_far);
float clip_near = intBitsToFloat(light.clip_near);
float clip_side = light.clip_side;

View File

@ -79,7 +79,7 @@ typedef struct Light {
float shadow_trace_distance;
float shadow_filter_radius;
float shadow_resolution_scale;
float shadow_jittering;
float shadow_jitter_overblur;
pragma37 marked this conversation as resolved Outdated

We should give is a small default. 0.1 looks fine.

We should give is a small default. `0.1` looks fine.
/* Preview */
struct PreviewImage *preview;
@ -142,6 +142,7 @@ enum {
LA_SHAD_CONTACT = 1 << 19,
LA_CUSTOM_ATTENUATION = 1 << 20,
LA_USE_SOFT_FALLOFF = 1 << 21,
LA_SHADOW_JITTER = 1 << 22
};
/** #Light::falloff_type */

View File

@ -315,10 +315,16 @@ static void rna_def_light_shadow(StructRNA *srna, bool sun)
RNA_def_property_override_flag(prop, PROPOVERRIDE_OVERRIDABLE_LIBRARY);
RNA_def_property_update(prop, 0, "rna_Light_update");
prop = RNA_def_property(srna, "shadow_jittering", PROP_FLOAT, PROP_FACTOR);
prop = RNA_def_property(srna, "use_shadow_jittering", PROP_BOOLEAN, PROP_NONE);
RNA_def_property_boolean_sdna(prop, nullptr, "mode", LA_SHADOW_JITTER);
RNA_def_property_ui_text(prop, "Shadow Jittering", "Shadow Jittering (TODO)");
RNA_def_property_override_flag(prop, PROPOVERRIDE_OVERRIDABLE_LIBRARY);
RNA_def_property_update(prop, 0, "rna_Light_update");
prop = RNA_def_property(srna, "shadow_jitter_overblur", PROP_FLOAT, PROP_FACTOR);
RNA_def_property_range(prop, 0.0f, 1.0);
RNA_def_property_ui_range(prop, 0.0f, 1.0f, 0.1f, 2);
RNA_def_property_ui_text(prop, "Shadow Jittering", "Shadow Jittering (TODO)");
RNA_def_property_ui_text(prop, "Shadow Jitter Overblur", "Shadow Jitter Overblur (TODO)");
RNA_def_property_override_flag(prop, PROPOVERRIDE_OVERRIDABLE_LIBRARY);
RNA_def_property_update(prop, 0, "rna_Light_update");