unconed / mathbox

Presentation-quality WebGL math graphing
MIT License
1.33k stars 109 forks source link

Surfaces are too dark #25

Closed ChristopherChudzicki closed 2 years ago

ChristopherChudzicki commented 2 years ago

Left: v0.0.5 / Right: v2+

Screen Shot 2022-04-07 at 8 30 26 PM

@sritchie I dunno if this is a consequence of changes we've made in Mathbox, or a consequence of the ThreeJS upgrades. Do you have any idea? I think you've looked at more versions of ThreeJS than I have. Otherwise... I'll try to learn something about ThreeJS :)

sritchie commented 2 years ago

Interesting, I only remember the second one through all my testing. I will look!

sritchie commented 2 years ago

It looks like it was dark at 98c37401f337cc61460e9b9d9e6f32bf0fdf78b7, my first big merge (check out and run npm install && gulp build). So I must have introduced some problem during the migration from Coffeescript...

sritchie commented 2 years ago

If it helps to debug, it also happens in:

file:///Users/sritchie/code/js/mathbox/examples/test/vertexcolor.html file:///Users/sritchie/code/js/mathbox/examples/test/transition.html file:///Users/sritchie/code/js/mathbox/examples/test/surface.html file:///Users/sritchie/code/js/mathbox/examples/test/subdivide.html file:///Users/sritchie/code/js/mathbox/examples/test/repeat.html file:///Users/sritchie/code/js/mathbox/examples/math/procedural.html file:///Users/sritchie/code/js/mathbox/examples/math/hopf.html file:///Users/sritchie/code/js/mathbox/examples/demo/shapes.html file:///Users/sritchie/code/js/mathbox/examples/demo/cylindrical-stream.html

It does NOT seem to happen in file:///Users/sritchie/code/js/mathbox/examples/test/fragmentcolor.html file:///Users/sritchie/code/js/mathbox/examples/demo/cylindrical.html

which seemed surprising.

ChristopherChudzicki commented 2 years ago

Whatever happened, I'm fairly certain it is not related to Shadergraph. I dropped an old version of Shadergraph into the current repo and stuff was still dark. I also plugged in an old v0.05 version of shaders.js (recall it used to get built to build/shaders.js) and things were still dark.

Znah's is not as dark as modern, but it does have dark lines. Steven's original looks best :/ Hope we can get back to that.

Screen Shot 2022-04-08 at 11 18 32 PM

EDIT: I think there are two separate issues: Shading and line color. If you turn off the lines, znah and 0.0.5 look the same.

sritchie commented 2 years ago

@ChristopherChudzicki I agree. here are some clues:

image

So that makes me suspect something in https://github.com/unconed/mathbox/blob/0.0.5/src/render/meshes/base.coffee#L120-L186... but they really read the same.

ChristopherChudzicki commented 2 years ago

On 0.0.5, setting lineX or lineY to true vs false has no effect. the lines are missing in both cases. THAT feels like a bug to me in 0.0.5

I'm pretty sure this does have an effect in 0.0.5, although the effect is not visible unless you're using a shaded surface. If not using shaded surface, the gridlines and surface are exact same color and you can't differentiate between the two. (I think the original intent was that the grid lines would be. adarker shade of the main surface color. I don't understand why the gridlines turned black, but I think I do understand why they were not behaving as intended in 0.0.5. Changing the x/y/z in this code to r/g/b fixes the lineX lineY issue in current master, and changing the corresponding lines in the old coffeescript code fix it in 0.0.5. https://github.com/unconed/mathbox/blob/4fd08d6cfdd41446685b53190be833055c10086f/src/primitives/types/draw/surface.js#L278-L288. It would be nice to understand why the turned balack, but I'll settle for understanding why they weren't working as intended originally...).

As for the why the shaded surface itself is darker... I spent a while investiating that this weekend. I still do not fully understand. The shading is done by the two GLSL shader snippts surface.position.normal and mesh.fragment.shaded. . I'm a GLSL noob, but my understanding is that the "inputs" to these shader snippets are the "uniforms", and the current master shaders vs 0.0.5 appear to be getting the same inputs.

As best I can tell, there's an issue with gl_FrontFacing, though I really don't understand why it was working in ThreeJS r71 but not ThreeJS r137. Sigh. See #26 for more.

I'll open a PR for the lineX, lineY thing soon, and hopefully get back to the mathbox-react bug you opened. (Thanks for opening that!)

sritchie commented 2 years ago

Fixed by #27.