-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
mesh3d: Different lighting results with different hardware #1368
Comments
Hmm… looks like possible flipped normals to me. I'd suggest maybe See: https://github.com/gl-vis/gl-surface3d/blob/master/shaders/fragment.glsl#L28-L30 |
cc @dfcreative which have insight about this. |
I don't have access to this machine but guessing is always fun:
One thing to try: visiting a bunch of WebGL content on the web to see if those work properly. |
Quick update: It doesn’t seem to matter what value I set for |
Do these work fine? http://webglsamples.org/ |
Gotchas like |
Yeah, nothing looks "off" with those. I can also change the |
Not that it should inherently make it fail, but |
Oops, sorry. I had the wrong module. Maybe………(though this is necessarily positive, isn't it?)? https://github.com/stackgl/glsl-specular-cook-torrance/blob/master/index.glsl#L26 |
One more thing - possibly relating to the above possibility (lower precision floats): I needed to tweak the EPSILON value in the normals calculation. The brain mesh is made up of very small triangles. With the wrong EPSILON value, whole swaths of lighting just don't happen. This is my current best guess. Way to test it: load some coarser Also, my old commits made EPSILON configurable from within Plotly: mikolalysenko/normals@a519d8a#diff-f02129f250a223ce2d22f0b6e4fa7297 |
Hm, yeah, larger triangles seem OK: https://plot.ly/~empet/13481/delaunay-triangulation-and-alpha-shapes-for-a-set-of-3d-points-1-delaunay-triang/ https://plot.ly/~RPlotBot/3008/moebius-band-triangulation/ (I found these |
I added the epsilon attribs to the codepen, with their respective default values: http://codepen.io/monfera/pen/XjxZjZ Could you test if lower values make it work? E.g. 1e-9 instead of 1e-6, or 1e-15 instead of 1e-12. Or some much more ambitious change. |
Another example on a trisurf plot with less vertices: Robert sees this shiny helicopter: I see this non-shiny midnight rider: |
Here are some more brain images that might be useful in debugging. One shows specular reflection (is "shiny"). Another is an odd mix of shiny and unshiny. Shiny: Odd mix of shiny + unshiny: Dark: |
@jackparmer what does this look like for you? https://rawgit.com/rreusser/057e5c31845192528cf62ba075540853/raw/index.html It's just a simple sphere that uses some of the same basic lighting functions (cook-torrance and diffuse-lambert). It looks like this for me: Source here |
Ah, sorry. I rotated it in the code at the last minute. That's a 👍 from me on being the same. Not that I thought it would be different, but just a quick sanity check. |
@jackparmer Ricky is right, We can use that workaround, considering the 99% support of Also we may want to consider replacing |
From #1423 (comment) : After digging into the behavior quite a bit with @alexcjohnson, here's what we conclude: The current behavior mostly make sense. The really weird stuff is all in the light positioning.
Apart from how it does work, here's how we think it should work: The light should be positioned effectively in clip coordinates with the addition of an aspect ratio. Conceptual test cases:
Note: scene aspect ratio and axis ranges seem to scale things entirely independently of the view, so I think they can be ignored. Open to feedback, but I think these constraints are enough to minimally fix the parts of the current behavior that are surprising without changing anything substantial. In talking with @alexcjohnson one conclusion is that existing behavior like the weird 1.00002 -> 1.00003 z threshold is best just fixed so that it's probably best not to preserve strict backward compatibility. |
Hey @dfcreative I just ran into this issue again. Did gl-vis/gl-mesh3d#8 fix the dark |
One things that is weird, I don't see any shading issues in this gl-mesh3d example: |
Just a minor note, these last two images aren't too different; both seem to be primarily or exclusively lit by ambient light; it appears that the darker one simply has a lower light level. A high ambient level is usually not good because it "washes away" detail and reduces plasticity, as can be seen on the brainbrowser example. Neither seem to have eg. the specular component, compare to this: In short, I think the dark brain above wants to be like what's seen in the codepen example, but misses components other than the (justly) subdued ambient component. I go by the look so I may well be wrong. |
@monfera The issue is that the same mesh looks completely different depending on the viewer's computer. Here's another example: https://plot.ly/~empet/14759/. This mesh look completely different on my laptop versus my phone. These are the exact same plot (no lighting as been adjusted). This is a huge bug IMO. Plotly meshes shouldn't look completely different depending on what computer you're using. |
We have that fixed in gl-mesh3d gl-vis/gl-mesh3d#8, it takes a separate effort meeting the requirements here though |
AFAICT it's a GPU issue. The MBP where Plotly.js meshes appear dark has an Intel 515 GPU, but the exact same MBP (same model, OS, and Chrome) with an Intel 5300 GPU displays meshes with "normal" lighting. The Plotly Cloud image server also renders Plotly.js meshes overly dark, wonder what GPU it's using ATM: |
Note: @nicolaskruchten also sees this with an Intel Iris GPU on his MBP |
FWIW the first few links now look great on my machine :) |
(ah, but they look fine with 1.35.0 also... What's an updated test case I can try?) |
Strange. I haven't updated the CDN links yet. |
Sorry, too quick on the draw! I had initially seen these issues in a Dash app that showed a brain. Not sure what an easy-to-access test case is. Happy to check on anything pre/post if you like on my machine. |
https://cdn.plot.ly/plotly-1.36.0.min.js is now up! |
Consider this codepen:
http://codepen.io/monfera/full/ggByRW
On my 12 in Macbook, I see this:
@etpinard 's Thinkpad and @monfera 's Macbook show this:
Another example - consider this codepen:
http://codepen.io/monfera/pen/XjxZjZ
Result on my 12 in Macbook:
Result on larger Macbooks and Thinkpad:
I tried this in Chrome, Safari, and Firefox with the same result.
Computer specs:
Chrome specs:
The text was updated successfully, but these errors were encountered: