Cc/fix dark surfaces #26
Conversation
| @@ -16,7 +18,12 @@ vec4 getShadedColor(vec4 rgba) { | |||
| vec3 light = normalize(vLight); | |||
| vec3 position = normalize(vPosition); | |||
| float side = gl_FrontFacing ? -1.0 : 1.0; | |||
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
gl_FrontFacing seems to be causing an issue with shaded surfaces. Some data: - using `side = gl_FrontFacing ? -1.0 : 1.0` worked in mathbox-0.0.5 (Also on znah's fork using r84) - using `side = glFrontFacting ? -1.0 : 1.0` appears to be rendering both sides of a DoubleSided surface as "dark" (the back side) on current mathbox. - switching to `side = 1.0` renders both sides as light; `side = -1.0` renders both sides as dark. The workaround in this commit requires a WebGL standard_derivatives extension. This workaround was suggested in https://www.reddit.com/r/glsl/comments/3q8kn2/gl_frontfacing_on_a_mac/ I do not understand why gl_FrontFacing worked on mathbox 0.0.5 but not hte current version. Something to do with threejs upgrades? Unsure.
ChristopherChudzicki
force-pushed
the
cc/fix-dark-surfaces
branch
from
514d61f to
0598b58
Compare
| // 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; |
There was a problem hiding this comment.
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:
dfDxanddfDymust return vectors; are they giving the derivative of the x and y components ofvPosition?- If that's right, the cross product will always be orthogonal... so we care if the magnitude is positive or negative.
- and then
dot(...) > 0is 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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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...
|
Just a couple of comments. Excellent work! |
What are the relevant tickets? #25
Note that #25 mentions two issues: dark surfaces and black gridlines. This addresses the dark surfaces, NOT the gridlines. I believe I understand how to fix that, too, but want to write some js tests for it.
What does this PR do?
This PR replaces existing usage of
gl_FrontFacingwith a cross-product workaround;gl_FrontFacingseems not to be behaving how mathbox expected.How should this be manually tested
npm run buildexamples/test/surface.html. Change the mathbox line to<script type="text/javascript" src="../../build/bundle/mathbox.js"></script>to use local buildlineXandlineYbits at end ofsurface.html. The affect is more proncounced without the gridlinesMaster / This branch
/ 