meshcat-dev / meshcat

Remotely-controllable 3D viewer, built on top of three.js
MIT License
256 stars 48 forks source link

Add more shadow controls, and change DirectionalLight to PointLight #61

Closed gizatt closed 4 years ago

gizatt commented 5 years ago

The main fix here is fixing some shadow banding issues I was seeing by increasing default shadow map size a lot (hopefully not too much? it's only 4mb, but I have a beefy computer), and most importantly, adding a little shadow bias.

I'm not super attached to the DirectionalLight->PointLight change -- it works well for my scene scales (handfuls of meters), but I dunno what other folks use. The distance control can be used to get light up to pretty large distances (with 0 distance supposed to be infinite lighting, so functionally the same as DirectionalLight? But I'm not sure I believe it after testing it).

Before, with and without shadows: image image

After, with and without shadows: image image

PointLights are particularly nice because they support falloff that adds some depth cues to otherwise flat scenes. The "good" settings are very scene-dependent, but as an example for this scene, these are the best parameters I could find tweaking only the parameters made adjustable in this PR: image


This change is Reviewable

rdeits commented 5 years ago

This is really cool! I think it's definitely an improvement. Let me just run a few tests locally to make sure the things I'm doing work with the parameters you've picked.

ferrolho commented 4 years ago

Bump! This looks great! 🧐

ferrolho commented 4 years ago

Is there something holding this back, @rdeits ?

rdeits commented 4 years ago

@gizatt mentioned in person that he had some more changes he wanted to make--is that still the case?

gizatt commented 4 years ago

Uhh... whatever they were, I've forgotten about them, so I'm happy with this going in as-is! (It's what I'm using on my own machine right now and I'm satisfied with it.)

ferrolho commented 4 years ago

:bump: @rdeits

rdeits commented 4 years ago

This is awesome. I've merge master into it over in #66 so I'll merge that PR. Thanks for making this happen!

ferrolho commented 4 years ago

Thank you both! This is really good.