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 backend not replicating properties #11

Open
bdice opened this issue Feb 13, 2019 · 5 comments
Open

pythreejs backend not replicating properties #11

bdice opened this issue Feb 13, 2019 · 5 comments
Labels
bug Something isn't working

Comments

@bdice
Copy link
Member

bdice commented Feb 13, 2019

When using the pythreejs backend, only the first shape of a primitive is drawn, and it's solid black.
I saw this issue with @k-l-wang the other day and it's occurring on my machine too.

Version information:

  • plato from master branch (156bc56, also tested an older commit, 975c704)
  • pythreejs=2.0.2=py36_1000

Code to reproduce, in a Jupyter notebook:

import plato.draw.fresnel as draw
import numpy as np
positions = np.zeros((10, 3))
positions[:, 0] = np.arange(10)
positions -= np.mean(positions, axis=0)
spheres = draw.Spheres(positions=positions)
scene = draw.Scene(spheres)
scene.show()

fresnel backend output:
image

pythreejs backend output (it appears to be drawing either the first or the last shape - the one on the right side is drawn in this case).
image

@bdice bdice added the bug Something isn't working label Feb 13, 2019
@bdice
Copy link
Member Author

bdice commented Feb 15, 2019

The shape attributes aren't being replicated properly, and the lights are disabled unless users specifically request lights. This code snippet solves both problems, but I think both behaviors are bugs.

import plato.draw.pythreejs as draw
import numpy as np
positions = np.zeros((10, 3))
positions[:, 0] = np.arange(10)
positions -= np.mean(positions, axis=0)
# Add full arrays for colors and radii or else only one particle shows up
colors = np.array([[1, 0, 0, 1]]*len(positions))
radii = np.array([0.5]*len(positions))
spheres = draw.Spheres(positions=positions, colors=colors, radii=radii)
scene = draw.Scene(spheres)
# Add lights
#scene.enable('ambient_light', 0.0)
scene.enable('directional_light')
scene.show()

Result:
image

The replication problem can be seen by investigating these arrays. They should be (Nparticles * Nvertices, width) where width is 4 for RGBA colors and 3 for normal vectors. In this case, with 10 particles and 64 vertices in the sphere mesh, the shapes should be (640, 4) and (640, 3). Since the values are not being replicated, they are (64, 4) and (64, 3) instead.

color = spheres.threejs_primitive.geometry.attributes['color'].array
normal = spheres.threejs_primitive.geometry.attributes['normal'].array

Some lights should be enabled by default, as they are for other backends.

@bdice
Copy link
Member Author

bdice commented Feb 15, 2019

It seems like the failure to replicate comes from mesh.unfoldProperties.
@klarh It was my understanding that if a single value was provided for colors or radii, it would be replicated automatically. At least, several backends act that way. Do inputs need to be pre-processed so that len(colors) = len(radii) = len(positions) before calling mesh.unfoldProperties?

@klarh
Copy link
Collaborator

klarh commented Feb 18, 2019

@bdice using the size of the smallest array you're given is the expected behavior for mesh.unfoldProperties at the moment. I would guess that any backends in which the default values get replicated automatically are getting lucky in the way they were written with respect to numpy broadcasting rules.

I think it would be a great feature to make things like colors, orientations, and diameters be automatically broadcast (see glotzerlab/platoviz#44), but it's a little more complicated because properties can be set (or not) in any order, often they should not be checked until the next draw call is triggered, and sometimes people do want to draw just one thing (in which case we wouldn't want to broadcast).

As a separate issue, I'm all for setting up a default lighting scheme for the pythreejs backend in the name of getting something decent drawn on the screen with a minimal amount of code (so the light feature isn't necessary in this case). Default lights were added in the vispy backend in 33d7b81 if you want to use the same directions and relative intensities.

@bdice
Copy link
Member Author

bdice commented Feb 19, 2019

@klarh I've spent about 4 hours trying (and failing) to find a reasonable way to make non-position properties replicate/tile out to the number of shapes by default. I can't figure out a good way to do it without duplicating a lot of code and/or breaking the existing patterns for data storage and class inheritance. I would welcome your thoughts.

@bdice bdice changed the title pythreejs backend not rendering correctly pythreejs backend not replicating properties Feb 19, 2019
@klarh
Copy link
Collaborator

klarh commented Feb 19, 2019

@bdice I don't think it would be particularly straightforward, but maybe setting properties with dimension D-1 from what they are expecting would indicate that it should be automatically broadcast? The value would need to be immediately dispatched in the setter function and specially dealt with in the update_arrays (vispy)/render (povray)/equivalent functions in the various backends.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants