WeTransfer / WeScan

Document Scanning Made Easy for iOS
MIT License
2.87k stars 561 forks source link

Save as PDF by Default #20

Closed samuelbeek closed 4 years ago

samuelbeek commented 6 years ago

What is the problem

I think we should save the documents as PDF by default, because:

jcampbell05 commented 6 years ago

I second this, whats best way of doing this ?

Boris-Em commented 6 years ago

PDFKit makes this pretty easy, but it's only supported on iOS 11+. Maybe we could drop support of iOS 10 for this specific feature @samuelbeek, @jcampbell05?

julianschiavo commented 6 years ago

I think this feature should be opt in by the creators of the app who use the framework - for example, my usage for WeScan would not use PDFs.

samuelbeek commented 6 years ago

@Boris-Em I agree that we could make it an iOS 11 only feature.

Boris-Em commented 6 years ago

Good point @justJS!

jcampbell05 commented 6 years ago

Some companies will be dropping iOS 10 support soon so I think there is little value trying to port PDF to iOS 10. So I'm happy for it to be just iOS 11.

julianschiavo commented 6 years ago

I also agree it's ok for PDF support to be iOS 11 only, but I really want WeScan to keep iOS 10 support for now. While it's only a small percent, it's still more users.

jcampbell05 commented 6 years ago

@justJS I'm just meaning to keep PDF iOS11 only and not the framework :) WeScan should support iOS 10 at least for little while longer

julianschiavo commented 6 years ago

Yeah, just making sure we're clear :smile:

julianschiavo commented 6 years ago

Thoughts on PSPDFKit? Supports iOS 10, has some other benefits

Note: I don't know about pricing or anything, just saw it around

jcampbell05 commented 6 years ago

That costs so a no go for me but I recalled that iOS always had lower level PDF graphics as Quartz was based on PDF. So we have the CGPDFContextCreate methods (Since iOS 2.0) we can use to easily write a basic PDF with the images into a file.

This should be okay as I think PDFKit was mainly introduced to handle other features beyond the rendering which the Quartz APIs didn't handle (annotations etc) which we won't likely need for a scanner.

So I think we should just use this API for iOS 10 and 11. The advantage is it can also export to JPG or PNG as well (Assuming you create correct context) https://developer.apple.com/documentation/coregraphics/cgpdfcontext

julianschiavo commented 6 years ago

👍 What does everyone think about how it should be implemented? I'm thinking, as I said, a host app setting, but any other ideas? Perhaps we could provide both the PDF and typical image back to the host app?

Boris-Em commented 6 years ago

Agreed. We could add a new scannedPDF property to ImageScannerResults. This way users get both a UIImage and a PDF back. Let's just make sure first that creating the pdf is very fast.

julianschiavo commented 6 years ago

👍 Since there's changes to the ImageScannerResults (the auto rotation) in #41, I can start working on this after that's merged (should be soon).

Edit: Well I might as well just check this out now, I have some free time. I'll update with how it goes.

julianschiavo commented 6 years ago

Hey, I've done some work on this but am waiting for #80 to be merged (it includes changes to ImageScannerResults)

jcampbell05 commented 5 years ago

@justJS now #80 is merged, did we merge the PDF stuff in ?

julianschiavo commented 5 years ago

I've started work: #133

julianschiavo commented 5 years ago

133 is done and ready for review 🎉

(it's a breaking change as it renames variables in ImageScannerResults)

EDIT: PDFs are supported on all versions of iOS! PDFKit wasn't suitable for this purpose so I'm using lower-level CoreGraphics APIs.

github-actions[bot] commented 4 years ago

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.