Vector Math's "Refract" node gives wrong results. #102068

New Issue

G. Solis · 2022-10-26T00:55:26+02:00

G. Solis commented

2022-10-26 00:55:26 +02:00

Blender Version
Broken: version: 3.3.0, branch: master, commit date: 2022-09-06 15:39, hash: 0759f671ce
Worked: Never! Not since the node was introduced.

Short description of error
The "Refract" node, contained in "Vector Math", gives wrong results. An easy one to spot happens when the incident vector is not normalized, but it's not the only one.
I've checked the source code in intern/cycles/util/math_float3.h and confirmed the formula is slightly wrong.

Spotted by Ian Ambrose on devtalk

Exact steps for others to reproduce the error
Use the Refract node and input a non-normalized incident vector.

**Blender Version** Broken: version: 3.3.0, branch: master, commit date: 2022-09-06 15:39, hash: `0759f671ce` Worked: Never! Not since the node was introduced. **Short description of error** The "Refract" node, contained in "Vector Math", gives wrong results. An easy one to spot happens when the incident vector is not normalized, but it's not the only one. I've checked the source code in `intern/cycles/util/math_float3.h` and confirmed the formula is slightly wrong. Spotted by [Ian Ambrose on devtalk ](https://devtalk.blender.org/t/mathematical-formula-of-vector-refraction/26207) ![Screenshot from 2022-10-25 19-30-11.png](https://archive.blender.org/developer/F13769322/Screenshot_from_2022-10-25_19-30-11.png) ![Screenshot from 2022-10-25 19-33-42.png](https://archive.blender.org/developer/F13769324/Screenshot_from_2022-10-25_19-33-42.png) **Exact steps for others to reproduce the error** Use the Refract node and input a non-normalized incident vector.

G. Solis commented

2022-10-26 00:55:26 +02:00

Added subscriber: @Gabriel-Solis

Edward commented

2022-10-26 02:07:59 +02:00

Added subscriber: @edward88

Edward commented

2022-10-26 02:07:59 +02:00

Hm, i can not spot the difference. Everything inside the root function is identical with k.
So according to the formula, the calculation in the return statement should be
sqrt(k) * normal + eta * (incident - dot(normal, incident) * normal)
expand with eta
eta * incident - eta * dot(normal, incident) * normal + sqrt(k) * normal
collapse normal
eta * incident - (eta * dot(normal, incident) + sqrt(k)) * normal

Am i missing something?

Hm, i can not spot the difference. Everything inside the root function is identical with `k`. So according to the formula, the calculation in the return statement should be `sqrt(k) * normal + eta * (incident - dot(normal, incident) * normal)` expand with `eta` `eta * incident - eta * dot(normal, incident) * normal + sqrt(k) * normal` collapse `normal` `eta * incident - (eta * dot(normal, incident) + sqrt(k)) * normal` Am i missing something?

G. Solis commented

2022-10-26 02:25:50 +02:00

Are you referring to the changes in D16341? If so, the fix is that the incident vector is now normalized before the calculation. If not, reading the patch message may clarify things a bit.

Are you referring to the changes in [D16341](https://archive.blender.org/developer/D16341)? If so, the fix is that the incident vector is now normalized before the calculation. If not, reading the patch message may clarify things a bit.

Edward commented

2022-10-26 02:28:00 +02:00

Yes i did not see the patch when i wrote that, ty

Charlie Jolly commented

2022-10-26 04:18:43 +02:00

Added subscriber: @CharlieJolly

Charlie Jolly commented

2022-10-26 04:18:43 +02:00

Hi, part of the issue is that it is based on the glsl and osl version of the function.

https://registry.khronos.org/OpenGL-Refpages/gl4/html/refract.xhtml

This is one of those bug/not bug things.

To quote the glsl docs…
The input parameters I and N should be normalized in order to achieve the desired result.

In either case the patch would need to touch the Eevee and Cycles code paths too. The other solution is to make sure the documentation and tooltips advise the inputs should be normalised.

Hi, part of the issue is that it is based on the glsl and osl version of the function. https://registry.khronos.org/OpenGL-Refpages/gl4/html/refract.xhtml This is one of those bug/not bug things. To quote the glsl docs… *The input parameters I and N should be normalized in order to achieve the desired result.* In either case the patch would need to touch the Eevee and Cycles code paths too. The other solution is to make sure the documentation and tooltips advise the inputs should be normalised.

Brecht Van Lommel commented

2022-10-26 10:29:07 +02:00

Added subscriber: @brecht

Brecht Van Lommel commented

2022-10-26 10:29:07 +02:00

For the vector math node it would be reasonable to make it work with unnormalized vectors, since it's more high level functionality. I would not consider that a bugfix but a useful improvement to the node.

The general math library implementation that can be used by other parts of the code should not be changed, to avoid a performance impact. A second function can be added that works with unnormalized vectors, to be used by the vector math node.

For the vector math node it would be reasonable to make it work with unnormalized vectors, since it's more high level functionality. I would not consider that a bugfix but a useful improvement to the node. The general math library implementation that can be used by other parts of the code should not be changed, to avoid a performance impact. A second function can be added that works with unnormalized vectors, to be used by the vector math node.

Omar Emara commented

2022-10-26 11:42:26 +02:00

Added subscriber: @OmarEmaraDev

Omar Emara commented

2022-10-26 11:42:26 +02:00

Changed status from 'Needs Triage' to: 'Archived'

Omar Emara closed this issue

2022-10-26 11:42:26 +02:00

Omar Emara commented

2022-10-26 11:42:26 +02:00

Since it was established that this is not exactly a bug and a patch was already submitted, I will archive this for now.

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Vector Math's "Refract" node gives wrong results. #102068