dagronf / QRCode

A quick and beautiful macOS/iOS/tvOS/watchOS QR Code generator/detector library for SwiftUI, Swift and Objective-C.
MIT License
438 stars 57 forks source link

Numeric/Alphanumeric Support in QRCode External #11

Closed CarterVE closed 1 year ago

CarterVE commented 1 year ago

Hi,

I've been using this package in my watchOS Swift project, and it's amazing for drop-in support for QR code generation on the Apple Watch! I'm having a small issue however, and I think I've identified a fix.

My use case requires dense information encoding, and if I could use the alphanumeric version of QR encoding I could store about 45% more information in a single qr code. You call the binary encoding function of swift-qrcode-generator: https://github.com/dagronf/QRCode/blob/fbc9a1d13a71205631ba46b018c562a2eb9d6c06/Sources/QRCodeExternal/QRCodeGeneratorWrapper.swift#L40 However, I noticed that the package does support different encoding types: https://github.com/dagronf/swift-qrcode-generator/blob/47b0bf6c66a21046a09839e01019f1de0918dddd/Sources/QRCodeGenerator/QRCode.swift#L66

Instead of calling the binary encoding method directly, if the text encoding method was called (by converting Data to a String, and then calling the encode(text: String, ...) method), this would allow for the potential for greater information density in the QR encoding. Even better, this method checks whether the text can be encoded as numeric, then alphanumeric, and then falls back to binary if neither of those works. If you'd rather call the binary method by default as it doesn't require a type conversion, this alternate call could be parameterized.

Would it be possible to switch to using this method in the QRCodeExternal call? With an Apple Watch there's so little screen real estate information density becomes very valuable, as the larger versions of a QR code can't really be used.

Thanks in advance, and again thanks for making it so easy to generate different QR codes!

dagronf commented 1 year ago

Hey mate - good idea! I've updated the library (tag 12.0.0) to support this feature -- If you now set the text on the document, it will pass the text to the external generator as text, meaning that if the generator can optimize the output it will.

I've added a set of tests in QRCodeExternalGenerationTests.swift and QRCodeDocGenerator.swift which exercises the new code.

Can you try updating to the 12.0.0 and see if you see the improvement?

CarterVE commented 1 year ago

Thank you for the fast response! Unfortunately it doesn't seem to be working for me, although it's a little hard to test because I just need to generate a qr code with the old method and then the new method and see if there's a difference. When you say set the text on the document, is there a way for me to do this other than initializing with or calling the .utf8String value on the document? Looking at your commit this seems like the correct approach, but the resulting output doesn't seem to be any different (I believe the alphanumeric version should be smaller as the same data can be encoded in less resolution). Do you know how I can verify that the alphanumeric encoding is actually being called (I can't modify the package without forking the external generator)?

Also, in upgrading from the version I was using (10.2.0) to the current version (12.0.0) I seem to have found another bug. The scale parameter was deprecated somewhere in between, and replaced with a dpi value. However, for some reason calling the function with a dpi equal to 72 * 3 = 216 (for scale = 3.0 on the apple watch) still generates a blurry code. Do you know what I'm doing wrong here (or is there a bug in the scaling)?

Thank you so much for your help, I'm happy to test anything you'd like to get this figured out!

Sharp vs. blurry qr code examples, both look the same (text being encoded is just repeated "this is a test to encode a bunch of data in the qr code" which should be able to use alphanumeric):

Simulator Screen Shot - Apple Watch Series 8 (45mm) - 2023-01-10 at 14 37 55 Simulator Screen Shot - Apple Watch Series 8 (45mm) - 2023-01-10 at 14 36 04

dagronf commented 1 year ago

setting utf8String should be good enough to trigger the optimisations if they apply. Can you send me a sample of the data you’re trying to encode for me to test with?

dagronf commented 1 year ago

Regarding the image blurriness, can you send me a sample of the code you’re using to generate the image?

CarterVE commented 1 year ago

Strange, I'm not sure why it's not triggering then, for example the data I'm encoding in those two codes is:

this is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr code

The other data was a JSON which wouldn't trigger the alphanumeric encoding, once I can get it to work I'm going to go through and replace JSON characters with supported alphanumeric characters.

CarterVE commented 1 year ago

For the image blurriness, the code I'm using to generate the image is:

let doc = QRCode(utf8String: string, errorCorrection: .medium, generator: QRCodeGenerator_External()) let generatedImage = doc.uiImage(CGSize(width: qrCodeSize, height: qrCodeSize), dpi: 72.0 * 3.0)

dagronf commented 1 year ago

Strange, I'm not sure why it's not triggering then, for example the data I'm encoding in those two codes is:

this is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr code

The other data was a JSON which wouldn't trigger the alphanumeric encoding, once I can get it to work I'm going to go through and replace JSON characters with supported alphanumeric characters.

Ah. The external generator only optimises UPPER case characters. That might explain it.

CarterVE commented 1 year ago

Strange, I'm not sure why it's not triggering then, for example the data I'm encoding in those two codes is: this is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr codethis is a test to encode a bunch of data in the qr code The other data was a JSON which wouldn't trigger the alphanumeric encoding, once I can get it to work I'm going to go through and replace JSON characters with supported alphanumeric characters.

Ah. The external generator only optimises UPPER case characters. That might explain it.

Yeah I was just messaging I realized the same thing, upper characters fixed it! My bad!

CarterVE commented 1 year ago

Any insights on the blurry QR codes with the new dpi setting? I can open a new issue for that if you'd like

dagronf commented 1 year ago

Yeah Im gonna take a look at it after breakfast... (still waking up here)

CarterVE commented 1 year ago

Amazing thanks man!

dagronf commented 1 year ago

@CarterVE I've pushed up a new tag (12.1.1) which fixes this issue. The UIImage wasn't correctly applying the dpi factor when generating the image.

I've also added the uiImage(dimension:scale:) api back on the QRCode.Document as scale is far more consistent with UIKit. The DPI calls are still there, but the current UIKit APIs use scale for more consistently. I'm a macOS dev so I'm much more used to seeing dpi.

API Change for uiImage(dimension:scale:)

See how you go!

CarterVE commented 1 year ago

@dagronf Thank you so much that worked perfectly, I'll probably use the uiImage(dimension:scale:) implementation because as you said it's more consistent with UIKit! Thank you so much for taking the time to fix/add these features, I can't believe how quickly you implemented this stuff, and I really appreciate it.

P.S. I saw you implemented my amazing test string for alphanumeric optimization, I love it lol

dagronf commented 1 year ago

@CarterVE Glad that your issue has been resolved mate.

P.S. I saw you implemented my amazing test string for alphanumeric optimization, I love it lol

Hehehe. Best way to track a bug is to write a test case first that shows the issue :-). Also means that if regressions occur the tests pick it up straight away.

Cheers -- Don't forget to star the project if you haven't already :-).