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

Ply reader #1

Merged
merged 3 commits into from
Dec 1, 2020
Merged

Conversation

rezural
Copy link
Contributor

@rezural rezural commented Dec 1, 2020

This adds support for reading from ply files, of the format:

ply
format ascii 1.0
comment ...
element vertex 2440181
property float x
property float y
property float z
property float nx
property float ny
property float nz
end_header

@rezural
Copy link
Contributor Author

rezural commented Dec 1, 2020

BTW, this worked straight up amazing on 3 million point ply files. Resulting in this rendering:

https://www.youtube.com/watch?v=BbkCnsKESJU

Thankyou very much!

@w1th0utnam3 w1th0utnam3 force-pushed the master branch 2 times, most recently from b01a543 to d3b303b Compare December 1, 2020 09:51
@w1th0utnam3 w1th0utnam3 merged commit e9888bb into InteractiveComputerGraphics:master Dec 1, 2020
@w1th0utnam3
Copy link
Member

Thanks! I'll make some small changes to the code

@w1th0utnam3
Copy link
Member

If you have the time, maybe consider rewriting it using custom structs instead of using the HashMap based access? See e.g. https://github.com/Fluci/ply-rs/blob/master/examples/read_ply_to_structs.rs

@rezural
Copy link
Contributor Author

rezural commented Dec 2, 2020

Yeah, OK.

I was going to do this, but couldn't implement the trait on na::Vector3 directly (I wonder if there is a workaround for this?), and it seemed kinda invasive to add those structs just for one file format.

I will look at this in the next couple of days.

Thanks!

@rezural rezural deleted the ply-reader branch December 2, 2020 03:19
@rezural rezural restored the ply-reader branch December 2, 2020 03:19
@rezural rezural deleted the ply-reader branch December 2, 2020 03:19
@w1th0utnam3
Copy link
Member

Yeah, right. I think one could work around this by introducing a transparent newtype:

use nalgebra::Vector3;

#[repr(transparent)]
struct Vec3r(Vector3<Real>);

implementing the trait on this and then just transmuting the result of the parser from Vec<Vec3r> to Vec<Vector3<Real>>.

Although in general, I don't really like that the PropertyAccess trait does not support any error reporting as it only returns Options and not Resultss... But I guess missing or unparsable components of the vectors should usually not be a problem (as PLY files are usually created by other software that probably writes correct files), and in the rare case this happens, one can still give a respective message in the panic.

One thing we should make sure later though is to give meaningful errors if you try reading e.g. a file with doubles as a f32 Vec.

@w1th0utnam3 w1th0utnam3 added the enhancement New feature or request label Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants