Page MenuHome

Lack of total internal reflection in refract
Closed, ResolvedPublic

Description

System Information

Blender Version
Broken: 2.78 4bb1e22

Short description of error
Currently, for k values less than 0 in the refract function, a vector of 0,0,0. This should return a reflection. In combination with fixing the Fresnel node with values under 1, this would allow for accurate total internal reflection.

Exact steps for others to reproduce the error

Create icosphere, set shading to smooth, assign refraction BSDF, view from inside sphere.

found at line 193 in stdosl.h

vector refract (vector I, vector N, float eta) {
    float IdotN = dot (I, N);
    float k = 1 - eta*eta * (1 - IdotN*IdotN);
    return (k < 0) ? vector(0,0,0) : (eta*I - N * (eta*IdotN + sqrt(k)));
}

correction should be something like

return (k < 0) ? reflect(I,N) : (eta*I - N * (eta*IdotN + sqrt(k)));

Event Timeline

I don't really agree that it should return a reflection - after all, it's explicitly called refract, not bounce_off_glass. So, when refracting isn't possible, the function shouldn't attempt to guess what you might want instead.
Another consideration is that you'll probably want to use fresnel weighting between reflection and refraction anyways, and in the case of TIR the refraction weight will be zero.

And even if it was wrong behavior, we can't just change functions that have been in releases in the past since it would mess up shaders that have been written with the old behavior in mind.

If you really want the behavior you're describing, you could just wrap refract() with a helper function that checks the result and returns reflect() instead.

I understand your point of view and agree that this function shouldn't be changed after reading your comment.

Should, then, the Fresnel node be corrected to produce the right value for rays coming out of a material? I feel like the physically correct way of rendering refractive materials (especially when it is computationally not much more expensive) shouldn't require coding for the user.

Currently the Fresnel node doesn't give correct values, and therefore can't be used by its-self to create Total Internal Reflection. All other aspects of this node are physically accurate, and I don't see this change being detrimental to existing materials.

The Fresnel node was primarily intended for mixing diffuse and glossy, since the glass BSDF already includes fresnel. For correct handling of multiple scattering and roughness you also need the Fresnel to be baked into the BSDF anyway, doing it using custom nodes is not physically correct.

The problem is that those two cases require different handling of normals, for glass you want to invert the IOR when exiting the object, for mixing diffuse and glossy you don't want that.

So you'd need to do something like adding an option to the Fresnel node to choose the behavior. But really I would prefer to eventually remove the Refractive BSDF node, since it's not possible to use it in a physically correct way.

Chris Cook (CCook) changed the task status from Unknown Status to Resolved.Feb 5 2017, 3:47 PM
Chris Cook (CCook) claimed this task.

Thanks for the insight, @Brecht Van Lommel (brecht), it hadn't occurred to me that it would be used for that, but it makes sense, as its current values mean a diffuse/glossy mix looks the same on both sides.

I would prefer to eventually remove the Refractive BSDF node, since it's not possible to use it in a physically correct way.

Okay. I have been playing with adding dispersion to refractive materials, both with node groups and with OSL, and both methods rely on the Refraction BSDF to work. It has been a goal of mine to get glass looking better in cycles (at the expense of samples)

As for Total Internal Reflection, adding a 'refractive' boolean option to the Fresnel node would be sufficient for purely smooth glass, but as you said, won't fix the issue for rough surfaces.

I will close this case as I've realised the issue is not in the refraction, but would like to continue discussing a backwards-compatible solution to this, as I believe it would be a great addition to a reasonably visible problem. I'm happy to help implement the changes.