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

Allow Multiple Scans in One Session (Batch Scanning) #11

Closed
Boris-Em opened this issue Mar 15, 2018 · 28 comments
Closed

Allow Multiple Scans in One Session (Batch Scanning) #11

Boris-Em opened this issue Mar 15, 2018 · 28 comments
Assignees
Labels
enhancement New feature or request Stale

Comments

@Boris-Em
Copy link
Contributor

It should be possible to scan multiple documents without having to go through the whole flow.

@Boris-Em Boris-Em added the enhancement New feature or request label Mar 15, 2018
@Boris-Em Boris-Em changed the title Allow Multiple Scans in One Session Allow Multiple Scans in One Session (Batch Scanning) Jun 27, 2018
@Boris-Em Boris-Em self-assigned this Jun 27, 2018
@jcampbell05
Copy link
Contributor

Was any progress made on this in the end ? :)

@julianschiavo
Copy link
Contributor

#41 adds auto scan, which may help. Also, like the PDF support, I think this should be a host app setting.
I'd be happy to work on a way for the host app to pass in settings (perhaps using the new CaptureSession singleton) after we get #41 merged.

@Boris-Em
Copy link
Contributor Author

@jcampbell05 I'm pretty much done with this already. I'm waiting for @justjs' pull request to be merged in, because I'll need to update a bunch of stuff at that point.

@jcampbell05
Copy link
Contributor

Cool cool, I'll get a move on with those tests :)

@jcampbell05
Copy link
Contributor

Now that #41 is in we can resume this :)

@Boris-Em
Copy link
Contributor Author

I'll open a PR for that soon!

@julianschiavo
Copy link
Contributor

Do we have any agreement on the host app setting for this yet? While I do think this would be a great idea, the app I'm currently working on (and I'm sure others too) would require a lot of work to support multiple scans, so I'd prefer it to not be forced on for now.

@jcampbell05
Copy link
Contributor

@justjs I think a simple true/false flag would probably be enough like a lot of the UIKit APIs have for multiple.

@julianschiavo
Copy link
Contributor

Yeah I'd be fine with that, but as WeScan don't currently have a preferences system in use, I think it should be implemented well so it doesn't come back to bite us in the future.

@Boris-Em
Copy link
Contributor Author

Good call @justjs. What's your suggestion for implementing it?

@julianschiavo
Copy link
Contributor

Well I'm not sure what others think, but I think we certainly shouldn't pass it in as separate arguments to ImageScannerController's init. I've seen that typically libraries use a, for example, "WeScanPreferences" class with all the different preferences. Then, you initialize the class and set it's values, and pass it into the init.

Example:

class WeScanPreferences {
  var shouldScanMultipleItems: Bool = true
}
class ImageScannerController {
  init(preferences: WeScanPreferences) { }
}

Then somewhere else (host app):

...
let prefs = WeScanPreferences()
prefs.shouldScanMultipleItems = true
let controller = ImageScannerController(preferences: prefs)

@jcampbell05
Copy link
Contributor

@justjs very like Zendesk which is very nice :)

@Boris-Em
Copy link
Contributor Author

This seems like a good solution. Essentially the introduce parameter object pattern.

@jcampbell05
Copy link
Contributor

Has anything been done on this, keen do some work on this myself :)

@julianschiavo
Copy link
Contributor

I'm not sure, but I believe @Boris-Em had done some work? I'll open a PR for that soon!

@echamussy
Copy link

I'm also very interested on having multi page scanning. I think one of the first steps is to re-organize the navigation code. My proposal is to make the 'ScannerViewController', 'EditScanViewController' independent from a navigation controller navigation. In the ScannerViewController instead of pushing to the EditScanViewController we could create a protocol with a method like

func scannerViewController(_ scannerViewController:ScannerViewController didCapturePicture picture: UIImage, withQuad quad: Quadrilateral?)

This protocol could be then implemented by the ImageScannerController to push the EditScanViewController but it would also allow to implement your own navigation if the ScannerViewController was to be invoked individually rather than via the ImageScannerController nav controller.

This would also allow to keep the user in the ScannerViewController to support multi page scanning and then show all images scanned in a different controller (similar to what the Scanner PRO from Readdle app does). I prefer this approach instead of passing a configuration object because it would allow to keep the ScannerViewController simple and we can group scanned data in a different class.

Let me know your thoughts. I can implement these changes and submit a pull request but I'd like to know your thoughts first.

@echamussy
Copy link

Hi. As a follow up to my previous comment I've been working on implementing multiple scans and I have made quite a lot of progress towards this objective. I had to make changes to the navigation structure and implemented a view controller to be able to view and edit all pages scanned in one session. You can see the progress I've made in my fork here:

https://github.com/echamussy/WeScan

If you have a chance to take a look at the code in my fork your feedback is welcome. I obviously still need to clean things up and wrap up all the details but I think is coming together nicely.

@n1tesh
Copy link

n1tesh commented Mar 4, 2019

@echamussy Just a suggestion, it would be great if you add preview of latest image where u shows counts of images clicked. Like CamScanner's batch/multiple scanning.

@n1tesh
Copy link

n1tesh commented Mar 11, 2019

@echamussy Getting memory warning/crash after clicking 7-8 images continuously. Memory goes beyond 1GB on iPhone X in few clicks.

@mobileonekh
Copy link

mobileonekh commented Apr 18, 2019

@echamussy after several images(4,5), the app is getting slow and it keeps spinning for around 1 minute before i can take another shot. I used iphone 6+.

Memory is also an issue. a 10 page scan crashed the app with Low Memory.

thanks

@jcampbell05
Copy link
Contributor

jcampbell05 commented May 2, 2019

@echamussy did you open a PR for this, My company can sponsor some developer time from one our developers to get this finished. cc @Boris-Em @AvdLee

@AvdLee
Copy link
Contributor

AvdLee commented May 13, 2019

@jcampbell05 that would be awesome! We would be more than happy to review your code to keep you up and running. Let me know if you have any questions!

@echamussy
Copy link

@jcampbell05 @AvdLee sorry for not responding earlier. I didn't create a PR for this as I felt that it still needed to be cleaned up, however the multi page scanning functionality is complete on the fork I created (on the develop branch), PDF creation works and I'm not experiencing the memory consumption issues mentioned above by @n1tesh and @mobileonekh

Unfortunately I haven't kept my fork up to date with the main project but if you want I can make a pull request and perhaps the developer you mentioned can do the merge? Let me know :)

@jcampbell05
Copy link
Contributor

@echamussy yes please that would be good 😀 we did some UI tweaks on our side so we can review it all.

Ping me when you're ready and I can loop with you @AvdLee

@echamussy
Copy link

@jcampbell05 Sounds good! I just spent couple of hours merging the develop branch from the main repo into my fork and I'm almost done fixing conflicts and merging things together. I should have it done soon and I will submit a PR for your review. Talk soon. Cheers!

@echamussy
Copy link

@jcampbell05 @AvdLee I just submitted a PR with support for multi page scans. This contains several breaking changes as I had to modify the navigation and the structure of some method signatures. I added a note to the PR with the changes I implemented (hopefully I listed all) and a few items that still need to be taken care of. The main challenge currently is to fix importing from camera roll which currently does not work well with images that are not in portrait orientation.

Your feedback is welcome. Let me know if you have any questions. Cheers! 😀

@jcampbell05
Copy link
Contributor

@echamussy Cheers I'll check this out tomorrow and will see if there is any improvements I can contribute from our internal version. I'll submit a PR to yours with any of our tweaks :)

@github-actions
Copy link

github-actions bot commented Feb 9, 2020

This issue is stale because it has been open for 30 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale
Projects
None yet
Development

No branches or pull requests

7 participants