Code refactor: make mixed small/large BSSRDF radii more robust.

3 files changed, 119 insertions, 108 deletions

diff --git a/intern/cycles/kernel/closure/bssrdf.h b/intern/cycles/kernel/closure/bssrdf.h
index 383c168463b..c8f505e8418 100644
--- a/intern/cycles/kernel/closure/bssrdf.h
+++ b/intern/cycles/kernel/closure/bssrdf.h
@@ -27,6 +27,7 @@ typedef ccl_addr_space struct Bssrdf {
 	float sharpness;
 	float texture_blur;
 	float roughness;
+	float channels;
 } Bssrdf;
 
 /* Planar Truncated Gaussian
@@ -343,42 +344,68 @@ ccl_device_inline Bssrdf *bssrdf_alloc(ShaderData *sd, float3 weight)
 	return (sample_weight >= CLOSURE_WEIGHT_CUTOFF) ? bssrdf : NULL;
 }
 
-ccl_device int bssrdf_setup(Bssrdf *bssrdf, ClosureType type)
+ccl_device int bssrdf_setup(ShaderData *sd, Bssrdf *bssrdf, ClosureType type)
 {
-	if(max3(bssrdf->radius) < BSSRDF_MIN_RADIUS) {
-		/* revert to diffuse BSDF if radius too small */
-		int flag;
+	int flag = 0;
+	int bssrdf_channels = 3;
+	float3 diffuse_weight = make_float3(0.0f, 0.0f, 0.0f);
+
+	/* Verify if the radii are large enough to sample without precision issues. */
+	if(bssrdf->radius.x < BSSRDF_MIN_RADIUS) {
+		diffuse_weight.x = bssrdf->weight.x;
+		bssrdf->weight.x = 0.0f;
+		bssrdf->radius.x = 0.0f;
+		bssrdf_channels--;
+	}
+	if(bssrdf->radius.y < BSSRDF_MIN_RADIUS) {
+		diffuse_weight.y = bssrdf->weight.y;
+		bssrdf->weight.y = 0.0f;
+		bssrdf->radius.y = 0.0f;
+		bssrdf_channels--;
+	}
+	if(bssrdf->radius.z < BSSRDF_MIN_RADIUS) {
+		diffuse_weight.z = bssrdf->weight.z;
+		bssrdf->weight.z = 0.0f;
+		bssrdf->radius.z = 0.0f;
+		bssrdf_channels--;
+	}
+
+	if(bssrdf_channels < 3) {
+		/* Add diffuse BSDF if any radius too small. */
 #ifdef __PRINCIPLED__
 		if(type == CLOSURE_BSSRDF_PRINCIPLED_ID) {
 			float roughness = bssrdf->roughness;
 			float3 N = bssrdf->N;
-			float3 weight = bssrdf->weight;
-			float sample_weight = bssrdf->sample_weight;
 
-			PrincipledDiffuseBsdf *bsdf = (PrincipledDiffuseBsdf*)bssrdf;
+			PrincipledDiffuseBsdf *bsdf = (PrincipledDiffuseBsdf*)bsdf_alloc(sd, sizeof(PrincipledDiffuseBsdf), diffuse_weight);
 
-			bsdf->N = N;
-			bsdf->roughness = roughness;
-			bsdf->weight = weight;
-			bsdf->sample_weight = sample_weight;
-			flag = bsdf_principled_diffuse_setup(bsdf);
-			bsdf->type = CLOSURE_BSDF_BSSRDF_PRINCIPLED_ID;
+			if(bsdf) {
+				bsdf->type = CLOSURE_BSDF_BSSRDF_PRINCIPLED_ID;
+				bsdf->N = N;
+				bsdf->roughness = roughness;
+				flag |= bsdf_principled_diffuse_setup(bsdf);
+			}
 		}
 		else
 #endif  /* __PRINCIPLED__ */
 		{
-			DiffuseBsdf *bsdf = (DiffuseBsdf*)bssrdf;
-			bsdf->N = bssrdf->N;
-			flag = bsdf_diffuse_setup(bsdf);
-			bsdf->type = CLOSURE_BSDF_BSSRDF_ID;
+			DiffuseBsdf *bsdf = (DiffuseBsdf*)bsdf_alloc(sd, sizeof(DiffuseBsdf), diffuse_weight);
+
+			if(bsdf) {
+				bsdf->type = CLOSURE_BSDF_BSSRDF_ID;
+				bsdf->N = bssrdf->N;
+				flag |= bsdf_diffuse_setup(bsdf);
+			}
 		}
-		
-		return flag;
 	}
-	else {
+
+	/* Setup BSSRDF if radius is large enough. */
+	if(bssrdf_channels > 0) {
+		bssrdf->type = type;
+		bssrdf->channels = bssrdf_channels;
+		bssrdf->sample_weight = fabsf(average(bssrdf->weight)) * bssrdf->channels;
 		bssrdf->texture_blur = saturate(bssrdf->texture_blur);
 		bssrdf->sharpness = saturate(bssrdf->sharpness);
-		bssrdf->type = type;
 
 		if(type == CLOSURE_BSSRDF_BURLEY_ID ||
 		   type == CLOSURE_BSSRDF_PRINCIPLED_ID)
@@ -386,8 +413,14 @@ ccl_device int bssrdf_setup(Bssrdf *bssrdf, ClosureType type)
 			bssrdf_burley_setup(bssrdf);
 		}
 
-		return SD_BSSRDF;
+		flag |= SD_BSSRDF;
 	}
+	else {
+		bssrdf->type = type;
+		bssrdf->sample_weight = 0.0f;
+	}
+
+	return flag;
 }
 
 ccl_device void bssrdf_sample(const ShaderClosure *sc, float xi, float *r, float *h)
@@ -395,17 +428,22 @@ ccl_device void bssrdf_sample(const ShaderClosure *sc, float xi, float *r, float
 	const Bssrdf *bssrdf = (const Bssrdf*)sc;
 	float radius;
 
-	/* Sample color channel and reuse random number. */
-	if(xi < 1.0f/3.0f) {
-		xi *= 3.0f;
-		radius = bssrdf->radius.x;
+	/* Sample color channel and reuse random number. Only a subset of channels
+	 * may be used if their radius was too small to handle as BSSRDF. */
+	xi *= bssrdf->channels;
+
+	if(xi < 1.0f) {
+		radius = (bssrdf->radius.x > 0.0f)? bssrdf->radius.x:
+		         (bssrdf->radius.y > 0.0f)? bssrdf->radius.y:
+		                                    bssrdf->radius.z;
 	}
-	else if(xi < 2.0f/3.0f) {
-		xi = (xi - 1.0f/3.0f)*3.0f;
-		radius = bssrdf->radius.y;
+	else if(xi < 2.0f) {
+		xi -= 1.0f;
+		radius = (bssrdf->radius.x > 0.0f)? bssrdf->radius.y:
+		                                    bssrdf->radius.z;
 	}
 	else {
-		xi = (xi - 2.0f/3.0f)*3.0f;
+		xi -= 2.0f;
 		radius = bssrdf->radius.z;
 	}
 
@@ -423,7 +461,10 @@ ccl_device void bssrdf_sample(const ShaderClosure *sc, float xi, float *r, float
 
 ccl_device float bssrdf_channel_pdf(const Bssrdf *bssrdf, float radius, float r)
 {
-	if(bssrdf->type == CLOSURE_BSSRDF_CUBIC_ID) {
+	if(radius == 0.0f) {
+		return 0.0f;
+	}
+	else if(bssrdf->type == CLOSURE_BSSRDF_CUBIC_ID) {
 		return bssrdf_cubic_pdf(radius, bssrdf->sharpness, r);
 	}
 	else if(bssrdf->type == CLOSURE_BSSRDF_GAUSSIAN_ID) {
@@ -446,7 +487,10 @@ ccl_device_forceinline float3 bssrdf_eval(const ShaderClosure *sc, float r)
 
 ccl_device_forceinline float bssrdf_pdf(const ShaderClosure *sc, float r)
 {
-	return average(bssrdf_eval(sc, r));
+	const Bssrdf *bssrdf = (const Bssrdf*)sc;
+	float3 pdf = bssrdf_eval(sc, r);
+
+	return (pdf.x + pdf.y + pdf.z) / bssrdf->channels;
 }
 
 CCL_NAMESPACE_END
diff --git a/intern/cycles/kernel/osl/osl_bssrdf.cpp b/intern/cycles/kernel/osl/osl_bssrdf.cpp
index b79cef244a0..3e7905f26df 100644
--- a/intern/cycles/kernel/osl/osl_bssrdf.cpp
+++ b/intern/cycles/kernel/osl/osl_bssrdf.cpp
@@ -51,38 +51,27 @@ using namespace OSL;
 class CBSSRDFClosure : public CClosurePrimitive {
 public:
 	Bssrdf params;
-	float3 radius;
-	float3 albedo;
 
 	void alloc(ShaderData *sd, int path_flag, float3 weight, ClosureType type)
 	{
-		float sample_weight = fabsf(average(weight));
-
-		/* disable in case of diffuse ancestor, can't see it well then and
-		 * adds considerably noise due to probabilities of continuing path
-		 * getting lower and lower */
-		if(path_flag & PATH_RAY_DIFFUSE_ANCESTOR) {
-			radius = make_float3(0.0f, 0.0f, 0.0f);
-		}
-
-		if(sample_weight > CLOSURE_WEIGHT_CUTOFF) {
-			/* sharpness */
-			float sharpness = params.sharpness;
-			/* texture color blur */
-			float texture_blur = params.texture_blur;
+		Bssrdf *bssrdf = bssrdf_alloc(sd, weight);
+
+		if(bssrdf) {
+			/* disable in case of diffuse ancestor, can't see it well then and
+			 * adds considerably noise due to probabilities of continuing path
+			 * getting lower and lower */
+			if(path_flag & PATH_RAY_DIFFUSE_ANCESTOR) {
+				params.radius = make_float3(0.0f, 0.0f, 0.0f);
+			}
 
 			/* create one closure per color channel */
-			Bssrdf *bssrdf = bssrdf_alloc(sd, weight);
-			if(bssrdf) {
-				bssrdf->sample_weight = sample_weight * 3.0f;
-				bssrdf->radius = radius;
-				bssrdf->albedo = albedo;
-				bssrdf->texture_blur = texture_blur;
-				bssrdf->sharpness = sharpness;
-				bssrdf->N = params.N;
-				bssrdf->roughness = params.roughness;
-				sd->flag |= bssrdf_setup(bssrdf, (ClosureType)type);
-			}
+			bssrdf->radius = params.radius;
+			bssrdf->albedo = params.albedo;
+			bssrdf->texture_blur = params.texture_blur;
+			bssrdf->sharpness = params.sharpness;
+			bssrdf->N = params.N;
+			bssrdf->roughness = params.roughness;
+			sd->flag |= bssrdf_setup(sd, bssrdf, (ClosureType)type);
 		}
 	}
 };
@@ -101,7 +90,7 @@ ClosureParam *closure_bssrdf_cubic_params()
 {
 	static ClosureParam params[] = {
 		CLOSURE_FLOAT3_PARAM(CubicBSSRDFClosure, params.N),
-		CLOSURE_FLOAT3_PARAM(CubicBSSRDFClosure, radius),
+		CLOSURE_FLOAT3_PARAM(CubicBSSRDFClosure, params.radius),
 		CLOSURE_FLOAT_PARAM(CubicBSSRDFClosure, params.texture_blur),
 		CLOSURE_FLOAT_PARAM(CubicBSSRDFClosure, params.sharpness),
 		CLOSURE_STRING_KEYPARAM(CubicBSSRDFClosure, label, "label"),
@@ -126,7 +115,7 @@ ClosureParam *closure_bssrdf_gaussian_params()
 {
 	static ClosureParam params[] = {
 		CLOSURE_FLOAT3_PARAM(GaussianBSSRDFClosure, params.N),
-		CLOSURE_FLOAT3_PARAM(GaussianBSSRDFClosure, radius),
+		CLOSURE_FLOAT3_PARAM(GaussianBSSRDFClosure, params.radius),
 		CLOSURE_FLOAT_PARAM(GaussianBSSRDFClosure, params.texture_blur),
 		CLOSURE_STRING_KEYPARAM(GaussianBSSRDFClosure, label, "label"),
 		CLOSURE_FINISH_PARAM(GaussianBSSRDFClosure)
@@ -150,9 +139,9 @@ ClosureParam *closure_bssrdf_burley_params()
 {
 	static ClosureParam params[] = {
 		CLOSURE_FLOAT3_PARAM(BurleyBSSRDFClosure, params.N),
-		CLOSURE_FLOAT3_PARAM(BurleyBSSRDFClosure, radius),
+		CLOSURE_FLOAT3_PARAM(BurleyBSSRDFClosure, params.radius),
 		CLOSURE_FLOAT_PARAM(BurleyBSSRDFClosure, params.texture_blur),
-		CLOSURE_FLOAT3_PARAM(BurleyBSSRDFClosure, albedo),
+		CLOSURE_FLOAT3_PARAM(BurleyBSSRDFClosure, params.albedo),
 		CLOSURE_STRING_KEYPARAM(BurleyBSSRDFClosure, label, "label"),
 		CLOSURE_FINISH_PARAM(BurleyBSSRDFClosure)
 	};
@@ -175,9 +164,9 @@ ClosureParam *closure_bssrdf_principled_params()
 {
 	static ClosureParam params[] = {
 		CLOSURE_FLOAT3_PARAM(PrincipledBSSRDFClosure, params.N),
-		CLOSURE_FLOAT3_PARAM(PrincipledBSSRDFClosure, radius),
+		CLOSURE_FLOAT3_PARAM(PrincipledBSSRDFClosure, params.radius),
 		CLOSURE_FLOAT_PARAM(PrincipledBSSRDFClosure, params.texture_blur),
-		CLOSURE_FLOAT3_PARAM(PrincipledBSSRDFClosure, albedo),
+		CLOSURE_FLOAT3_PARAM(PrincipledBSSRDFClosure, params.albedo),
 		CLOSURE_FLOAT_PARAM(PrincipledBSSRDFClosure, params.roughness),
 		CLOSURE_STRING_KEYPARAM(PrincipledBSSRDFClosure, label, "label"),
 		CLOSURE_FINISH_PARAM(PrincipledBSSRDFClosure)
diff --git a/intern/cycles/kernel/svm/svm_closure.h b/intern/cycles/kernel/svm/svm_closure.h
index b2fc01f617e..5a5cf2db401 100644
--- a/intern/cycles/kernel/svm/svm_closure.h
+++ b/intern/cycles/kernel/svm/svm_closure.h
@@ -153,7 +153,6 @@ ccl_device void svm_node_closure_bsdf(KernelGlobals *kg, ShaderData *sd, float *
 #ifdef __SUBSURFACE__
 			float3 mixed_ss_base_color = subsurface_color * subsurface + base_color * (1.0f - subsurface);
 			float3 subsurf_weight = weight * mixed_ss_base_color * diffuse_weight;
-			float subsurf_sample_weight = fabsf(average(subsurf_weight));
 
 			/* disable in case of diffuse ancestor, can't see it well then and
 			 * adds considerably noise due to probabilities of continuing path
@@ -182,27 +181,19 @@ ccl_device void svm_node_closure_bsdf(KernelGlobals *kg, ShaderData *sd, float *
 						sd->flag |= bsdf_principled_diffuse_setup(bsdf);
 					}
 				}
-				else if(subsurface > CLOSURE_WEIGHT_CUTOFF && subsurf_sample_weight > CLOSURE_WEIGHT_CUTOFF) {
-					/* radius * scale */
-					float3 radius = subsurface_radius * subsurface;
-					/* sharpness */
-					float sharpness = 0.0f;
-					/* texture color blur */
-					float texture_blur = 0.0f;
-
-					/* create one closure per color channel */
+				else if(subsurface > CLOSURE_WEIGHT_CUTOFF) {
 					Bssrdf *bssrdf = bssrdf_alloc(sd, subsurf_weight);
+
 					if(bssrdf) {
-						bssrdf->sample_weight = subsurf_sample_weight * 3.0f;
-						bssrdf->radius = radius;
+						bssrdf->radius = subsurface_radius * subsurface;
 						bssrdf->albedo = subsurface_color;
-						bssrdf->texture_blur = texture_blur;
-						bssrdf->sharpness = sharpness;
+						bssrdf->texture_blur = 0.0f;
+						bssrdf->sharpness = 0.0f;
 						bssrdf->N = N;
 						bssrdf->roughness = roughness;
 
 						/* setup bsdf */
-						sd->flag |= bssrdf_setup(bssrdf, (ClosureType)CLOSURE_BSSRDF_PRINCIPLED_ID);
+						sd->flag |= bssrdf_setup(sd, bssrdf, (ClosureType)CLOSURE_BSSRDF_PRINCIPLED_ID);
 					}
 				}
 			}
@@ -756,35 +747,22 @@ ccl_device void svm_node_closure_bsdf(KernelGlobals *kg, ShaderData *sd, float *
 		case CLOSURE_BSSRDF_CUBIC_ID:
 		case CLOSURE_BSSRDF_GAUSSIAN_ID:
 		case CLOSURE_BSSRDF_BURLEY_ID: {
-			float3 albedo = sd->svm_closure_weight;
 			float3 weight = sd->svm_closure_weight * mix_weight;
-			float sample_weight = fabsf(average(weight));
-			
-			/* disable in case of diffuse ancestor, can't see it well then and
-			 * adds considerably noise due to probabilities of continuing path
-			 * getting lower and lower */
-			if(path_flag & PATH_RAY_DIFFUSE_ANCESTOR)
-				param1 = 0.0f;
-
-			if(sample_weight > CLOSURE_WEIGHT_CUTOFF) {
-				/* radius * scale */
-				float3 radius = stack_load_float3(stack, data_node.z)*param1;
-				/* sharpness */
-				float sharpness = stack_load_float(stack, data_node.w);
-				/* texture color blur */
-				float texture_blur = param2;
-
-				/* create one closure per color channel */
-				Bssrdf *bssrdf = bssrdf_alloc(sd, weight);
-				if(bssrdf) {
-					bssrdf->sample_weight = sample_weight * 3.0f;
-					bssrdf->radius = radius;
-					bssrdf->albedo = albedo;
-					bssrdf->texture_blur = texture_blur;
-					bssrdf->sharpness = sharpness;
-					bssrdf->N = N;
-					sd->flag |= bssrdf_setup(bssrdf, (ClosureType)type);
-				}
+			Bssrdf *bssrdf = bssrdf_alloc(sd, weight);
+
+			if(bssrdf) {
+				/* disable in case of diffuse ancestor, can't see it well then and
+				 * adds considerably noise due to probabilities of continuing path
+				 * getting lower and lower */
+				if(path_flag & PATH_RAY_DIFFUSE_ANCESTOR)
+					param1 = 0.0f;
+
+				bssrdf->radius = stack_load_float3(stack, data_node.z)*param1;
+				bssrdf->albedo = sd->svm_closure_weight;
+				bssrdf->texture_blur = param2;
+				bssrdf->sharpness = stack_load_float(stack, data_node.w);
+				bssrdf->N = N;
+				sd->flag |= bssrdf_setup(sd, bssrdf, (ClosureType)type);
 			}
 
 			break;