"Mikolaj Zalewski" mikolajz@google.com wrote:
This is some overlap with freetype as freetype 2.3.5 can load fonts from PE resources. But the code is so small (we have all the tools needed) and helps many programs (including working around the Photoshop Z-order problem during startup as now the dialogs doesn't show) that I thought it is useful.
Then there is no point in using FreeType for loading font files at all, (or adding support for new font file formats to FreeType) since FreeType can load fonts from memory.
TRACE("WineEndAddFontResourceEx failed on PE file %s - trying to load resources manualy\n",wine_dbgstr_w(str));if (EnumResourceNamesW(hModule, rt_font, load_enumed_resource, (LONG_PTR)&num_resources))
This code snippet causes a stack corruption since load_enumed_resource has wrong calling convention. I wonder whether gcc issued a warning here.
Then there is no point in using FreeType for loading font files at all, (or adding support for new font file formats to FreeType) since FreeType can load fonts from memory.
Do you mean that in AddFontResource I shouldn't try to call WineEngAddFontResourceEx but do only a LoadLibraryEx (that won't work as at least according to MSDN the parameter to AddFontResource can be a FNT file or a Win16 NE executable) or that in AddFileToList I should not make a distinction between memory and file fonts but always load the font from memory, mmaping it before if necessary?
This code snippet causes a stack corruption since load_enumed_resource has wrong calling convention. I wonder whether gcc issued a warning here.
Thanks for catching it. I haven't noticed a warning but I think there should have been one.
Mikolaj Zalewski
"Mikolaj Zalewski" mikolaj@zalewski.pl wrote:
Then there is no point in using FreeType for loading font files at all, (or adding support for new font file formats to FreeType) since FreeType can load fonts from memory.
Do you mean that in AddFontResource I shouldn't try to call WineEngAddFontResourceEx but do only a LoadLibraryEx (that won't work as at least according to MSDN the parameter to AddFontResource can be a FNT file or a Win16 NE executable) or that in AddFileToList I should not make a distinction between memory and file fonts but always load the font from memory, mmaping it before if necessary?
I mean that we should choose from either leaving the task to FreeType, or doing the whole job of loading the font files internally in Wine.
Dmitry Timoshkov pisze:
"Mikolaj Zalewski" mikolaj@zalewski.pl wrote:
Then there is no point in using FreeType for loading font files at all, (or adding support for new font file formats to FreeType) since FreeType can load fonts from memory.
Do you mean that in AddFontResource I shouldn't try to call WineEngAddFontResourceEx but do only a LoadLibraryEx (that won't work as at least according to MSDN the parameter to AddFontResource can be a FNT file or a Win16 NE executable) or that in AddFileToList I should not make a distinction between memory and file fonts but always load the font from memory, mmaping it before if necessary?
I mean that we should choose from either leaving the task to FreeType, or doing the whole job of loading the font files internally in Wine.
I admit I don't understand - what would we gain from doing all the loading internally in Wine? Wouldn't that be simply rewriting a big part of freetype? You wrote that loading fonts from memory will help but isn't FT_New_Memory_Face using the same parsing code as FT_New_Face and only the source differs? Anyhow it looks to me as something rather orthogonal to this patch as I don't try to parse the content of the font but only remove the PE "wrapping".
Mikolaj Zalewski
"Mikolaj Zalewski" mikolaj@zalewski.pl wrote:
Then there is no point in using FreeType for loading font files at all, (or adding support for new font file formats to FreeType) since FreeType can load fonts from memory.
Do you mean that in AddFontResource I shouldn't try to call WineEngAddFontResourceEx but do only a LoadLibraryEx (that won't work as at least according to MSDN the parameter to AddFontResource can be a FNT file or a Win16 NE executable) or that in AddFileToList I should not make a distinction between memory and file fonts but always load the font from memory, mmaping it before if necessary?
I mean that we should choose from either leaving the task to FreeType, or doing the whole job of loading the font files internally in Wine.
I admit I don't understand - what would we gain from doing all the loading internally in Wine? Wouldn't that be simply rewriting a big part of freetype?
That's exactly my point. We shouldn't add workarounds for deficiencies in FreeType, especially since FreeType now handles fonts in PE format.
You wrote that loading fonts from memory will help but isn't FT_New_Memory_Face using the same parsing code as FT_New_Face and only the source differs? Anyhow it looks to me as something rather orthogonal to this patch as I don't try to parse the content of the font but only remove the PE "wrapping".
We don't we do that for other font formats, what's so special about PE, that Wine can handle it? But Wine can handle NE format as well, and there are older FreeType versions that can't handle NE fonts either. If that's just a worlaround for Photoshop's (because "many programs" that you mention your patch helps is so much vague, and while searching for fonts in PE fomat to test my FreeType patch I haven't found anything except Photoshop, so I had to create a sample font set in PE format on my own) Z-order problem during startup then your fix is in wrong area, you need to fix Z-order bug instead.
-
Dmitry Timoshkov -
Mikołaj Zalewski