QRImageWidget: Sizing and font fixups #506

• Margin of QRCode should be constant, not change based on complexity of QRCode
• Text region should be sized based on height of actual text, not a constant guess
• Width of image should not be adjusted based on vertical text size/margins
• Adjusted constant QRCode and margin sizes to visually match what we had previously
• If text won't fit, grow up to 25% wider, or split across multiple lines

(Note: Out of scope for this PR, after #497, I think we should allow customising the font used here and default to the embedded monospace for better privacy)

Concept ACK, however, I'll try a different approach for the multi-line case.

@luke-jr feel free to try the commit 0ac4f7535a90bacb0e5ffe8f110505aa44075ad6 (branch) which avoids manual text wrap by using Qt::TextWrapAnywhere flag.

Another thing I noticed is that the default minimum font size in calculateIdealFontSize is too small.

        auto rect = fm.boundingRect(max_rect, Qt::AlignCenter, text);
        if (rect.width() > max_rect.width() * 5 / 4) {

Isn't this impossible? I find the unnecessary QRect usage hard to follow.

Another thing I noticed is that the default minimum font size in calculateIdealFontSize is too small.

IMO out of scope for this PR

Isn't this impossible?

No because it doesn't wrap.

I find the unnecessary QRect usage hard to follow.

And I think it's unnecessary to calculate lines and manually wrap the text. What is hard to follow?

@luke-jr Screenshots "before" and "after" in the PR description could be useful for further discussion.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

Concept ACK, stylistic changes look fine to me.

<img width="628" alt="Screen Shot 2022-06-28 at 2 11 01 AM" src="https://user-images.githubusercontent.com/23396902/176106266-1e871c8f-f315-445e-9af0-dafde75a40eb.png"> | <img width="628" alt="Screen Shot 2022-06-28 at 2 12 19 AM" src="https://user-images.githubusercontent.com/23396902/176106283-f0d058db-c1da-45b8-a834-350f24029f40.png"> |

Tested 4c5143d56c98d4f9f961059692f155bbe2248c44.

This PR effectively renders font size smaller that makes it harder to read.

• master branch:

• this PR:

<!--8ac04cdde196e94527acabf64b896448-->

There hasn't been much activity lately. What is the status here?

Finding reviewers may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

This #506#pullrequestreview-1043535557:

This PR effectively renders font size smaller that makes it harder to read.

remains unaddressed.

Concept ACK, I agree with the improvements on achieving a more consistent adjustment of the QR code image size. However, it appears that there is still an issue with the font size, which makes the address in the picture difficult to read. Is there a way to improve the legibility of the address text?

tested ACK 4c5143d56c98d4f9f961059692f155bbe2248c44

• master:

• this PR (rebased):

<!--2e250dc3d92b2c9115b66051148d6e47-->

🤔 There hasn't been much activity lately and the CI seems to be failing.

If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn't been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in.

Closing due to lack of support from the PR author (https://github.com/bitcoin-core/gui/pull/506#issuecomment-1549832808, #506#pullrequestreview-1630119899).