Skip to content
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

pythreejs Low Level Modifications (e.g. add threeJS Spot_Light) #276

Open
TimoFriedri opened this issue Oct 8, 2019 · 16 comments
Open

pythreejs Low Level Modifications (e.g. add threeJS Spot_Light) #276

TimoFriedri opened this issue Oct 8, 2019 · 16 comments

Comments

@TimoFriedri
Copy link
Contributor

m using your library extensively for the plotting of 3D objects (meshes and voxel models).

Unfortunately, the lightning is quite simple and it is hard to infer the shape from some angles.
I'd like to add additional light sources and activate shadows, which seems to be possible from the pythreejs side.

So far I have not been successful is doing so. It is quite hard to figure out what I have to edit/add ... in the respective scenes/meshes/ ... which are stored in a figure.widgets container.

Could someone give some hints on how to achieve "adding and modifying lightning for ipyvolume plot?

Maybe an low level modification guide in the docs would be nice.

Thanks

@maartenbreddels
Copy link
Collaborator

Hi Timo,

I think we have to change the shaders to take into account the pythreejs lighting. That involves diving into threejs, I am happy to assist you with the ipyvolume side of things.
Basically js\glsl\*needs to be edited, and Figure should probably be an Scene subclass, or Object3D, so it can have lights, and then follow something along these lines:
https://stackoverflow.com/questions/30151086/threejs-how-do-i-make-a-custom-shader-be-lit-by-the-scenes-lights.
Do you feel like attacking this issue?

cheers,

Maarten

@rinftech-github
Copy link

rinftech-github commented Mar 23, 2020

Greetings @maartenbreddels !

We are working on adding Lighting(Ambiental, Directional, Spot, Point, Hemisphere) support for ipyvolume rendering.
Below we are describing how we are planning to go about it.

Similar to pylab functions like plot_mesh, we are adding functions for every light type(exp addDirectionalLight()), each with specific parameters (some being adjsutable through sliders and color pickers).
Further, a Light widget class is necessary with associated TypeScript classes(LightView, LightModel).

Our current shader tests feature vertex and fragment code from three.js(Lambert, Phong, Physical), combined with existing ipyvolume shader code, in order to be up to the visual standard from three.js examples.

What is your opinion regarding the plan above?
Are there any shader modules that should not be present in the new code? (exp: skinning, morphtargets)
We are planning to specify as parameter the target scene in which a specific light will be added in order to diferentiate between scene_scatter, scene_volume, scene_opaque. Do you agree with this approach?

Please, if you have time, can you give any feedback in order for us to better prepare for the eventual Pull Request?

@rinftech-github
Copy link

We are concentrating on this until the end of March 2020. So we would appreciate any feedback as soon as possible. Thank you!

@maartenbreddels
Copy link
Collaborator

That is awesome to hear. I think no widgets should be required, I think we should try to build as much as possible on top of pythreejs. What do think about that?

@rinftech-github
Copy link

Thank you for your answer!
So you propose not to add classes Light(widgets.Widget) (python) and LightView, LightModel (ts)?
In order to specify the scene in which the a light is added (scatter, volume, opaque) where do you recommend to add the logic?

@maartenbreddels
Copy link
Collaborator

Indeed, I thought a bit about this, and I think ipyvolume can use for instance:

I did the same with the Camera class/widget. It should also be possible to rely on the shader snippets from pythreejs. I think it does not require a lot of coding, but a lot of sorting out how threejs works. But I think it will pay of in not having to make a lot of new widgets, and possibly have more integrations with other threejs based projects.

@rinftech-github
Copy link

rinftech-github commented Mar 24, 2020

I'm really grateful for your prompt answers!
I investigated the code more and you are right. Creating a separate Light widget would be an nonoptimal way to go about it. Using pythree.js would be much easier.
Below is a very simple test snippet added to pylab.py. addAmbientLight is called within a jupyter notebook.

def addAmbientLight(color=default_color):
ambientLight = pythreejs.AmbientLight(color)
fig = gcf()
fig.scene.add(ambientLight)

This does not give the desired result - no light added.
Mention: I'm using plot_mesh, so scene_scatter should contain the added light(s). Shouldn't there be a link between this.renderer.scene_scatter(FigureView) and fig.scene ?
Mention 2: I have changed the mesh shaders in order to support lighting. When adding lights from ts code, the result is as expected.
Again, thank you for your answers, help and time!

@rinftech-github
Copy link

rinftech-github commented Mar 25, 2020

@maartenbreddels Please excuse the long list of questions. Let me reduce them to just one:
Is there a way to access the main three.js scene (scatter) from python code? (pylab, widgets etc). ipv.gcf() does not appear to be it. Thank you!

@maartenbreddels
Copy link
Collaborator

No, there is no scene in the python side, it is not exposed, it's only on the js side.

@rinftech-github
Copy link

Thank you :)
I almost finished the implementation for adding all types of lights + shadow support(if available).

@maartenbreddels
Copy link
Collaborator

Ok, there is actually!
But indeed, it requires adding the code snippets to the shaders, are you saying you have that working?? :)

@rinftech-github
Copy link

Yes :)
https://imgur.com/a/wLsmg9x

@maartenbreddels
Copy link
Collaborator

Whow, I'm impressed! :)
Looking forward to seeing a PR. Please open a PR soon, so I can take a look, it doesn't have to be pretty even.

@rinftech-github
Copy link

rinftech-github commented Mar 27, 2020

Thank you! We will open a PR as soon as possible!

@TimoFriedri
Copy link
Contributor Author

Hi,

the PR will be handled from our official HRI-EU github account (https://github.com/HRI-EU).
I plan to manage this the next days/weeks.

The first implementations of light support should be finished tomorrow.

@TimoFriedri
Copy link
Contributor Author

TimoFriedri commented Apr 6, 2020

Hi,

here is some feedback from @rinftech-github regarding the issues remaining with the scatter plot type:


Regarding the shadow mapping issue with instanced geometry, this was discovered in three.js back in 2018:
mrdoob/three.js#13995

After some back and forth the maintainer implemented a fix in September 2019 which looks like it was included in release 109 as seen here:
mrdoob/three.js#17505

To benefit from it we would need pyhtreejs to be updated to at least three.js 109; We opened an issue on their github for that:
https://github.com/jupyter-widgets/pythreejs/issues/31

I tried to assess what it would look like for us to make the change and it is not something to be done in a day or two, it would be so much more efficient for the maintainer to do the update.
After updating pythreejs, we would still have to refactor the code in the scatter class to use mrdoob's new class InstancedMesh so the fix wouldn't be applied automatically so to speak, but it would be solvable.

Attached is a screenshot with the shadow mapping issue – you can clearly see that the map only features one cube shadow.

instance shadow mapping issue


Maybe we will tackle this the next weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants