Bug Hunting Session
Bug 97171 - Artifacts for italic text in GL rendering mode
Summary: Artifacts for italic text in GL rendering mode
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
5.2.0.0.alpha0+
Hardware: All Windows (All)
: medium major
Assignee: Michael Meeks
URL:
Whiteboard: target:5.2.0 target:5.1.2
Keywords: bibisected, bisected, regression
: 96116 97732 97860 97994 98101 98166 98170 98172 98697 98765 (view as bug list)
Depends on:
Blocks: OpenGL
  Show dependency treegraph
 
Reported: 2016-01-16 09:45 UTC by Urmas
Modified: 2016-10-17 15:08 UTC (History)
14 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot (1.16 KB, image/png)
2016-01-16 09:45 UTC, Urmas
Details
Trivial tahoma italic test case (8.07 KB, application/vnd.oasis.opendocument.text)
2016-02-23 11:03 UTC, Michael Meeks
Details
prototype fix. (10.17 KB, application/mbox)
2016-02-23 17:33 UTC, Michael Meeks
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Urmas 2016-01-16 09:45:32 UTC
Created attachment 121983 [details]
Screenshot

In 5.2 alpha, there is persistent garbage around the italicized text.
Comment 1 raal 2016-01-17 16:35:29 UTC
Please attach test file. No repro with Version: 5.2.0.0.alpha0+
Build ID: 0174562fa9e49bf989a571c6ccd51e558109b561
CPU Threads: 1; OS Version: Windows 6.1; UI Render: default; 
TinderBox: Win-x86@39, Branch:master, Time: 2016-01-16_00:03:18
Comment 2 Adolfo Jayme 2016-01-22 08:34:58 UTC
> UI Render: default;

I think you need to test with OpenGL rendering enabled.
Comment 3 raal 2016-01-22 09:27:46 UTC
(In reply to Adolfo Jayme from comment #2)
> > UI Render: default;
> 
> I think you need to test with OpenGL rendering enabled.

Of course, you're right. Confirmed. I'll bisect it.
Comment 4 raal 2016-02-15 11:45:05 UTC
This seems to have begun at the below commit.
Adding Cc: to Tor Lillqvist; Could you possibly take a look at this one? Thanks

93f2a6c86faefa237975b8b21e5b0d00cbd319e5 is the first bad commit
commit 93f2a6c86faefa237975b8b21e5b0d00cbd319e5
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Tue Aug 25 06:29:23 2015 -0700

    source sha:9a68eb9c1f54d4c4e14a46c11ba9eafca35a2b82

    source sha:9a68eb9c1f54d4c4e14a46c11ba9eafca35a2b82

author	Tor Lillqvist <tml@collabora.com>	2015-08-25 08:52:20 (GMT)
committer	Tor Lillqvist <tml@collabora.com>	2015-08-25 08:55:40 (GMT)
commit	9a68eb9c1f54d4c4e14a46c11ba9eafca35a2b82 (patch)
Comment 5 Buovjaga 2016-02-20 15:51:51 UTC
*** Bug 97732 has been marked as a duplicate of this bug. ***
Comment 6 Buovjaga 2016-02-20 21:00:58 UTC
*** Bug 97994 has been marked as a duplicate of this bug. ***
Comment 7 Buovjaga 2016-02-22 16:13:55 UTC
*** Bug 97860 has been marked as a duplicate of this bug. ***
Comment 8 Michael Meeks 2016-02-23 11:03:07 UTC
Created attachment 122903 [details]
Trivial tahoma italic test case
Comment 9 Michael Meeks 2016-02-23 13:14:41 UTC
disabling glyph caching retains the problem; which is obviously (in my test) the ` character 0x60 glyph next to the 0x61 'a' glyph

info:vcl.gdi.opengl:5196:5324:vcl/win/gdi/winlayout.cxx:246: this=08D158A8 97 old: {[101..106]}
info:vcl.gdi.opengl:5196:5324:vcl/win/gdi/winlayout.cxx:336: ABC widths: -1:10:0 7:1:1 1:8:-1 0:10:-1 1:7:-1 1:11:-3
info:vcl.gdi.opengl:5196:5324:vcl/win/gdi/winlayout.cxx:390: Tahoma: Escapement=0 Orientation=0 Ascent=16 InternalLeading=3 Size=(51,19) totWidth=89
info:vcl.gdi.opengl:5196:5324:vcl/win/gdi/winlayout.cxx:495: this=08D158A8 now: {[95..100],[101..106]}
info:vcl.gdi.opengl:5196:5324:vcl/win/gdi/winlayout.cxx:175: Bitmap 0E05160B: 89x23:
info:vcl.gdi.opengl:5196:5324:vcl/win/gdi/winlayout.cxx:185:




                              5X7                329                               915
                              915               915                                508
                               33               708                               923
                                                329                               807
                                  80XXX29      9152XX3           3XXX5        7XXXX19
                                  79  805      7X08 7X7        9129 57      9229 923
                                      917      329  807       9229          329  807
                                   3XXX08     915   708       708          708   519
                                 7X3  33      708   329      9229          329  923
                                807  807      329  915       915          915   807
                                519  519     915   329       805          807   519
                                319 5X3      7X5  319        913  57      915  3X3
                                80XX307     9221XX3           7XXX29       5XXX507

 3XXXXXXXX3

Is the relevant debug output; almost looks as if the ` is being moved towards the 'a' by the renderer ... looks visually nice but ... ;-)
Comment 10 Michael Meeks 2016-02-23 13:28:47 UTC
Adding some more fun debug to show where the glyphs are sliced up:

                      |       |             |                |             |
                      |       | 5X7         |       329      |             |
                      |       | 915         |      915       |             |
                      |       |  33         |      708       |             |
                      |       |             |      329       |             |
                      |       |     80XXX29 |     9152XX3    |       3XXX5 |
                      |       |     79  805 |     7X08 7X7   |     9129 57 |
                      |       |         917 |     329  807   |    9229     |
                      |       |      3XXX08 |    915   708   |    708      |
                      |       |    7X3  33  |    708   329   |   9229      |
                      |       |   807  807  |    329  915    |   915       |   9
                      |       |   519  519  |   915   329    |   805       |   8
                      |       |   319 5X3   |   7X5  319     |   913  57   |   9
                      |       |   80XX307   |  9221XX3       |    7XXX29   |
                      |       |             |                |             |
 3XXXXXXXX3           |       |             |                |             |

shows the ' clearly in the wrong box =)
Comment 11 Michael Meeks 2016-02-23 16:51:37 UTC
Italic fonts have some exciting overhang values; however with a bit of work I can get this rendering nicely; we could use some more cleanup in the caching logic here though I think; also I'll need to re-validate a number of other text rendering issues that may be similar.
Comment 12 Michael Meeks 2016-02-23 17:33:46 UTC
Created attachment 122926 [details]
prototype fix.

Fix needs some more cleanup, and ideally some more testing; it passes the gamut of what I've thrown at it (surprisingly including vertical text which just shouldn't work AFAICS ;-) anyhow - encouraging; should also save some texture memory if we're lucky.
Comment 13 V Stuart Foote 2016-02-24 06:03:20 UTC
(In reply to Michael Meeks from comment #12)
> Created attachment 122926 [details]
> prototype fix.
> 
> Fix needs some more cleanup, and ideally some more testing; it passes the
> gamut of what I've thrown at it (surprisingly including vertical text which
> just shouldn't work AFAICS ;-) anyhow - encouraging; should also save some
> texture memory if we're lucky.

You might check your fixed build against the test case from bug 95060 (combining diacritics) and also check rendering of glyphs from SEP in the Special Symbols dialog (with Segoe UI Symbols or Symbola) for bug 97319
Comment 14 Michael Meeks 2016-02-24 14:07:19 UTC
Hi there,

> You might check your fixed build against the test case from bug 95060
> (combining diacritics)

already done; doesn't fix it.

> and also check rendering of glyphs from SEP in the Special Symbols
> dialog (with Segoe UI Symbols or Symbola) for bug 97319

This is independent of GL - but its a good test-case; was not in my set - I see some oddness with OpenSymbol. Interesting. Thanks !
Comment 15 V Stuart Foote 2016-02-24 14:29:17 UTC
(In reply to Michael Meeks from comment #14)
> > and also check rendering of glyphs from SEP in the Special Symbols
> > dialog (with Segoe UI Symbols or Symbola) for bug 97319
> 
> This is independent of GL - but its a good test-case; was not in my set - I
> see some oddness with OpenSymbol. Interesting. Thanks !

Don't know, it seems like it is in the mix. For Windows at least, enabling / disabling OpenGL rendering nicely toggles the "glitch" in composing the grid of glyphs for codepoints beyond the BMP.  

But you are right there is something in adition to OpenGL rendering involved with composing the SpecialCharacter dialog -- see bug 97839
Comment 16 Adolfo Jayme 2016-02-25 00:19:55 UTC
*** Bug 98166 has been marked as a duplicate of this bug. ***
Comment 17 Adolfo Jayme 2016-02-25 00:21:55 UTC
*** Bug 96116 has been marked as a duplicate of this bug. ***
Comment 18 Buovjaga 2016-02-28 16:44:31 UTC
*** Bug 98172 has been marked as a duplicate of this bug. ***
Comment 19 Buovjaga 2016-02-29 06:25:59 UTC
*** Bug 98170 has been marked as a duplicate of this bug. ***
Comment 20 Michael Meeks 2016-03-02 18:20:44 UTC
My patch does some much better management of the DX offset, and works for reasonably sensible fonts =) but - it breaks in some corner-cases. One example is this:

info:vcl.gdi.opengl:4212:3344:vcl/win/gdi/winlayout.cxx:540: DX:offset : 17:1 10:2 11:1 9:2 11:0 0:1
info:vcl.gdi.opengl:4212:3344:vcl/win/gdi/winlayout.cxx:266: this=0CC6A0E8 32 old: {[43..48],[182..187],[286..291],[9832..9837],[9858..9863],[55356..55361]}
info:vcl.gdi.opengl:4212:3344:vcl/win/gdi/winlayout.cxx:357: ABC widths: 0:1:14 3:3:3 1:8:1 0:16:0 1:8:2 0:22:2
info:vcl.gdi.opengl:4212:3344:vcl/win/gdi/winlayout.cxx:431: OpenSymbol: Escapement=0 Orientation=0 Ascent=23 InternalLeading=0 Size=(87,29) totWidth=82
info:vcl.gdi.opengl:4212:3344:vcl/win/gdi/winlayout.cxx:533: this=0CC6A0E8 now: {[32..37],[43..48],[182..187],[286..291],[9832..9837],[9858..9863],[55356..55361]}
info:vcl.gdi.opengl:4212:3344:vcl/win/gdi/winlayout.cxx:176: Bitmap 7705322A: 82x33:
info:vcl.gdi.opengl:4212:3344:vcl/win/gdi/winlayout.cxx:205:      |       |            |                    |            |
     |       |            |                    |            |
     |       |            |                    |            |
     |       |            |                    |            |
     |       |            |       7X29  92X7   |            |
     |  719  |  3X0880X3  |       3X5   9108   |            |    3XX19         925
     | 80X3  |  3X0880X3  |      92X7   8029   |      818   |  92199219        53
     | 7XX29 |  3X0880X3  |      9108   5X3    |      818   |  7X5  7X3       829
     | 7XX29 |  3X0880X3  |      8029   3X5    |    80XXXX7 |  3X7  8019     927
     | 7XX29 |  3X1991X5  |      5X3   9208    |   5XXXXXXX3| 91X7  8008     53
     | 80X3  |  7X2992X7  | 92XXXXXXXXXXXXXX5  |  80X58187XX|791X7  9108    829
     | 80X3  |  803  508  | 92XXXXXXXXXXXXXX5  |  5X3 818 3X|391X7  8008   927
     | 91X5  |            |     9108   5X3     |  3X5 818   |  3X7  8019   53
     | 92X7  |            |     8029   3X7     |  3X3 818   |  7X5  7X3   829
     |  3X7  |            |     5X3   9208     |  7XX5818   |  92199219  927
     |  308  |            |     3X5   8019     |   5XXXX29  |    3XX19   53
     |  519  |            |    9208   7X29     |    92XXXXX5|           829   92XX19
     |  729  |            |  3XXXXXXXXXXXXXX3  |      8160XX|5         927   92199208
     |  84   |            |  3XXXXXXXXXXXXXX3  |      818 3X|29        53    5X5  7X3
     |  95   |            |    5X3   9208      |      818 80|19       829   92X7  8019
     |       |            |    3X5   8019      | 91X7 818 80|19      927    91X7  9108
     |       |            |   92X7   7X29      | 92X3 818 5X|29      53     91X7  9108
     | 92X7  |            |   9119   5X5       |  7XX58187XX|5      829     92X7  8019
     | 7XX29 |            |   7X29   3X7       |   3XXXXXXX3|      927       5X5  7X3
     | 92X7  |            |   5X3   9108       |    80XXX08 |      53        92199208
     |       |            |                    |      818   |     719         92XX19
     |       |            |                    |      818   |
     |       |            |                    |            |
     |       |            |                    |            |
     |       |            |                    |            |
     |       |            |                    |            |
     |       |            |                    |            |
     |       |            |                    |            |
01234 0123456 012345678901 01234567890123456789 012345678901 01234567890123456789012345

There is no $ symbol in OpenSymbol - so - where that comes from is of interest: not entirely obvious; and of course - that the metrics are wrong for it is not that surprising I guess.
Comment 21 Michael Meeks 2016-03-02 20:34:33 UTC
Interesting; seems to be the fallback from the simple layout code; we need to pass: ETO_GLYPH_INDEX into ExtTextOutW to avoid some internal windows fallback/confusion for missing glyphs for that use-case it seems. When using the:

VCL_NO_SIMPLEWINLAYOUT=1

code-paths; I (now) get ~perfectly matching output for the nasty emoji test document in bug#97319. So it seems the nasties are focused on the GL vs. non-GL SimpleWinLayout; which seems to be triggering some unfortunate and over-clever behaviour in the ExtTextOutW method hmm ... =)

Unfortunately; what is 'correct' is also not terribly obvious here.
Comment 22 Michael Meeks 2016-03-02 21:34:08 UTC
Interesting; https://barkingbogart.wordpress.com/2014/04/23/how-to-specify-character-and-word-spacing-that-supports-non-latin-languages-windows/ is quite helpful:

   "TextOut() and ExtTextOut() have some built-in font fallback and visual 
    reorder mechanism when used without any “special settings”. As seen in line 1 
    and 2, the text contains English, Chinese and Hebrew characters and “Calibri” 
    is selected as the font. “Calibri” does not have glyphs for Chinese and 
    Hebrew, yet all characters show up correctly. Also note that Hebrew text is 
    in correct visual order as Hebrew is a right-to-left language."

I imagine that GetCharABCWidths and other methods are not using the same fallback approach; and are instead doing this moderately unhelpful thing:

   "The ABC widths of the default character are used for characters
    outside the range of the currently selected font."

Ho hum. There is also some interesting retrospective on the GDI font API here:

http://blogs.msdn.com/b/text/archive/2009/04/15/introducing-the-directwrite-font-system.aspx

that confirms the suspicion that to have control and something approaching a physical-font API we need to use ETO_GLYPH_INDEX etc.
Comment 23 Tor Lillqvist 2016-03-10 15:10:47 UTC
A simple fix would be to reduce the DEFAULT_CHUNK_SIZE in WinFontInstance::AddChunkOfGlyphs() to one... (then the code that handles the cached glyphs could obviously also be simplified as each chunk would always have exactly one glyph). No idea how that would affect performance, though. Probably a hash map from glyph indices (or characters, in the SimpleWinLayout case) to glyphs would be needed at least.
Comment 24 Michael Meeks 2016-03-10 16:25:02 UTC
> A simple fix would be to reduce the DEFAULT_CHUNK_SIZE in 
> WinFontInstance::AddChunkOfGlyphs() to one...

I tried this; but the problem is not per-se the size of the texture (although it can be) - but our calculation of the size and metrics of the glyph we want to stamp / copy =) Also - it seems likely that we need to pack the glyphs into a single texture to accelerate rendering performance and save memory.

My patch fixes the italic issue quite nicely; but it's not a complete fix wrt. the lame metrics API.
Comment 25 Commit Notification 2016-03-14 11:05:03 UTC
Tim Eves committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e45d80f20ff6a05888f92ae942abed67609d5b20

tdf#97171: Use DirectWrite for OpenGL glyph caching

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 Commit Notification 2016-03-14 17:55:01 UTC
Tim Eves committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=620a1351a7627246f139a54a8cc3b35fa7ef8434&h=libreoffice-5-1

tdf#97171: Use DirectWrite for OpenGL glyph caching

It will be available in 5.1.2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 27 Michael Meeks 2016-03-14 21:34:27 UTC
Thanks to Tim & Tor for fixing this; italics and other faces should give no problem in master & 5.1.2.1 - thanks for reporting !
Comment 28 V Stuart Foote 2016-03-16 18:40:53 UTC
*** Bug 98697 has been marked as a duplicate of this bug. ***
Comment 29 Adolfo Jayme 2016-03-19 22:02:35 UTC
*** Bug 98765 has been marked as a duplicate of this bug. ***
Comment 30 V Stuart Foote 2016-03-19 23:46:46 UTC
*** Bug 98101 has been marked as a duplicate of this bug. ***