Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cc/fix dark surfaces #26

Merged
merged 1 commit into from
Apr 14, 2022
+9 −2
Merged

Cc/fix dark surfaces #26

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/shaders/glsl/mesh.fragment.shaded.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
export default /* glsl */ `varying vec3 vNormal;
export default /* glsl */ `
#extension GL_OES_standard_derivatives : enable
varying vec3 vNormal;
varying vec3 vLight;
varying vec3 vPosition;

Expand All @@ -16,7 +18,12 @@ vec4 getShadedColor(vec4 rgba) {
vec3 light = normalize(vLight);
vec3 position = normalize(vPosition);

float side = gl_FrontFacing ? -1.0 : 1.0;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sritchie If you feel like futzing with this, try using gl_FrontFacing ? +1.0 : - 1.0...you'll see both sides of the surface turn light.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems to bite many folks. I've been reading up on it, and would NOT have found this on my own. Great find on this fix.

// Workaround to avoid gl_FrontFacing. See https://github.com/unconed/mathbox/pull/26
vec3 pdx = dFdx(vPosition);
vec3 pdy = dFdy(vPosition);
bool frontFacting = dot(vNormal, cross(pdx, pdy)) > 0.0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, slight typo with frontFacting vs frontFacing.

Wow, there is some magic going on in glsl. SO, indulge me in trying to understand what this does:

  • dfDx and dfDy must return vectors; are they giving the derivative of the x and y components of vPosition?
  • If that's right, the cross product will always be orthogonal... so we care if the magnitude is positive or negative.
  • and then dot(...) > 0 is telling us, "is vNormal pointing in the same direction"

But I am missing my concepts here, since I would have expected vNormal to be in reference to some plane... ah!!

I would love a one sentence description of the algorithm here for my own edification, and probably for future folks trying to debug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I am not 100% clear on the coordinate systems involved here. I think the following sentence would be accurate:

"vNormal (calculated in the vertex shader) and cross(pdx, pdy) (calculated in this fragment shader) are both surface normals, but might be in opposite directions depending on which side of the surface the camera is, hence dot product".

My impression is that vNormal is calculated differently for the different objects, that's why there's surface.position.normal, face.position.normal, and strip.position.normal. [(but represents a surface normal in all cases), but represents a surface normal in all three cases.]
If you look at how vNormal in surface.position.normal is calculated, you'll see its also basically a cross product of derivatives. But I suspect the coordinates are slightly different (potentially reversed) hence the two cross products may differ in direction. (Experimentally, they definitely do differ in direction, since both conditions in the frontFacing ? 1.0 : -1.0 are getting triggered.)

Understanding the shader stuff better is definitely a goal :/ I'm hoping not to tinker with it too much.

In terms of posterity... I decided just leave a link to the PR as the code comment. (Though git history does that, too).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In particular, one thing I am confused about is getPosition in the shaders. A lot of the vertex shaders include

// External
vec3 getPosition(vec4 xyzw, float canonical);

and then getPosition is used. But I can't find where the heck this "External" function is coming from...


float side = frontFacting ? 1.0 : -1.0;
float cosine = side * dot(normal, light);
float diffuse = mix(max(0.0, cosine), .5 + .5 * cosine, .1);

Expand Down