Bug 128687 - Internal leading not receiving same rounding as other font metrics
Summary: Internal leading not receiving same rounding as other font metrics
Status: RESOLVED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
6.0.0.3 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: Paragraph-Line-Spacing
  Show dependency treegraph
 
Reported: 2019-11-09 18:03 UTC by V Stuart Foote
Modified: 2020-06-18 14:24 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
sample various font metrics with underlining and needing internal leading (13.58 KB, application/vnd.oasis.opendocument.text)
2019-11-26 20:16 UTC, V Stuart Foote
Details

Note You need to log in before you can comment on or make changes to this bug.
Description V Stuart Foote 2019-11-09 18:03:36 UTC
When Caolán fixed bug 107249 [1][2] by casting scaled font metrics to float and then rounding, seems like the Internal Leading got missed for rounding. It continues to use the mnHeight value (and is left unscaled?) [3].

Not sure where exactly we use the Internal Leading as in ImplInitAboveTextLineSize() but something seems off.

Notice clipping with some fonts with the single U+01FA, or for strings with glyphs of a couple of combining glyphs. Eg., "AU+0300U+0302" or U+00C1U+0302"

As you zoom Writer canvas in and out--the glyphs get clipped at different heights.

=-ref-=
[1] https://gerrit.libreoffice.org/#/c/41933/
[2] https://gerrit.libreoffice.org/#/c/41944/
[3] https://opengrok.libreoffice.org/xref/core/vcl/source/font/fontmetric.cxx?r=07359ea2#379
Comment 1 Xisco Faulí 2019-11-25 15:06:32 UTC
Hi V Stuart,
is there any way to reproduce it systematically ?
Comment 2 Xisco Faulí 2019-11-26 11:00:40 UTC
@Caolán, if you have a minute, could you please comment here ?
Comment 3 Caolán McNamara 2019-11-26 11:42:16 UTC
is it speculation or something you're able to check ? I'm not sure what we should do differently, we're deriving mnIntLeading from the rounded values.
Comment 4 V Stuart Foote 2019-11-26 14:45:38 UTC
(In reply to Caolán McNamara from comment #3)
> is it speculation or something you're able to check ? I'm not sure what we
> should do differently, we're deriving mnIntLeading from the rounded values.

Hmm, am I reading the code for mnIntLeading wrong?

derived at line 379         mnIntLeading = mnAscent + mnDescent - mnHeight;

the scaled mnAscent is converted and rounded, at line 374
the scaled mnDescent is converted and rounded, at line 375

but mnHeight remains as read from font metric, at line 83, with no scaling, nor a conversion to float for rounding. Wouldn't that make a result for mnIntLeading questionable.

Otherwise, will try to work up a sample text with: scaled, underlined, bolded, and combining diacritics, with representative fonts, that should show any effect of incorrect internal leading.

@Khaled, any input?
Comment 5 Caolán McNamara 2019-11-26 15:45:37 UTC
fScale is derived from mnHeight at line 340 the other values are scaled by fScale to bring them into the same space as mnHeight
Comment 6 V Stuart Foote 2019-11-26 20:16:52 UTC
Created attachment 156132 [details]
sample various font metrics with underlining and needing internal leading

Sample text with STIX Two Math, Arial, and DejaVu Serif

Each has different units per EM -- UPEM, and different font metric schemes.

Position cursor and select the U+01D9 Latin Capital Letter U With Diaersis and Caron on the line with Font DF set at 27pt.  Then zoom the page in an out 60% <--> 600%

Watch the highlight box of the selected Ǚ glyph: relative to the generated underline for text above, and its intersection with the Ǻ glyph below.

The line height changes, and underline position, recalculated during zoom seems to be more than just from the floating point glyph positioning of bug 103322

If we had everything correct with the line height then shouldn't the internal and external leading be calculated the same and not change with each zoom step?

=-testing-=
This was just on Windows 10 Ent 64-bit en-US (1903) with
Version: 6.3.3.2 (x64)
Build ID: a64200df03143b798afd1ec74a12ab50359878ed
CPU threads: 8; OS: Windows 10.0; UI render: GL; VCL: win; 
Locale: en-US (en_US); UI-Language: en-US
Calc: threaded

Both OpenGL rendering and Default GDI rendering are affected, but with slight differences.
Comment 7 Xisco Faulí 2020-06-18 10:34:58 UTC
Hello V Stuart,
Is this issue still reproducible with a master build ?
Comment 8 V Stuart Foote 2020-06-18 14:24:17 UTC
On Windows 10 (1909) against recent LO master with fonts needed for test document installed. 

STR of test document no longer showing differing line heights during zoom (presumably result of mishandling internal 'leading' for font).

Checked GDI default, GDI with HA, Skia with Vulkan or raster, and OpenGL

Any residual mispositioning/scaling issues would require work on bug 103322 to move layout to floating point.

=-testing-= 


Version: 7.1.0.0.alpha0+ (x64)
Build ID: a3c8ea5e644ca2fc04de9f01ba9f8ace47f520f0
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: default; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: threaded

Version: 7.1.0.0.alpha0+ (x64)
Build ID: a3c8ea5e644ca2fc04de9f01ba9f8ace47f520f0
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: Skia/Vulkan; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: threaded

Version: 7.1.0.0.alpha0+ (x64)
Build ID: a3c8ea5e644ca2fc04de9f01ba9f8ace47f520f0
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: GL; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: threaded