Bug 139778 - Switch to ZXing-C++ library for generating QR codes
Summary: Switch to ZXing-C++ library for generating QR codes
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium enhancement
Assignee: Homeboy_445
URL:
Whiteboard: target:7.2.0
Keywords: difficultyInteresting, easyHack, skillCpp
Depends on:
Blocks: QR-code
  Show dependency treegraph
 
Reported: 2021-01-20 07:39 UTC by Buovjaga
Modified: 2021-07-15 00:25 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Buovjaga 2021-01-20 07:39:23 UTC
This task consists of bundling another QR code library while switching the library calls to use it instead of the old one.

Background: the QR code generating functionality was added in a 2019 GSoC project: https://wiki.documentfoundation.org/Development/GSoC/Successfully_Implemented_Ideas#Generating_QR_codes
At the time, "QR Code generator" seemed like the only usable C++ library. Later, the creator of the library said it wasn't actually ready for production use.

Recently, a new C++ port of the venerable "Zebra Crossing" library appeared: https://github.com/nu-book/zxing-cpp
It is seeing rather good adoption in Linux distros: https://repology.org/project/zxing-cpp-nu-book/versions

It has the advantage that it can also generate classic barcodes. Exposing the barcode functionality in the UI can be a future easy hack.

The source archive has already been uploaded to https://dev-www.libreoffice.org/src/

For code pointers, here is the commit that bundled the current library:
https://git.libreoffice.org/core/commit/b4141cade04dac0c9d47293313a4521282975f12

Other relevant code paths can be discovered by looking at Shubham's commit history.

Samuel has agreed to do code reviews for this task.

Small note: ZXing-C++ says of the compiler baseline: "minimum VS 2019 16.8 / gcc 7 / clang 5". However, Visual Studio 16.8 is only required when using the Python wrapper, so it is not a blocker for us.
Comment 1 Tomaz Vajngerl 2021-01-20 10:10:26 UTC
It looks like it also supports other 1D and 2D barcode formats too, which would be great if we make this available after the work.
Comment 2 Buovjaga 2021-01-29 18:21:25 UTC
The Help content for this feature: https://help.libreoffice.org/latest/en-US/text/shared/guide/qrcode.html
Comment 3 Homeboy_445 2021-02-19 16:09:59 UTC
I would like to ask a few questions regarding bundling:

1.)Where to find the name of the package so as to include it in bundling like qrcodegen for QR-code-generator library.

2.)Also, should we include multiple header files in the configure.ac file instead of only one(as the requirement I think is to include multiple headers see: https://github.com/nu-book/zxing-cpp/blob/master/example/ZXingWriter.cpp) & which I think would require the use of the macro AC_CHECK_HEADERS.

3.)How to generate SHA256SUM in download.lst file for the package zxing-cpp-1.1.1.tar.gz.
Comment 4 Buovjaga 2021-02-19 16:57:25 UTC
(In reply to Homeboy_445 from comment #3)
> 3.)How to generate SHA256SUM in download.lst file for the package
> zxing-cpp-1.1.1.tar.gz.

At least on Linux you can just say

sha256sum zxing-cpp-1.1.1.tar.gz

thanks to https://www.gnu.org/software/coreutils/manual/html_node/sha2-utilities#sha2-utilities
Comment 5 Homeboy_445 2021-03-09 14:54:03 UTC
Since the bundling is somewhat complete, I would like to propose some slight changes in the UI of the Dialog box for Qrcodegen:

1.)Since the generation of QR code in ZXing requires margin length instead of the border(which was the case with qrcodegen lib). what would be a nice option here, changing 'Border' to 'Margin' or keep it 'Border' only?

2.)Also, in the qrcodegen lib error correction code levels are explicitly defined i.e LOW, MED, QUARTILE & HIGH but in ZXing only discrete values are allowed from 0 to 8 and so I propose that we keep these levels as is and assign some discrete value to each of them like LOW=1, MED=3, QUARTILE=6, HIGH=8 or as needed.

3.)We only need Qr code generation for now & so I think we should put QRCODE in its type by default(as ZXing has various other modes as well & so it needs to be specified).

Please see:https://nu-book.github.io/zxing-cpp/demo_writer.html
Comment 6 Commit Notification 2021-03-20 20:36:40 UTC
homeboy445 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/f72cd65eef4f9c32201ac785c424d5b18651464f

tdf#139778 bundle external:zxing lib

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2021-03-20 20:36:51 UTC
homeboy445 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/cdeb5046be7f6259bd60f0951c51559edcab91c5

tdf#139778 Switch to ZXing for generating QR code

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2021-03-20 20:38:02 UTC
homeboy445 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/66e88f46f7b9ef7a0b3366805860fd581bed6146

tdf#139778 qrcodegen library removal.

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Adolfo Jayme 2021-03-20 23:46:11 UTC
(In reply to Homeboy_445 from comment #5)
> 1.)Since the generation of QR code in ZXing requires margin length instead
> of the border(which was the case with qrcodegen lib). what would be a nice
> option here, changing 'Border' to 'Margin' or keep it 'Border' only?

Let’s change it.
Comment 10 Julien Nabet 2021-03-21 07:49:09 UTC
Remark: 
- QR-Code-generator-1.4.0.tar.gz 136kB
- zxing-cpp-1.1.1.tar.gz 123MB
Hope being able generate barcodes (even 1D) really worths it.
Comment 11 Buovjaga 2021-03-21 08:10:57 UTC
(In reply to Julien Nabet from comment #10)
> Remark: 
> - QR-Code-generator-1.4.0.tar.gz 136kB
> - zxing-cpp-1.1.1.tar.gz 123MB
> Hope being able generate barcodes (even 1D) really worths it.

The huge size comes from the test samples in the test subdirectory.
Comment 12 Julien Nabet 2021-03-21 08:19:01 UTC
(In reply to Buovjaga from comment #11)
> (In reply to Julien Nabet from comment #10)
> > Remark: 
> > - QR-Code-generator-1.4.0.tar.gz 136kB
> > - zxing-cpp-1.1.1.tar.gz 123MB
> > Hope being able generate barcodes (even 1D) really worths it.
> 
> The huge size comes from the test samples in the test subdirectory.

Great then! I was afraid it would inflate LO built package.
Thank you for the quick feedback!
Comment 13 Homeboy_445 2021-03-21 08:58:36 UTC
(In reply to Adolfo Jayme from comment #9)
> (In reply to Homeboy_445 from comment #5)
> > 1.)Since the generation of QR code in ZXing requires margin length instead
> > of the border(which was the case with qrcodegen lib). what would be a nice
> > option here, changing 'Border' to 'Margin' or keep it 'Border' only?
> 
> Let’s change it.

Alright! I'll see what can be done & send a new patch for it.
Comment 14 sdc.blanco 2021-03-21 15:08:18 UTC
> > option here, changing 'Border' to 'Margin' or keep it 'Border' only?
> Let’s change it.
I trust the help page will be updated to change "Border" to "Margin" and check if the other information on the page is still accurate.
https://help.libreoffice.org/7.2/en-US/text/shared/guide/qrcode.html
Comment 15 Buovjaga 2021-03-21 16:11:15 UTC
(In reply to sdc.blanco from comment #14)
> > > option here, changing 'Border' to 'Margin' or keep it 'Border' only?
> > Let’s change it.
> I trust the help page will be updated to change "Border" to "Margin" and
> check if the other information on the page is still accurate.
> https://help.libreoffice.org/7.2/en-US/text/shared/guide/qrcode.html

I can help with this when the time comes
Comment 16 Buovjaga 2021-03-23 09:28:08 UTC
(In reply to Buovjaga from comment #0)
> It has the advantage that it can also generate classic barcodes. Exposing
> the barcode functionality in the UI can be a future easy hack.

Now created as bug 141193
Comment 17 Homeboy_445 2021-03-24 08:00:33 UTC
(In reply to Buovjaga from comment #15)
> (In reply to sdc.blanco from comment #14)
> > > > option here, changing 'Border' to 'Margin' or keep it 'Border' only?
> > > Let’s change it.
> > I trust the help page will be updated to change "Border" to "Margin" and
> > check if the other information on the page is still accurate.
> > https://help.libreoffice.org/7.2/en-US/text/shared/guide/qrcode.html
> 
> I can help with this when the time comes

Fortunately, The change has been merged successfully.['Border' has been changed to 'Margin']
Comment 18 Commit Notification 2021-03-24 10:29:08 UTC
Ilmari Lauhakangas committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/help/commit/a202f3a40b93e86354a32bcac95e595f5f636769

tdf#139778 Change border to margin in QR code help
Comment 19 stragu 2021-07-10 06:30:21 UTC
Wondering if we are missing a mention of ZXing in /core/readlicense_oo/license/license.xml ?