Bug Hunting Session
Bug 67086 - embedded Fonts renders strangely on first load
Summary: embedded Fonts renders strangely on first load
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.1.0.3 rc
Hardware: Other All
: medium normal
Assignee: Luboš Luňák
URL:
Whiteboard: target:4.2.0 target:4.1.1
Keywords:
Depends on:
Blocks: mab4.1
  Show dependency treegraph
 
Reported: 2013-07-19 12:58 UTC by Michael Meeks
Modified: 2013-11-21 18:46 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
font test document. (1.92 MB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2013-07-19 12:59 UTC, Michael Meeks
Details
trace (8.06 KB, text/plain)
2013-07-19 14:20 UTC, Michael Meeks
Details
valgrind trace of loading the many font sample (122.82 KB, text/plain)
2013-07-22 09:26 UTC, Michael Meeks
Details
re-order the cache cleanup - prototype patch (2.12 KB, patch)
2013-07-22 16:17 UTC, Michael Meeks
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2013-07-19 12:58:48 UTC
Of course; this could be an issue with all rendering of Ariel Narrow - and a more fundamental font problem, but it's interesting; it looks like the width metrics get busted.

Selecting and changing font seems to fix that.
Comment 1 Michael Meeks 2013-07-19 12:59:12 UTC
Created attachment 82683 [details]
font test document.
Comment 2 Michael Meeks 2013-07-19 13:22:45 UTC
To make this more surreal; if you re-load the document then everything is fine :-) My suspicion would be that we are doing something too clever: using the metrics from one font with another font (or somesuch?).

Either way - it looks beautiful as/when the metrics are right, I'll attach a more comprehensive test.
Comment 3 Michael Meeks 2013-07-19 13:24:52 UTC
http://users.freedesktop.org/~michael/fonttest-many.docx has a few more funky fonts in it - ~all of which work perfectly (AFAICS) but only on the 2nd load.
Comment 4 Michael Meeks 2013-07-19 13:46:29 UTC
Perhaps this is expected, but saving as DOCX having loaded, then re-loading that yields still broken fonts, the second reload of that though mends them ( at least for me ).
Comment 5 Michael Meeks 2013-07-19 13:53:16 UTC
Fridrich reports success on first-load on a minimal Windows VM without the C* / Bauhaus etc. fonts installed: http://people.freedesktop.org/~fridrich/fonts.png
I guess that makes it Linux specific ?

Norbert reports a crash on load on Mac, but it seems to be an unrelated coretext issue; so lets hope this is a Linux-specific issue.
Comment 6 Michael Meeks 2013-07-19 14:20:19 UTC
Created attachment 82691 [details]
trace

I attach Norbert's trace, possibly some font lifecycle issue ?
Comment 7 Michael Meeks 2013-07-22 09:26:59 UTC
Created attachment 82810 [details]
valgrind trace of loading the many font sample

In case it's a memory issue - I attach a valgrind log which may be helpful. Certainly looks like some font related stuff there.
Comment 8 Michael Meeks 2013-07-22 15:58:20 UTC
Help much appreciated with this one; here is what I managed to dig out by using valgrind and SAL_DEBUG in conjunction.

It -seems- that the invalid data is a stale PhysicalFontFace pointer - which seem to get inserted into the mpFontList here:

@@ -214,10 +221,13 @@ void OutputDevice::ImplUpdateFontData( bool bNewFontLists )
                 {
                     if( mpOutDevData )
                         mpOutDevData->maDevFontSubst.Clear();
!!                    mpGraphics->GetDevFontList( mpFontList );
!!                    mpGraphics->GetDevFontSubstList( this );

Those entries appear to be valid at insertion time, but then are later freed at the end of ImplUpdateAllFontData :-)

Which is somewhat annoying - and causes the underlying problem (AFAICS) - both the Mac crash and the intermittent Linux mis-substitution.
Comment 9 Michael Meeks 2013-07-22 16:17:16 UTC
Created attachment 82827 [details]
re-order the cache cleanup - prototype patch

Suggested patch; not hyper-happy with what's going on here. There seems to be some very significant cost to clearing these caches - why are we doing anything per Window around fonts ? :-) Anyhow - this at looks plausible I hope and only touches the ImplUpdateAllFontData method.
Comment 10 Michael Meeks 2013-07-22 20:05:57 UTC
Prototype patch is also valgrind clean (for me) for a load / exit.
Comment 11 Norbert Thiebaud 2013-07-23 06:48:53 UTC
with michael patch, mac does not crash anymoreI'm not sure if the displayed output is what was intented.
I see a plain document with a 'Embedded fonts test' title
then 
Callibi Baushaus Cambria Ariel Narrow and Brodway
in different fonts... I'm not familiar what the font must look like, but broadway at least looks like what I immagine it would (and Callibi is prolly really Callibri :-) )
Comment 12 Luboš Luňák 2013-07-23 09:05:50 UTC
This is good for me for master, but I'm somewhat uneasy about backporting this so short before release without more testing. I don't think we can do better though.
Comment 13 Commit Notification 2013-07-24 19:52:02 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

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

fdo#67086 - clear device font list before re-fetching it.



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 14 Commit Notification 2013-07-26 14:12:13 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "libreoffice-4-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ea82c62a3968181507581724000045d33c216e0d&h=libreoffice-4-1

fdo#67086 - clear device font list before re-fetching it.


It will be available in LibreOffice 4.1.1.

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 15 Michael Meeks 2013-07-26 14:41:21 UTC
Closing - unhappily :-) more review of this patch much appreciated, and/or a better two-stage cleanup of all those low-level graphics / glyph caches.