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

(PointCloudLayer) add points filtre based on classification #2374

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ftoromanoff
Copy link
Contributor

@ftoromanoff ftoromanoff commented Jul 30, 2024

Any classification scheme used for PointCloudLayer havea property 'visible' for each class.
This PR aims at using this value to filter point whichever the visualization mode choosen.

In other word, if a class has its property 'visible' set to false, we want to discard all points having the value in the rendering, (even if we choose to use the 'intensity' rendering)

In this PR, I used migrate all PointCloud examples to lilGUI instead of datGUI

@ftoromanoff ftoromanoff force-pushed the filtrePoint branch 4 times, most recently from f024a73 to fa74f28 Compare July 31, 2024 14:49
@ftoromanoff ftoromanoff marked this pull request as ready for review August 1, 2024 14:33
@ftoromanoff ftoromanoff changed the title Filtre point (PointCloudLayer) add points filtre based on classification Aug 1, 2024
@HoloTheDrunk
Copy link
Contributor

I see we're using a conditional discard in the fragment shader; it might be worth benchmarking the performance of this against using a transparent color or performing the filtering earlier on CPU (which, given the right optimization work, might turn out to be the best solution).
See this (old) SO post for an explanation: https://stackoverflow.com/questions/8509051/is-discard-bad-for-program-performance-in-opengl
This is always going to depend on each user's hardware anyways, but checking wouldn't hurt if we can gain a bit of performance.

Copy link
Contributor

@Desplandis Desplandis left a comment

Choose a reason for hiding this comment

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

Need another review.

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