Skip to content

gui: disable font antialiasing for QR image address - #17257

Merged
laanwj merged 1 commit into
bitcoin:masterfrom
fanquake:disable_qr_font_antialiasing
Oct 26, 2019
Merged

laanwj merged 1 commit into
bitcoin:masterfrom
fanquake:disable_qr_font_antialiasing

Conversation

@fanquake

Copy link
Copy Markdown
Member

The address text inside the QR code is currently fairly blurry / unreadable. Explicitly disabling font antialiasing improves that somewhat.

master (693e400):
macOS_master

PR (e156b9d):
macOS_pr

@fanquake fanquake added the GUI label Oct 25, 2019
@luke-jr

luke-jr commented Oct 25, 2019

Copy link
Copy Markdown
Contributor

That sounds like a platform bug?

Would rather not disable antialias (an improvement) because of some platforms doing it wrong...

@fanquake

Copy link
Copy Markdown
Member Author

Would rather not disable antialias (an improvement) because of some platforms doing it wrong...

I didn't mention a specific platform in the PR description, because disabling has been an improvement on all the platforms I tested. Can you show me where it's a regression in readability?

@laanwj

laanwj commented Oct 26, 2019 •

Copy link
Copy Markdown
Member

ACK e156b9d

Readability takes precedence here over a debatable "improvement" in rendering. On the longer term someone could figure out the cause, and come up with a better patch, but at least this trivial change we can still sneak into 0.19.0.

67591424-6dd76d80-f72b-11e9-86b6-b3911f8e07e6

FWIW this looks is a bitmap font, which really don't lend themselves to anti-aliasing, only vector fonts do.
(so apparently for bitmap fonts, Qt interprets "anti-aliasing" as doing a smooth scale of the letters instead of a nearest-filter one, i'd say this is a very bad idea)

@laanwj laanwj added this to the 0.19.0 milestone Oct 26, 2019
@JeremyCrookshank

Copy link
Copy Markdown
Contributor

ACK

@DrahtBot

Copy link
Copy Markdown
Contributor

Gitian builds for commit 25d7e2e (master):

Gitian builds for commit 7499c796fb447a9de846252a7fa336c684ace90b (master and this pull):

laanwj added a commit that referenced this pull request Oct 26, 2019
e156b9d gui: disable font antialiasing for QR image address (fanquake)

Pull request description:

  The address text inside the QR code is currently fairly blurry / unreadable. Explicitly disabling font antialiasing improves that somewhat.

  master (693e400):
  ![macOS_master](https://user-images.githubusercontent.com/863730/67591414-644e0580-f72b-11e9-8399-2cd0584e7d62.png)

  PR (e156b9d):
  ![macOS_pr](https://user-images.githubusercontent.com/863730/67591424-6dd76d80-f72b-11e9-86b6-b3911f8e07e6.png)

ACKs for top commit:
  laanwj:
    ACK e156b9d

Tree-SHA512: 32aeb2ffe8164a1006f80e76c6e413fcb88e32ced42d2b2af69cca908bd32673f3e379184be917f1870864b940db943e7f46a7ecb0779343d5d129b381660c38
@laanwj
laanwj merged commit e156b9d into bitcoin:master Oct 26, 2019
laanwj pushed a commit that referenced this pull request Oct 26, 2019
More info available here: https://doc.qt.io/qt-5/qfont.html#StyleStrategy-enum

Github-Pull: #17257
Rebased-From: e156b9d
Tree-SHA512: 6e36d626215d5e66796bd3935037eef89ae810e4855e321f9f37b90c3be0b61de21cd655f23bc6a29f41038b5179d92578ab19ab89ccd525d9389ab868456827
@promag

promag commented Oct 27, 2019

Copy link
Copy Markdown
Contributor

ACK eb5a899.

@fanquake
fanquake deleted the disable_qr_font_antialiasing branch October 28, 2019 12:03
SmarterHomes pushed a commit to SmarterHomes/chaincoin that referenced this pull request Nov 17, 2019
More info available here: https://doc.qt.io/qt-5/qfont.html#StyleStrategy-enum

Github-Pull: bitcoin#17257
Rebased-From: e156b9d
Tree-SHA512: 6e36d626215d5e66796bd3935037eef89ae810e4855e321f9f37b90c3be0b61de21cd655f23bc6a29f41038b5179d92578ab19ab89ccd525d9389ab868456827
@hebasto

hebasto commented Sep 3, 2020

Copy link
Copy Markdown
Member

Reverting suggested in bitcoin-core/gui#71

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants