Skip to content

fix return value of computeCloud2CloudDistance#129

Merged
dgirardeau merged 1 commit intoCloudCompare:masterfrom
tmontaigu:fix-computeDist-return
May 1, 2026
Merged

fix return value of computeCloud2CloudDistance#129
dgirardeau merged 1 commit intoCloudCompare:masterfrom
tmontaigu:fix-computeDist-return

Conversation

@tmontaigu
Copy link
Copy Markdown
Contributor

It did not normalize to RESULTS::sucess
and the document about it was incorrect

From a check in CC it does not seems like this will break anything

@tmontaigu tmontaigu force-pushed the fix-computeDist-return branch from aa8219b to 6524d87 Compare April 24, 2026 20:55
Copy link
Copy Markdown
Member

@dgirardeau dgirardeau left a comment

Choose a reason for hiding this comment

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

Good catch. I see that most of the call to computeCloud2CloudDistances don't check the return value against DISTANCE_COMPUTATION_RESULTS::SUCCESS anyway (they simply compare it ti 0).

Can you fix the calls to computeCloud2CloudDistances in CCCoreLib at least in this PR? (mostly in RegistrationTools.cpp I believe)

@tmontaigu tmontaigu force-pushed the fix-computeDist-return branch from 6524d87 to 1bcc359 Compare April 27, 2026 20:05
@tmontaigu
Copy link
Copy Markdown
Contributor Author

I was thinking that maybe we can make the SUCCESS value be 0 and not 1

@dgirardeau
Copy link
Copy Markdown
Member

That would make sense in terms of consistency with the old code.

It did not normalize to RESULTS::sucess
and the document about it was incorrect
@tmontaigu tmontaigu force-pushed the fix-computeDist-return branch from 1bcc359 to 74ea6e1 Compare April 28, 2026 18:32
@tmontaigu
Copy link
Copy Markdown
Contributor Author

(I don't have the rights to merge on this repo)

@dgirardeau dgirardeau merged commit be320e0 into CloudCompare:master May 1, 2026
4 checks passed
@dgirardeau
Copy link
Copy Markdown
Member

I'll update things on CC's side

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