-
Notifications
You must be signed in to change notification settings - Fork 302
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
feature: Add potree 2.0 parser #2137
Conversation
Hi @ketourneau and thanks for your contribution!
I advise to add some tests (duplicates of potree 1.x parser tests and/or your own). |
@Desplandis thanks for your reply, I will add potree 2 tests. Actually we got some trouble to use use itowns package with potree2 loader (web worker) in a vite project. If we change in itowns .babelrc So if someone can help to fix the problem (I think it's a babel config problem) ? |
@Desplandis FYI those very same web workers cannot work on our end without some kind of tinkering with Babel, which breaks all iTowns' tests. At least, that's what we got so far since our resources around web workers are currently busy and we cannot allocate any of their time. What we do know: since we call those web workers from We could add more logic to this worker loading process (something in line with what pdf-js is doing) but we still wanted to ask any of this project's contributor if there will be some plan on the way for either outputing or if it should be done in a certain way. Our endgoal would be to put iTowns' library as it is, without cloning or adding on top of it any building logic. |
@ketourneau @Makaronelle If I understand your issue correctly, it seems the problem is that I am not familiar with After setting a relative path to the workers file, it seems to fix this issue: ✓ 624 modules transformed. dist/index.html 0.52 kB │ gzip: 0.31 kB dist/assets/potree2-decoder.worker-ffeff9b8.js 10.37 kB dist/assets/potree2-brotli-decoder.worker-7fc1bb98.js 162.99 kB dist/assets/index-323ff7d8.css 0.07 kB │ gzip: 0.08 kB dist/assets/index-3d1b9e4c.js 2,241.98 kB │ gzip: 614.49 kB |
@Desplandis Thanks for your reply. I try with relative path but we got the same error in vite (maybe we could resolve it later).
I try to add babel-plugin-transform-import-meta but I got this error when I try to execute potree2BinParser test :
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This a nice start. =)
I left some comments that should be addressed before getting this merged.
I did not however reviewed the two workers because eslint
generates two many errors, I'll do it in the following review.
@ketourneau I also see that you have added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I modified code according to your comments.
I still have 2-3 remarks to deal with and I will fix and add tests on potree2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add more description about metadata.json and improve Promise in Potree2BinParser.
I'm still working on tests.
@Desplandis I've completed the tests. Is everything good except the bigint issue ? |
3d7f72b
to
2fa2194
Compare
d0dc19d
to
153789b
Compare
Hi, any update on this? |
@gauthier-th Following #2195, I expect this PR to be merged upstream by the end of February.
|
d73a8bf
to
d3398eb
Compare
6cad6a7
to
163ce34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for your PR. =)
Description
We add Potree2Source, Potree2Layer, Potree2Node and example (potree2_25d_map.html) for loading potree 2.0 file format in iTowns.
Pull request for sample data must be accepted before (iTowns/iTowns2-sample-data#15).
Motivation and Context
We need to load potree 2.0 file format in iTowns to meet our project requirements.
Do I need to duplicate the potree tests to run them on potree2 ?