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

Fix issue #201 : add PDAL reader #2014

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Husamm
Copy link

@Husamm Husamm commented Feb 24, 2025

Implemented initial PDAL support that currently handles only LAS file formats, laying the groundwork for future enhancements and additional file format integration.

"options": {
"load-plugins": "pdal",
"volume": true,
"volume-inverse": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think pdal formats should be opened as volumes though

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove both volume and volume-inverse, since i don't exactly what they do , i read this :
{"volume", "v", "Show volume if the file is compatible", "", "1"},
{"volume-inverse", "i", "Inverse opacity function for volume rendering", "", "1"} } },

but i'm not sure exactly what that's means, and if that needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these .json file are the "default" config file that we install for each plugin.
For vdb, it makes sens to use volume rendering because in essence vdb format contains volume not surfaces or point cloud.

In you case, you may want to enable point-sprites.

In short, whatever option we may need to make the rendering look as expected by the user, we should put here.

You can just remove the volume part for now, just keep the load-plugins part. We will choose together the right options once this work is more advanced.

"options": {
"load-plugins": "pdal",
"volume": true,
"volume-inverse": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think pdal formats should be opened as volumes though

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove both volume and volume-inverse, since i don't exactly what they do , i read this :
{"volume", "v", "Show volume if the file is compatible", "", "1"},
{"volume-inverse", "i", "Inverse opacity function for volume rendering", "", "1"} } },

but i'm not sure exactly what that's means, and if that needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets discuss it here: #2014 (comment)

@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<mime-info xmlns="http://www.freedesktop.org/standards/shared-mime-info">
<mime-type type="application/vnd.pdal">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think this is correct:
https://www.iana.org/assignments/media-types/media-types.xhtml

Search for vnd.las

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


if (F3D_PLUGIN_BUILD_PDAL)
add_subdirectory(pdal)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing end of line

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean at the end of the file ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

CMakeLists.txt Outdated
@@ -109,6 +109,7 @@ find_package(VTK 9.2.6 REQUIRED
opengl
IOExodus
IOOpenVDB
IOPDAL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you used a tab here, lets use spaces

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll change this in my editor settings.

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.

2 participants