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

Updates from ShaderMaterial to PointsMaterial in Particles.js #217

Closed
wants to merge 1 commit into from

Conversation

carlosjoserg
Copy link

Hi,

I was having issues with the current implementation using the latest of the THREE lib, and after struggling a bit with my very bad javascript skills, I managed to make it work.

I couldn't generate the build files, since I'm on Ubuntu 16.04 and the commands in the readme file didn't work for me. If someone tells me how to do it, I'd be glad to add the build files to this PR.

I can tell you it works for me, here you can see a screenshot with two laser scans of size 0.1 (different from the default 0.05) and separate colors (different from the default 0xFFA500).

screenshot from 2018-04-03 16-42-52

@mbredif mbredif mentioned this pull request Apr 4, 2018
@mbredif
Copy link
Contributor

mbredif commented Apr 4, 2018

Thanks for this PR.
I have managed to clean the code of #216 and incorporate this PR into #218.
Could you please test it ?

@carlosjoserg
Copy link
Author

I don't see my changes in the build files from your branch (the one being reviewed in #218), in fact, when I try them, I see it like this and should be like above:

screenshot from 2018-04-08 11-36-28

However, I see the Particles.js file did change, but those changes are not reflected in the build files either.

I'm not fully familiar with JS dev pipelines, but my guess is that you didn't build (that is putting all function into a single file) the ROS3D.js library? It'd be easier for me if you build it and push it (you'd need to do it eventually anyway, right?), so I can just drop the library file into my setup for a quick test.

@mbredif
Copy link
Contributor

mbredif commented Apr 9, 2018

I forgot to mention that I have introduced an API change which has now a proper deprecation warning.

I looks like it impacted you because the color option moved into the material option (see pointcloud2 and kitti examples), so that the material option can directly be passed to construct a THREE.PointsMaterial.

Alternatively to this topic-wise coloring, the colorsrc/colormap options enable per-point coloring by selecting a colorsrc source field/axis and converting its per-point value to a color using a colormap function (default is constructing a THREE.Color directly from a 'rgb' field), as in the kitti example :

image

@jihoonl
Copy link
Member

jihoonl commented Apr 15, 2018

@carlosjoserg Hello, I just merged #218 that updates the major pipeline to visualize pointcloud2 and laserscan. Could you check if it fixes your issue? Note that Particles.js has been renamed to Points.js in favor of change in three.js.

@carlosjoserg
Copy link
Author

Hi @jihoonl

Nope, that PR doesn't work for me. I understand Particle.js being renamed to Points.js is within LaserScan, and I'm using it like:

lidar_front_ = new ROS3D.LaserScan({
		ros : ros_,
		topic : '/device/lidar_front_left/scan',
		tfClient : tf_client_,
		size : 0.1,
		color : 0xFFA500,
		rootObject : viewer3D_.scene});

With the underscored names being previously intialized.

I'll try to figure out what changed then. I'll come back if I find it.

In the meantime I'll close the PR, since it got outdated.

Cheers, and thanks.

@mbredif
Copy link
Contributor

mbredif commented Apr 16, 2018

Could you try this ? (the material defining options have been moved to their own object)

lidar_front_ = new ROS3D.LaserScan({
		ros : ros_,
		topic : '/device/lidar_front_left/scan',
		tfClient : tf_client_,
		material: { size : 0.1, color : 0xFFA500 },
		rootObject : viewer3D_.scene});

@carlosjoserg
Copy link
Author

Thanks for the fast reply.

The colors are recognized but not the number of points, I'm only seeing one point per topic. I don't get any messages on the web console either.

screenshot from 2018-04-16 16-50-19

The setup is the same as in the comment above.

@mbredif
Copy link
Contributor

mbredif commented Apr 16, 2018

Thanks for the bug report, there was some spelling issue.
I am opening a new PR (#223) to fix that, could you please give it a try and give feedback there ?

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

Successfully merging this pull request may close these issues.

3 participants