Add fallback locator point recenter #166
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
cozmo
reviewed
Apr 22, 2020
| @@ -212,6 +212,20 @@ function scorePattern(point: Point, ratios: number[], matrix: BitMatrix) { | |||
| } | |||
| } | |||
|
|
|||
| function recenterLocation(matrix: BitMatrix, p: Point): Point { | |||
There was a problem hiding this comment.
Any reason we wouldn't recenter vertically too?
There was a problem hiding this comment.
I didn't think it would matter, but after testing, it appears that also leads to a slight improvement.
| @@ -65,7 +65,7 @@ describe("locate", () => { | |||
|
|
|||
| it("locates a damaged QR code and guesses the finder pattern location", async () => { | |||
There was a problem hiding this comment.
Don't feel too strongly but wouldn't hurt to add a test case for this - Would be as simple as taking one of the originally failing ones and committing it into the locator tests.
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "counts": { | |||
| "failed": 47, | |||
cozmo
approved these changes
Apr 28, 2020
Closed
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.
We normally use the center of the quads as the location of the tracking points, which is optimal for most cases and will account for a skew in the image. However, In some cases, a slight skew might not be real and instead be caused by image compression errors and/or low resolution. For those cases, we'd be better off centering the point exactly in the middle of the black area. This change computes the location data for the naively centered points as it is little additional work and allows for multiple attempts at decoding harder images.
An example of this bug:
This change fixes 5 of the 47 currently failing tests, with no regressions.