[Bug 50896] New: cloned member of PrivateFontCollection is supposed to survive deletion of the collection
https://bugs.winehq.org/show_bug.cgi?id=50896
Bug ID: 50896 Summary: cloned member of PrivateFontCollection is supposed to survive deletion of the collection Product: Wine Version: 6.5 Hardware: x86-64 URL: http://ytomy.sakura.ne.jp/cgi-bin/dl/dl.php?dl=trgssx OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: gdiplus Assignee: wine-bugs@winehq.org Reporter: galtgendo@o2.pl Distribution: ---
First summary is a guesstimate based upon What I'm seeing through WINEDEBUG="font,gdiplus".
An RPGMakerVX game using TRGSSX.dll has a problem with proper font display. AFAICT, the library function in use is DrawTextFastA. Anyway, what I can see in the log is roughly:
- call to GdipNewPrivateFontCollection - call to GdipPrivateAddFontFile loading 'UmePlus Gothic', followed by a call to GdipPrivateAddMemoryFont for that font - a somewhat long dive into gdi32 follows, ending with calls to GdipGetFontCollectionFamilyCount and GdipGetFontCollectionFamilyList on this collection - next is GdipCloneFontFamily call for 'UmePlus Gothic' and a GdipGetFamilyName call on the *cloned* family - next is a call to GdipDeletePrivateFontCollection...
and now, we're fucked - this has just deleted the font family, yet the app is still using the cloned version that from this point is just garbage in wine.
To be exact, what happens later is a GdipCreateFont call on the cloned family.
Game doesn't crash, but the picked font is obviously wrong.
Chances are this is a regression caused by commit b9307cfa61e9884b01d34c2310ec1ffbb1868728, but it's over a year ago, so I didn't test that.
What I did test was that changing GdipCloneFontFamily from '*clone = family' to an obviously wrong 'GdipCreateFontFamilyFromName(family->FamilyName, NULL, clone)' seems to fix the problem.
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #1 from Rafał Mużyło galtgendo@o2.pl --- I've been looking at some of gdiplus bugs. There were two that could be related to this problem.
AFAICT, when GdipPrivateAddFontFile is used, it doesn't add the font to standard gdi32 lookup list, so if an app wanted to use a bundled font via gdiplus, it might very well find itself unable to - generic fallback might kick in in most cases, but sometimes it might be simply lacking the glyphs found in the bundled font.
Also, I'm seeing something odd - fallbacks seem to work for CJK ranges (well, kana/kanji at least), but not for U+2600-U+26FF range. This might be due to a hack for bug 50880 (that makes other things work better), but it's still odd it depends on the range. My first guess would be that a font is falsely advertising coverage for the later range, which gets current matching scheme confused, but as of yet I've been unable to verify it one way or the other.
https://bugs.winehq.org/show_bug.cgi?id=50896
Esme Povirk madewokherd@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |madewokherd@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #2 from Esme Povirk madewokherd@gmail.com --- Based on the mingw headers' implementation of FontCollection::GetFamilies, it seems that GdipCloneFontFamily does something. According to comments, the example at https://docs.microsoft.com/en-us/windows/win32/api/gdiplusheaders/nf-gdiplus... crashes trying to delete the font families, if the wrapper does not clone them.
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #3 from Esme Povirk madewokherd@gmail.com --- I sent a patch for this: https://www.winehq.org/pipermail/wine-devel/2021-May/187654.html
I don't really have a way to test it, though. As you've observed, Wine doesn't complain if the font family is used after being deleted, so I just have to hope I've implemented this correctly.
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #4 from Rafał Mużyło galtgendo@o2.pl --- Please explain this patch to me, as it seems it wouldn't do the necessary work anyway - at most it would prevent invalid access (I'm not testing it yet, but I'll likely get to it soon).
AFAIU, in wine most of gdiplus is built on top of gdi32. I'm not quite sure what the lib code is doing, but I suspect it simply makes the font from the file available to gdi32 - I'm not sure if wine's gdiplus is able to do that (at least without explicit Replacement key entry for the family (or alternatively a fontconfig substitution) - and that wouldn't do quite what it's supposed to). That would mean that *some* font would be used, perhaps even one having necessary glyphs, but not the one meant to be used, despite being technically available.
So, the only part fixed would be trying to access garbage. A definite step up and an improvement over my incorrect hack, but not full solution yet.
Oh, and if I got that part on current wine code not making font families from PrivateFontCollection available to gdi32, would you point me to the code that does it ?
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #5 from Esme Povirk madewokherd@gmail.com --- I'm just working on one thing at a time - getting the memory management correct.
https://bugs.winehq.org/show_bug.cgi?id=50896
winetest@luukku.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |winetest@luukku.com
--- Comment #6 from winetest@luukku.com --- some patch went in which had link into this bug report.
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #7 from Rafał Mużyło galtgendo@o2.pl --- So, as expected, the patch worked the expected way - meaning the main problem is far from solved, but at least we're working with actual data now.
Which reminds me of something I've failed to mentioned before, that I've noticed while coming up with the hack.
GdipCreateFont as implemented in wine leaves lfCharSet for the font at 0. Just a guess, but shouldn't that be DEFAULT_CHARSET instead ?
Also, I wonder how would a proper fix for this would look like. I see that fontconfig has API for direct file scans, but FcFileScan is for adding to fontsets and currently wine seems to only be adding existing fontsets from fontconfig cache. Not to mention that removing those fonts afterwards might get tricky...
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #8 from Esme Povirk madewokherd@gmail.com --- Sadly this is beyond my font knowledge.
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #9 from Esme Povirk madewokherd@gmail.com ---
AFAICT, when GdipPrivateAddFontFile is used, it doesn't add the font to standard gdi32 lookup list, so if an app wanted to use a bundled font via gdiplus, it might very well find itself unable to - generic fallback might kick in in most cases, but sometimes it might be simply lacking the glyphs found in the bundled font.
We use AddFontMemResourceEx to add the font in the calling process. My understanding is that this makes it usable through gdi32.
We don't have any code to handle fallbacks in gdiplus, so unless ExtTextOut and GetGlyphOutline both handle it for us, we'd have to do more work for that.
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #10 from Rafał Mużyło galtgendo@o2.pl --- Well, I might be misreading the log, but the lines there seem to suggest that matching goes into System:Tahoma fallback and seems to ends up using glyphs from Source Han Sans.
...right now, this caught me at a bad time, might add a better comment next week...
https://bugs.winehq.org/show_bug.cgi?id=50896
--- Comment #11 from Rafał Mużyło galtgendo@o2.pl --- So, a week...never mind...
1. that minor GdipCreateFont question remains open - I will likely open a separate bug for it
2. the other part is likely *not* a gdiplus problem: I'm seeing something seemingly quite alike when gdiplus is not in use; I'll think about it a bit more and update bug 50880 (its current content looks invalid when paired with the data here - might end up moving it in the opposite to initial direction)
-
WineHQ Bugzilla