fix homography assert #325
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wxmerkt
approved these changes
Apr 17, 2024
|
Sorry for the delayed response (and also for my lack of activity on this repo for a long time). My intention was that the following if statement, the |
|
As for the axis-aligned quads, I don't yet see why those would cause failure, since there should still be only one valid homography solution. I'll look a bit more next week |
mkrogius
approved these changes
May 5, 2024
| @@ -445,7 +445,9 @@ static matd_t* homography_compute2(double c[4][4]) { | |||
| } | |||
| } | |||
|
|
|||
| assert(max_val_idx >= 0); | |||
| if (max_val_idx < 0) { | |||
There was a problem hiding this comment.
Optional: This condition could be combined with the max_val < epsilon check below
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the "Find best row to swap with." section in
homography_compute2would not find a value greater than 0 in a row,max_val_idxwould stay-1and maybe be used later to index a matrix. We protected against the negative indexing (in debug mode) by checking for a positive index in 3a0a155.But this causes issues for false-positive quad detections (#321). In this particular case, ignoreing the negative index would lead to
return NULL;viamax_val < epsilon, so we can also justreturn NULL;here and ignore that quad instead of stopping the process.Fixes #321 .
@mkrogius As far as I understand
c[4][4]contains the 4 correspondences from the 2D pixel coordinates in the image to the 2D coordinates of the unit square with +/-1 edge coordinates. But I don't fully understand whyhomography_compute2does not deal with this special case of axis-aligned quads. Do you see a better way to handle this case?