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

[filter] Revert Morton #1786

Merged
merged 5 commits into from
Jan 25, 2018
Merged

[filter] Revert Morton #1786

merged 5 commits into from
Jan 25, 2018

Conversation

pblottiere
Copy link
Contributor

Hi,

This PR adds a new option for the MortonOrder filter: revert. If this option is true, then the indexing is made according to a revert Morton code. It allows to have a good spatialisation.

For example, with a pipeline Splitter -> Revert Morton, we may successively select N representative points of a patch:

lod

We are using this filter in LOPoCS with the pgpointcloud writer. BTW: Oslandia/lopocs#23.

Let me know what you think about it and if I have to update something.

@hobu
Copy link
Member

hobu commented Jan 24, 2018

@pblottiere Can you tweak the docs to include this picture and a section about what the option does with links to LOPoCS? https://github.com/PDAL/PDAL/blob/master/doc/stages/filters.mortonorder.rst

@abellgithub
Copy link
Contributor

abellgithub commented Jan 24, 2018

I'm thinking that it would be nice if the original code could be modified to fit this model: don't have a special sort for the map, but return an integer that represents the order. This would eliminate duplicate code. I could do this as a second pass, I guess.

@abellgithub
Copy link
Contributor

Also, is there some documentation that explains the math/logic of this ordering? "Revert morton" doesn't mean anything to me. I didn't find anything Googling, either.

@hobu
Copy link
Member

hobu commented Jan 24, 2018

Also, is there some documentation that explains the math/logic of this ordering? "Revert morton" doesn't mean anything to me. I didn't find anything Googling, either.

I think maybe the option should be called reverse for "Reverse Morton", or maybe invert for "Inverted Morton". It's something they made up for the LOPoCS to create dispersement of data so when they data are fetched, they are presorted for pleasant looking visualization.

@abellgithub
Copy link
Contributor

Perhaps it would be better to pass a function to the filter that would allow one to generate an index based on a coordinate? We could build-in the current and proposed ordering. Maybe this is a to-do.

@pblottiere
Copy link
Contributor Author

Can you tweak the docs to include this picture and a section about what the option does with links to LOPoCS?

Done.

@pblottiere
Copy link
Contributor Author

I'm thinking that it would be nice if the original code could be modified to fit this model: don't have a special sort for the map, but return an integer that represents the order. This would eliminate duplicate code. I could do this as a second pass, I guess.

I didn't want to update the original code but I may propose a commit to clean duplicate code if you want?

@pblottiere
Copy link
Contributor Author

Also, is there some documentation that explains the math/logic of this ordering? "Revert morton" doesn't mean anything to me. I didn't find anything Googling, either.

for the LOPoCS to create dispersement of data so when they data are fetched, they are presorted for pleasant looking visualization.

Exactly. But I'm sure that @Remi-C will be happy to talk to you about it if you want more information :).

@abellgithub
Copy link
Contributor

My only concern is that Morton-ordering implies a scheme for ordering 2-D points in 1-D while preserving locality to some extent. Unless I'm mistaken, this algorithm isn't attempting to preserve locality. Perhaps this is a naming issue and nothing more.

@hobu hobu added this to the 1.7 milestone Jan 25, 2018
@hobu hobu merged commit 5e7c398 into PDAL:master Jan 25, 2018
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.

None yet

3 participants