Page MenuHome
Differential D1000 Diff 3174 source/blender/nodes/shader/nodes/node_shader_vectMath.c

Changeset View

Standalone View

View Options

source/blender/nodes/shader/nodes/node_shader_vectMath.c

Show First 20 Lines • Show All 70 Lines • ▼ Show 20 Lines else if (node->custom1 == 2) { /* Average */
out[0]->vec[2] = vec1[2] + vec2[2]; out[0]->vec[2] = vec1[2] + vec2[2];
out[1]->vec[0] = normalize_v3(out[0]->vec); out[1]->vec[0] = normalize_v3(out[0]->vec);
} }
else if (node->custom1 == 3) { /* Dot product */ else if (node->custom1 == 3) { /* Dot product */
out[1]->vec[0] = (vec1[0] * vec2[0]) + (vec1[1] * vec2[1]) + (vec1[2] * vec2[2]); out[1]->vec[0] = (vec1[0] * vec2[0]) + (vec1[1] * vec2[1]) + (vec1[2] * vec2[2]);
} }
else if (node->custom1 == 4) { /* Cross product */ else if (node->custom1 == 4) { /* Cross product */
out[0]->vec[0] = (vec1[1] * vec2[2]) - (vec1[2] * vec2[1]); float vec[3];
out[0]->vec[1] = (vec1[2] * vec2[0]) - (vec1[0] * vec2[2]);
out[0]->vec[2] = (vec1[0] * vec2[1]) - (vec1[1] * vec2[0]); vec[0] = (vec1[1] * vec2[2]) - (vec1[2] * vec2[1]);
vec[1] = (vec1[2] * vec2[0]) - (vec1[0] * vec2[2]);
vec[2] = (vec1[0] * vec2[1]) - (vec1[1] * vec2[0]);
out[1]->vec[0] = len_v3(vec);
out[0]->vec[0] = vec[0] / len_v3(vec);
out[0]->vec[1] = vec[1] / len_v3(vec);
out[0]->vec[2] = vec[2] / len_v3(vec);
mont29Unsubmitted
Not Done
Inline Actions

Why is this change needed? To me (noob) eyes it only adds verbose code with no benefits at all?

mont29: Why is this change needed? To me (noob) eyes it only adds verbose code with no benefits at all?
kevindietrichAuthorUnsubmitted
Not Done
Inline Actions

Well, the node has two outputs : a vector and a value. Previously the code would do the following:

vector = cross(input1, input2);
value = normalize(vector);

and now (fixed):

vector = cross(input1, input2);
value = length(vector);
vector = normalize(vector);

That's how Cycles goes, and also GLSL except the normalization of the vector was missing at the end.

Also I admit it's a tad verbose but apparently there are no cross and normalize function to return a vector in BLI. As far as I could see, they only return a single float.

kevindietrich: Well, the node has two outputs : a vector and a value. Previously the code would do the…
mont29Unsubmitted
Not Done
Inline Actions

I do not see the issue with value = normalize_v3(vector);, normalize_v3() does return the length of the vector (before normalization), and normalizes the vector in-place…

mont29: I do not see the issue with `value = normalize_v3(vector);`, `normalize_v3()` does return the…
kevindietrichAuthorUnsubmitted
Not Done
Inline Actions

hmm, I did not see that... guess I just got stuck on the name... also, quite a funny one: while debugging, I kept changing the code but couldn't see any difference, until I realized I was compiling a release build while launching a debug one to test! :/ Anyhows, will update.

kevindietrich: hmm, I did not see that... guess I just got stuck on the name... also, quite a funny one: while…
mont29Unsubmitted
Not Done
Inline Actions

Tsst, empty line ;)

mont29: Tsst, empty line ;)
kevindietrichAuthorUnsubmitted
Not Done
Inline Actions

Okay so who am I now to act as the code style police from time to time in this code review system... :P

kevindietrich: Okay so who am I now to act as the code style police from time to time in this code review…
out[1]->vec[0] = normalize_v3(out[0]->vec);
} }
else if (node->custom1 == 5) { /* Normalize */ else if (node->custom1 == 5) { /* Normalize */
if (in[0]->hasinput || !in[1]->hasinput) { /* This one only takes one input, so we've got to choose. */ if (in[0]->hasinput || !in[1]->hasinput) { /* This one only takes one input, so we've got to choose. */
out[0]->vec[0] = vec1[0]; out[0]->vec[0] = vec1[0];
out[0]->vec[1] = vec1[1]; out[0]->vec[1] = vec1[1];
out[0]->vec[2] = vec1[2]; out[0]->vec[2] = vec1[2];
} }
else { else {
▲ Show 20 Lines • Show All 61 Lines • Show Last 20 Lines