Three.js: Default lighting

[paulmasson]

CC: @novoselt @egourgoulhon @kcrisman

Component: graphics

Author: Paul Masson

Branch/Commit: b0ea28a

Reviewer: Andrey Novoseltsev

Issue created by migration from https://trac.sagemath.org/ticket/22261

[paulmasson]

comment:1

As part of a comment, Andrey said:

Now I noticed that jmol's version also has visible rotation only due to defects of the surface, but it is the most pleasing one, IMHO. And a big difference seems to be that jmol rotates everything keeping (a single?) light fixed, while threejs rotates objects together with lights. Is it possible to have fixed lights in threejs? That's is ideal for our applications, I think - you kind of have an object under a desk lamp, and rotate it to see different sides.

Lights can be put in a fixed position relative to the view of the user by adding them to the camera and not the scene. I personally like having some shadows on an object to make contours more interesting, and that won't happen if the light is coming from a spot near the view of the user. That will also make it more difficult for users to customize lighting if that feature will be desired in the future.

Should the lights be fixed relative to the objects in the scene or relative to the view of the user?

[novoselt]

comment:3

I also like shadows, but with my desk lamp analogy your eyes and lamp are not at all in the same place, they are just fixed while you rotate your apple in the light, with one side brighter than another. When you rotate the apple, then bright/dark spots and shadows change. When lights are tied to the scene, it is like having your apple nailed under the desk lamp and moving your head around to get a good look. Apart from unnatural physical positions this will sometimes mean that you are looking at the dark side of the apple against the light...

[paulmasson]

Branch: u/paulmasson/22261

[paulmasson]

Author: Paul Masson

[paulmasson]

comment:5

Here's a first shot at changing the default lighting. Having only one directional light off to the viewer's left resulted in too much dark space on the right side of objects, so I put in two directional lights. The code is writen to allow a future option for user customization, but see if you like this better than the current lighting.

While this looks similar to Jmol for spherical surfaces, it's a bit different for flat surfaces like cubes: these end up with more shadow than in Jmol. Also, the lighting in Jmol appears to be rendered once and stays invariant during zooming, but in Three.js the live rendering means that the lighting will change during zooming in or out.

---

New commits:

0361f41

Modify default lighting

[paulmasson]

Commit: 0361f41

[novoselt]

comment:6

I definitely like that the light stays in the same place relative to my eyes, thanks for implementing it!

Regarding number of lights one indeed seems to be not enough, but I am not keen on two symmetrical ones from two sides either. I think one should be theoretically enough, if it is combined with some "ambient" light from all directions to avoid deep shadows. At the moment when there is only one light the bright spots look almost white while dark ones look close to black. Is it possible to make it less drastic? In particular for the bright spots it seems to me that things should be close to "bright blue" rather then white, i.e. the color itself should not change, just its intensity.

For the record apart from cube and sphere I was looking at this example from an earlier ticket as well:

var('y')
plot3d(sin(x*y),(x,-1,1),(y,-10,10),viewer='threejs',aspect_ratio=[1,.1,1])

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

697adab	Adjust default lighting
49083c5	JSONize ouput

[sagetrac-git]

Changed commit from 0361f41 to 49083c5

[paulmasson]

comment:8

Andrey, see if you like these defaults. It does look better to me!

I've also updated the Python output that appears in the web page source so that it all looks like proper JSON. This is to make all data in the page consistent with #22253, which is one bad doctest away from positive review.

I do want to give users the option to customize lighting, but that can be done or this ticket or another. The only reference point for user input that I have found is the Tachyon method light, which appears to only handle point lights so it's not much help.

I'm thinking of adding two keyword arguments, ambient_light and directional_lights, since those need to be specified differently. Does that sound reasonable to you? Or is there some other expected form of input that makes more sense?

[novoselt]

comment:9

Can't check how it looks at the moment but will next week.

Regarding keywords: since we didn't have such functionality before, I think whatever makes sense in threejs would work best (assuming that it will make sense for those who don't use threejs directly). If it is under user control, it may make sense to have lights which are both fixed and rotate with the scene.

[novoselt]

comment:10

Hmmm, as I understand it you have increased intensity of the ambient light and decreased the directional one. The result now looks a bit too flat to me, especially without actively rotating the plot, so maybe the same change but less drastic would be better. Otherwise I think this is the best setup I've seen for threejs so far.

Would be nice to get more opinions!

Also for those (like me) for whom it is not obvious: after git trac try 22261 you have to do make start, not just sage -b to install the new template.

[sagetrac-git]

Changed commit from 49083c5 to 2f6c4a2

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

2f6c4a2

Expose light colors in Python

[paulmasson]

comment:12

Since the option for customizing lights will take some effort to make its input user proof, I think that should happen on another ticket. For now I've exposed the default colors in sage/plot/plot3d/base.pyx so that you can fiddle with them if you want, just make sure all three inputs for Color() are equal to produce whitish light.

The previous settings were equivalent to ambient of .6 and directional of .4, and the settings before that were ambient .25 and directional .87. The current settings are .5 for both.

Thanks for mentioning git trac try since it's not currently in the user manual. I'll add it to the documentation for reviewing tickets at some point.

[novoselt]

comment:13

Merge conflict! Also I think we should merge this and listen to what others think of lightning.

[novoselt]

Reviewer: Andrey Novoseltsev

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b0ea28a

Modify default lighting

[sagetrac-git]

Changed commit from 2f6c4a2 to b0ea28a

[paulmasson]

comment:15

Merge conflict resolved

[vbraun]

Changed branch from u/paulmasson/22261 to b0ea28a