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

Add option to disable inverting #78

Merged
merged 2 commits into from
Aug 15, 2018
Merged

Conversation

cozmo
Copy link
Owner

@cozmo cozmo commented Jul 18, 2018

Inverting the image in the case of not finding a code in the initial image introduces a ~50% performance hit. This adds an options object to allow disabling the inversion if you don't expect to be scanning inverted QRs.

Defaults to the existing behavior (image inversion) but adds a note to the readme that this will probably be disabled in a future backwards incompatible change.

This also sets up the options object for #76.

@cozmo cozmo requested a review from jefff July 18, 2018 22:51
david-boles added a commit to david-boles/jsQR that referenced this pull request Jul 19, 2018
@david-boles
Copy link

david-boles commented Jul 22, 2018

@cozmo What would you think about extending this new option to also support scanning only inverted and scanning inverted first? I think I'm going to be scanning images that mostly have inverted QR codes, so the later would make things a bit faster for me. It'd be more efficient for jsQR to do the inversion once it's binarized than me inverting all the RGB data before sending it to jsQR.

@cozmo
Copy link
Owner Author

cozmo commented Jul 24, 2018

Interesting,

What about something like attemptInverted: true | false | "first" | "only" - Where "first" would try the inverted one first, but still attempt both, and "only" would invert and then only try that one.

@david-boles
Copy link

That sounds good to me! Maybe make it an enum instead of boolean/string combo?

@cozmo cozmo force-pushed the add-option-to-disable-inverting branch from 26e4ad2 to ced4ba4 Compare August 15, 2018 08:17
@cozmo cozmo merged commit 807b073 into master Aug 15, 2018
@cozmo cozmo deleted the add-option-to-disable-inverting branch August 15, 2018 08:23
@alexandertrefz
Copy link

@cozmo
This seems like a major performance boost for those that don't need the inversion – can we cut a 1.1.2(since it defaults to the same behaviour) release from this?

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