search

Bacon / BaconQrCode

QR Code Generator for PHP
BSD 2-Clause "Simplified" License
1.82k stars 208 forks source link

Fix unreadable Shift-JIS encoded QR Code #173

Open askdkc opened 5 months ago

askdkc commented 5 months ago

This PR is fix for the issue #172

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (main@c01758c). Click here to learn what that means.

Files Patch % Lines
src/Encoder/Encoder.php 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #173 +/- ## ======================================= Coverage ? 64.93% Complexity ? 980 ======================================= Files ? 48 Lines ? 3094 Branches ? 0 ======================================= Hits ? 2009 Misses ? 1085 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

DASPRiD commented 5 months ago

I'm not a big fan of this – the kanji mode exists specifically to massively reduce the amount of storage required for that.

Looking at the original zxing code I noticed that they subtract 1 from the length to make it uneven, which we don't do.

Could you test locally if that makes a difference for your result?

DASPRiD commented 5 months ago

I also tried to generate the same QR code with ZXing, which yielded a valid QR code, so there must be some bug in Bacon, but I can't tell, which.

QR Code

askdkc commented 5 months ago

@DASPRiD

I tested your suggestion but unfortunately the result is still unreadable QR code. qrcode

askdkc commented 5 months ago

I also tried to generate the same QR code with ZXing, which yielded a valid QR code, so there must be some bug in Bacon, but I can't tell, which.

@DASPRiD

Could you test with "あいうえお" not "あいうえお123"?

DASPRiD commented 5 months ago

You can open the link yourself :)

askdkc commented 5 months ago

You can open the link yourself :)

My bad😅 (I thought you are testing it in locally.)

It works with "あいうえお" in ZXing. So yeah, I think something wrong with Bacon but I'm not sure what is.

DASPRiD commented 5 months ago

I'd recommend to poke around and compare the code paths for kanji between Bacon and ZXing, it might be some very small knob.