http://bugs.winehq.org/show_bug.cgi?id=30798
Bug #: 30798 Summary: gdiplus: GdipNewInstalledFontCollection fails in case system have got at least one OTF font installed Product: Wine Version: 1.5.5 Platform: x86 OS/Version: Linux Status: UNCONFIRMED Severity: minor Priority: P2 Component: gdiplus AssignedTo: wine-bugs@winehq.org ReportedBy: mooroon2@mail.ru CC: dmitry@baikal.ru Classification: Unclassified
Recently I had been investigating Wine testcase failures on my linux box and one of them being gdiplus testcase failing two tests at lines font.c:396 and font.c:397.
Wine tests results database have results from other linux boxes where this test does succeed so I was wondering what's wrong with it on my system. After digging into the code for about an hour it turned out that gdiplus implementation is flawed in a way that it doesn't expect font type to be anything other than RASTER_FONTTYPE or TRUETYPE_FONTTYPE. It is not the case with current Wine's freetype-based font engine driver for OpenType fonts (OTF) - they are treated to be DEVICE_FONTTYPE.
It results in situation when add_font_proc() callback trying to add/process OTF fonts followed by is_font_installed_proc() terminating enumeration process as soon as it encounters a font with type != TRUETYPE_FONTTYPE.
Trivial patch as I attach to this bug report workarounds the problem and makes testcase pass these tests. I'm not sure that there are no bad consequences in treating DEVICE_FONTTYPE fonts the same way it treats TRUETYPE_FONTTYPE in gdiplus, but looking through Wine's freetype font driver makes me believe that the only font type that gets assigned DEVICE_FONTTYPE is the OTF format - and AFAIK it is pretty much the same as TTF WRT its properties and capabilities.
Another way to "fix" the thing is to skip fonts which have type other than TRUETYPE_FONTTYPE in add_font_proc() callback or rewrite Wine's freetype font engine driver to report TRUETYPE_FONTTYPE for OTF fonts (and make sure that the behavior for these font types is just the same). I don't gave enough knowledge in this area so we need someone competent it this question to fix this bug.
http://bugs.winehq.org/show_bug.cgi?id=30798
--- Comment #1 from Alexey Loukianov mooroon2@mail.ru 2012-05-30 01:56:34 CDT --- Created attachment 40332 --> http://bugs.winehq.org/attachment.cgi?id=40332 Trivial patch that fixes problem and makes test suite pass font.c tests. This patch is rather untested and might have unknown consequences on real apps.
Attaching patch, failed to properly attach it upon bug report submission.
http://bugs.winehq.org/show_bug.cgi?id=30798
--- Comment #2 from Vincent Povirk madewokherd@gmail.com 2012-05-30 08:56:26 CDT --- Patches are not picked up from Bugzilla. See http://wiki.winehq.org/SubmittingPatches
http://bugs.winehq.org/show_bug.cgi?id=30798
Dmitry Timoshkov dmitry@baikal.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|dmitry@baikal.ru |
http://bugs.winehq.org/show_bug.cgi?id=30798
--- Comment #3 from Alexey Loukianov mooroon2@mail.ru 2012-05-30 10:36:44 CDT --- (In reply to comment #2)
Patches are not picked up from Bugzilla. See http://wiki.winehq.org/SubmittingPatches
Vincent, this patch is not something that I would dare to submit, it's just a blind uneducated guess on how to fix the problem. As I already wrote I don't have enough knowledge of freetype and Wine internals to fix this problem properly, thus I filed a bugreport and CC'ed author of the line in question - is was Dmitry Timoshkov accordingly to git blame output. It seems that he isn't interested in looking at this bug (at least now) as he had removed himself from CC list though.
http://bugs.winehq.org/show_bug.cgi?id=30798
--- Comment #4 from Dmitry Timoshkov dmitry@baikal.ru 2012-05-30 11:37:50 CDT --- (In reply to comment #3)
Vincent, this patch is not something that I would dare to submit, it's just a blind uneducated guess on how to fix the problem. As I already wrote I don't have enough knowledge of freetype and Wine internals to fix this problem properly, thus I filed a bugreport and CC'ed author of the line in question - is was Dmitry Timoshkov accordingly to git blame output. It seems that he isn't interested in looking at this bug (at least now) as he had removed himself from CC list though.
You may need to have another look at git blame output.
http://bugs.winehq.org/show_bug.cgi?id=30798
--- Comment #5 from Alexey Loukianov mooroon2@mail.ru 2012-06-02 04:21:40 CDT --- (In reply to comment #4)
You may need to have another look at git blame output.
$ git blame -L 647,647 font.c 83e88af3 (Dmitry Timoshkov 2012-05-11 19:21:00 +0900 647) if (type != TRUETYPE_FONTTYPE)
Dmitry, did you mean that the line 1374 is one to be blamed instead of 647?
$ git blame -L 1374,1374 font.c 531219f8 (Vincent Povirk 2010-04-15 11:49:20 -0500 1374) if (type == RASTER_FONTTYPE)
Looking at the commit dates I could tell that your commit is much more recent and a general empirical rule is that the "recent changes are ones to cause troubles most of the times". Actually I had already written that I have no enough knowledge on topic, so it either you or Vincent (or someone else from devteam) who are competent enough to diagnose the problem and develop a proper fix.
I realize that this one bug is a minor per se, but having Wine's gdiplus implementation to behave more correctly when the host system have OTF fonts installed isn't a bad thing really.
http://bugs.winehq.org/show_bug.cgi?id=30798
Dmitry Timoshkov dmitry@baikal.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW Ever Confirmed|0 |1
--- Comment #6 from Dmitry Timoshkov dmitry@baikal.ru 2012-06-02 04:39:52 CDT --- (In reply to comment #5)
Dmitry, did you mean that the line 1374 is one to be blamed instead of 647?
$ git blame -L 1374,1374 font.c 531219f8 (Vincent Povirk 2010-04-15 11:49:20 -0500 1374) if (type == RASTER_FONTTYPE)
I was referring to
Recently I had been investigating Wine testcase failures on my linux box and one of them being gdiplus testcase failing two tests at lines font.c:396 and font.c:397.
The change introduced by 83e88af325c993fad3052cb9e29fd137c332bb87 looks like this:
@@ -550,35 +625,84 @@ GpStatus WINGDIPAPI GdipGetFontHeightGivenDPI(GDIPCONST GpFont *font, REAL dpi, static INT CALLBACK is_font_installed_proc(const LOGFONTW *elf, const TEXTMETRICW *ntm, DWORD type, LPARAM lParam) { - if (!ntm || type == RASTER_FONTTYPE) - { + if (type != TRUETYPE_FONTTYPE) return 1; - }
'if (!ntm)' check is clearly wrong, so you may change back 'if (type != TRUETYPE_FONTTYPE)' to 'if (type == RASTER_FONTTYPE)', and send a patch.
P.S. Adding me to cc: list usually does nothing, this bugzilla is mostly ignored/ filtered out these days.
http://bugs.winehq.org/show_bug.cgi?id=30798
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, patch, source, | |testcase
--- Comment #7 from Austin English austinenglish@gmail.com 2012-06-13 17:43:35 CDT --- I see this as well on Debian testing, from: ii fonts-cantarell 0.0.8-1 sans serif font family designed for on-screen readability
though it appears this may be a dupe of bug 30755 (though this bug has a patch..).
http://bugs.winehq.org/show_bug.cgi?id=30798
Dmitry Timoshkov dmitry@baikal.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |ac38e69a92a443759cc8fb3291c | |50dce3bc5a83c Status|NEW |RESOLVED Resolution| |FIXED
--- Comment #8 from Dmitry Timoshkov dmitry@baikal.ru 2012-06-14 21:14:32 CDT --- Should be fixed now.
http://bugs.winehq.org/show_bug.cgi?id=30798
--- Comment #9 from Austin English austinenglish@gmail.com 2012-06-18 17:49:49 CDT --- (In reply to comment #8)
Should be fixed now.
Yep, thanks!
http://bugs.winehq.org/show_bug.cgi?id=30798
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #10 from Alexandre Julliard julliard@winehq.org 2012-06-22 13:29:51 CDT --- Closing bugs fixed in 1.5.7.