Fix visual quality of text in QR image #71

1. 
   hebasto commented at 9:21 PM on August 23, 2020: member

   Master (197450f80868fe752c6107955e5da80704212b34):

   This PR (6954156b4091bc1e561502f0eef0cece56c76eec):

   • macOS 10.15.6

   • Linux Mint 20

   Fix #54 Fix https://github.com/bitcoin/bitcoin/issues/19103

   ---

   The first commit is easy to review with git diff --word-diff.

2. qt, refactor: Limit scope of QPainter object 8071c75d45
3. 
   MarcoFalke commented at 5:27 AM on August 24, 2020: contributor

   Concept ACK

4. 
   Sjors commented at 12:52 PM on August 28, 2020: member

   f4016dd looks way better on macOS. Can you explain what fixed it?

5. 
   hebasto commented at 1:42 PM on August 28, 2020: member

   @Sjors

   f4016dd looks way better on macOS. Can you explain what fixed it?

   • ~QFontDatabase::GeneralFont instead of QFontDatabase::FixedFont~ see #71 (comment)
   • dropped QFont::NoAntialias (actually reverse of https://github.com/bitcoin/bitcoin/pull/17257)
   • added QFont::AbsoluteSpacing 1 px
6. 
   michaelfolkson commented at 7:15 AM on September 3, 2020: member

   Looking for Concept (N)ACKs (plus comments if you have them) from designers cc @GBKS @Bosch-0 @johnsBeharry

7. 
   michaelfolkson commented at 7:18 AM on September 3, 2020: member

   Concept ACK from me on fixing visual quality. I'll leave the font discussion to the designers.

8. 
   Bosch-0 commented at 12:02 PM on September 3, 2020: none

   Concept ACK

   If #79 gets merged and we use an embedded mono font this should be used here.

9. 
   hebasto commented at 12:17 PM on September 3, 2020: member

   @Bosch-0

   If #79 gets merged and we use an embedded mono font this should be used here.

   In image or in "Address" label?

10. 
    Bosch-0 commented at 12:20 PM on September 3, 2020: none

    Both

11. 
    luke-jr commented at 1:59 PM on September 3, 2020: member

    Would prefer to keep the font monospaced

12. 
    GBKS commented at 11:31 AM on September 7, 2020: none

    Concept ACK on fixing the visual quality.

    A monospace font would be nice, and also a narrow space every 4 characters (like in credit card numbers, some exploration here) for easier comparison, but all of that seems like a different issue.

13. qt: Fix visual quality of text in QR image 6954156b40
14. hebasto force-pushed on Sep 7, 2020
15. 
    hebasto commented at 12:57 PM on September 7, 2020: member

    Updated f4016dd0190cd60bfc86f095d41ad41afbc6c891 -> 6954156b4091bc1e561502f0eef0cece56c76eec (pr71.01 -> pr71.02, diff):

    • reverted back to a system monospaced font as requested by @Bosch-0, @luke-jr and @GBKS
16. 
    luke-jr commented at 1:30 PM on September 18, 2020: member

    Before: After:

    I don't see a noteworthy improvement...

17. 
    hebasto commented at 1:52 PM on September 18, 2020: member

    @luke-jr

    I don't see a noteworthy improvement...

    Mind sharing your system details?

    On your screenshots the address string does not even fit in the allocated space... Going to reproduce such behavior.

18. 
    luke-jr commented at 2:14 PM on September 18, 2020: member

    Qt GUI 5.14.2

    Preferred font https://luke.dashjr.org/education/tonal/glyphs/fonts/Console/

    (Note that it is a bitmap font and only supports one size)

19. 
    hebasto commented at 5:57 PM on September 18, 2020: member

    (Note that it is a bitmap font and only supports one size)

    This explains the string size issue.

20. 
    RandyMcMillan commented at 7:46 AM on September 20, 2020: contributor

    It may be best to not add the address to the qrcode image. This would free up space to display a larger qrcode image in the imageView. Maybe the save image dialogue should auto populate the name field with the address. This would allow the user to check the address as they are saving the image. Maybe the image name field should not be editable by default - and an edit check box option added to ensure that the user only changes the name of the image intentionally.

21. jonasschnelli added this to the milestone 0.21.0 on Oct 23, 2020
22. 
    jonasschnelli commented at 9:50 AM on October 23, 2020: contributor

    Tested ACK 6954156b4091bc1e561502f0eef0cece56c76eec - tested on macOS 10.15. Fixes the problem.

    The font still remains non-HiDPI. This is the image saved through the dialogs save function:

    The image seems to use a HiDPI resolution but the font isn't rendered in HiDPI.

23. jonasschnelli merged this on Oct 23, 2020
24. jonasschnelli closed this on Oct 23, 2020

    ---

25. hebasto deleted the branch on Oct 23, 2020
26. sidhujag referenced this in commit a4909c6b06 on Oct 23, 2020
27. apoelstra referenced this in commit 50437676db on Dec 3, 2020
28. gwillen referenced this in commit 6682a291c2 on Mar 23, 2021
29. bitcoin-core locked this on Feb 15, 2022

Milestone
0.21.0