Skip to content

WIP: Fix font size dpi issue - bug 404011

Introduction

This MR contains Hack#2 (setPointSizeF(size*0.75)). 0.75 comes from 72/96.

Using setPixelSize() instead would lead to very similar (if not the same) results, but it was dismissed because:

  • we can't put fractional numbers there (another Qt patch...)
  • it makes libs/flake/tests/TestSvgText testTextTabSpacing ASSERT, because QTextLine::setNumColumns have different effect on fonts with pixel size versus fonts with point size (I don't know why :( )
  • not an argument against but if someone chooses to test setPixelSize(), they need this patch as well: fix_for_em_in_points_size.diff

Why I'm still unhappy about this MR:

  • size*0.75 looks very ugly. I'm afraid it will lead to more and more difficult to fix bugs in the future.
  • I believe there must be a way to fix it differently. Somewhere deep inside there is a misconception whether we paint text on 72dpi or 96dpi. But I really couldn't find it.
  • unit tests - from what I've found, they take a bit different path to get the results drawn than normal Krita. In the result if I put dpi there, it will give me the exact opposite effect than in Krita: higher dpi => smaller text in pixels. I had a patch for that: fix_for_unit_tests_dpi_weirdness.diff which wouldn't break Test 3, 4 or 5, but it would break Test 1 and 2.

Explanation of commits:

  • Dmitry's commit: that's the patch Dmitry gave me to fix the issues on loading the same file with different dpis and getting different-looking results.
  • Tests commit: contains changes to tests. However they are not fixed anyway.
  • Patch 2 commit: the commit with a hack.

Before merging

(if this MR ever comes to the merging state):

  • please make sure all commit messages are nice and pretty and explain what those commits did.

Test Plan

  1. Make a new document (144 or 300 dpi; must be reasonably bigger than 72dpi), add text. Copy and paste text shape. See if the copied text shape is the same size as the original one. (If broken, the new text shape would be bigger).
  2. Make a new document (144 or 300 dpi; must be reasonably bigger than 72dpi), add text. Save. Open in a new tab. See if text is the same size as on the original.
  3. Text "Chylanka" on Chilanka font (or any regular font) with 20pt font size should be approximately 20 pixels high (including both the upper part of "C" and the lower part of "y") when used on 72dpi document.
  4. Text "Chylanka" on Chilanka font (or any regular font) with 20pt font size should be approximately 40 pixels high (including both the upper part of "C" and the lower part of "y") when used on 144dpi document.
  5. libs/flake/tests/TestSvgText testTextTabSpacing will fail, but it should not ASSERT.

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken.
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.
Edited by Agata Cacko

Merge request reports