stackgl / shader-school

:mortar_board: A workshopper for GLSL shaders and graphics programming
Other
4.27k stars 251 forks source link

Bug in 22-npr-1 (cel shading)? #130

Open dhcoder opened 9 years ago

dhcoder commented 9 years ago

22-npr-1 asks the user to go back and get their lambert diffuse solution, which, in mine, has the line:

vec3 viewNormal = normalize((vec4(normal, 0.0) * inverseModel * inverseView).xyz);

However, in the cel shading sample, my solution only works if I remove the inverseView matrix adjustment:

vec3 viewNormal = normalize((vec4(normal, 0.0) * inverseModel).xyz);

I'm not sure if that's intentional. If so, it might be worth hinting at why there's a difference in the problem description. I'm still having trouble reasoning why I'm only moving my normal into world space now and not view space. Isn't the light direction specified in view space (to be in the same coordinate space as the eye)?

As it is now, I only found this out by trial and error rather than understanding why this works.

unclejimbo commented 8 years ago

I think this is a bug too.

DCtheTall commented 8 years ago

I think it was a bug, but you can also tell visually that this is the correct answer.

The transformations you see on the dragon are due to the shader being buffered a different view matrix uniform every frame, the model matrix is not really changing too much (I learned this through experimentation in other exercises).

As the dragon moves through the scene in the depiction of the solution, the lighting doesn't move with the dragon. That means that the light direction does not transform with the changing view matrix, but it does seem change with the model matrix.

It's still obnoxious that the programs are arbitrarily changing which vectors transform with matrices, ugh, so it goes.

JetStarBlues commented 7 years ago

Bug or otherwise, something in the instructions should indicate the change... Considering that the entire previous chapter is spent drilling that vec4(normal, 0.0) * inverseModel * inverseView is the way to calculate worldNormal